H1 reslice: close feed grammar and kind states - #44
Conversation
Co-Authored-By: Codex <noreply@openai.com>
🤖 Codex PR Review🚫 Merge blocked: 1 serious issue(s) found in high-risk files ⚖️ Codex Review Arbitration🚫 block: 🚫 Blocking IssuesThese issues must be fixed before this PR can be merged: 1. 🟠 [HIGH] Logic in
|
Co-Authored-By: Codex <noreply@openai.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 491350b9c6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| error_code = item["error_code"] | ||
| if error_code is None and state != "accepted": | ||
| _error("feed_error_invalid") | ||
| if error_code is not None and (type(error_code) is not str or not _ERROR.fullmatch(error_code)): | ||
| _error("feed_error_invalid") |
There was a problem hiding this comment.
When validating a wire status supplied to serialize_status() or parse_status_bytes(), this only requires an error for non-accepted feeds and never rejects error_code on an accepted feed. That admits a canonical status that says a feed is successful, increments successful_feed_count, and can set publication_complete/eligible_for_live_publication to true while still carrying error_code: "fetch_failed"; build_status() itself forbids that combination, so external or mutated status artifacts can become internally contradictory and mislead downstream health gates.
Useful? React with 👍 / 👎.
| DefusedXmlException, | ||
| ET.ParseError, | ||
| LookupError, | ||
| OSError, | ||
| RuntimeError, |
There was a problem hiding this comment.
In the RSS workflow I checked, .github/workflows/rss_source_pipeline.yml passes --continue-on-feed-error; with a truncated or malformed HTTP response, urllib/http.client can surface exceptions such as IncompleteRead or BadStatusLine, which are HTTPException subclasses rather than OSError in Python 3.11. Because this narrowed handler does not include that family, one flaky feed now aborts the whole scheduled pipeline instead of being recorded as a failed feed, regressing the previous except Exception behavior.
Useful? React with 👍 / 👎.
| ValueError, | ||
| UnicodeError, | ||
| ) as exc: | ||
| code = getattr(exc, "code", "fetch_failed") |
There was a problem hiding this comment.
When a feed is malformed, oversized, or schema-invalid and --continue-on-feed-error is used, this looks for exc.code, but FeedXmlError is only a ValueError raised with messages like feed_schema_invalid and never defines a code attribute. The status therefore records parser rejections as generic fetch_failed, making producer/schema problems indistinguishable from network failures and defeating the new sanitized error_code contract.
Useful? React with 👍 / 👎.
| error=f"{type(exc).__name__}: {exc}", | ||
| ) | ||
| parsed = parse_feed_snapshot(fetcher(feed.feed_url), feed, max_items=max_items_per_feed) | ||
| feed_rows = [PrimitiveRow.from_mapping(row).to_mapping() for row in parsed.entries] |
There was a problem hiding this comment.
For feed CSVs that omit or leave author blank, load_feed_config() still defaults feed.author to "", which previously produced valid rows with an empty author. This new PrimitiveRow conversion rejects empty strings for every row field, so an otherwise valid feed with a blank author now raises row_invalid or is marked failed under --continue-on-feed-error; either preserve the optional author behavior or validate it when loading config.
Useful? React with 👍 / 👎.
| if state == "failed" and item["accepted_row_count"] != 0: | ||
| _error("feed_state_invalid") | ||
| accepted_count = _counter(item["accepted_row_count"], "feed_counter_invalid") | ||
| _counter(item["rejected_row_count"], "feed_counter_invalid") |
There was a problem hiding this comment.
A wire status can set a feed-level and top-level rejected_row_count to a positive value while every feed is otherwise accepted; this only type-checks the count, and the later completeness calculation ignores rejected rows entirely, so serialize_status()/parse_status_bytes() can bless a canonical status with publication_complete and eligible_for_live_publication still true despite producer rows being rejected. Since build_status() never emits rejected rows today, mutated or external status artifacts with rejections should either be rejected or made ineligible instead of passing as complete.
Useful? React with 👍 / 👎.
1 participant
Scope
Fresh reslice from latest
origin/main@294e30b8eed36f070299b1dc01d34955fb262918; PR #43 is closed/frozen evidence and was not reopened or reused.This producer-boundary slice:
channelchild and rejects every extra root child;rss2/atomfor zero-entry feeds;unknownfeed kind only for failed outcomes; accepted/quarantined outcomes requirerss2oratomand contribute zero rows/digest when quarantined;No workflow, publish, QAR, weekly, permission, or automation changes are included.
Validation