Skip to content

Gate RSS live publication on complete fetches - #38

Closed
Pigbibi wants to merge 2 commits into
mainfrom
feat/pert-weekly-workflow-w0
Closed

Gate RSS live publication on complete fetches#38
Pigbibi wants to merge 2 commits into
mainfrom
feat/pert-weekly-workflow-w0

Conversation

@Pigbibi

@Pigbibi Pigbibi commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Summary

PR-W0 hardens the existing RSS production workflow only. It does not add the weekly artifact.

  • validates main-only workflow identity and canonical manual input paths before checkout
  • keeps the existing fetch behavior and uploads the legacy/debug source artifact before completeness gating
  • validates the authoritative fetch status shape; partial/failed feeds stop before any data/live mutation
  • verifies the checked-out SHA and pushes explicitly to HEAD:refs/heads/main
  • scopes push credentials to the push step and cleans the local extraheader on success/failure

Ordering contract

early guard -> checkout/SHA check -> fetch -> legacy debug artifact upload -> completeness gate -> complete-only live commit/push

A partial debug artifact is evidence only and is not a successful live publication. Weekly artifact generation is deferred to PR-W1 after this PR merges and post-main CI succeeds.

Validation

  • python3 -m pytest -q — 123 passed
  • uv sync --locked --extra test && uv run pytest -q — 123 passed
  • python3 -m compileall -q src scripts tests — passed
  • actionlint .github/workflows/rss_source_pipeline.yml — passed
  • git diff --check — passed
  • ruff unavailable in environment

Non-scope

No QAR, Pages, publisher, weekly/monthly artifact, identity/store/registry, new data source, or permission expansion.

Co-Authored-By: Codex <noreply@openai.com>
@github-actions

github-actions Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

🤖 Codex PR Review

🚫 Merge blocked: 2 serious issue(s) found in high-risk files

🚫 Blocking Issues

These issues must be fixed before this PR can be merged:

1. 🔴 [CRITICAL] Logic in src/political_event_tracking_research/rss_source_fetch.py

validate_fetch_status() treats any status with failed_feed_count == 0 as complete, even when feed_count == 0 and successful_feed_count == 0. If the canonical feed CSV is accidentally emptied or otherwise yields zero configured feeds, the workflow will pass the gate and overwrite data/live with empty CSVs and a fresh manifest. (line 81)

Suggestion: Fail closed when no feeds were fetched. At minimum require feed_count > 0 and successful_feed_count > 0; ideally pass the canonical feed config into the validator and require the status feed_count to match the expected configured feed count exactly.

2. 🟠 [HIGH] Logic in src/political_event_tracking_research/rss_source_fetch.py

parse_feed_items() returns an empty list for any XML document that has neither RSS <channel><item> entries nor Atom <entry> elements, and fetch_rss_sources() records that feed as ok=True. The new publication gate trusts that status, so a provider serving a syntactically valid XML/HTML error page can still be counted as a complete fetch and silently publish truncated or empty live outputs. (line 185)

Suggestion: Treat 'parsed but no recognizable RSS/Atom entries' as a fetch failure. Raise an error or emit ok=False when the root does not match a supported feed shape, so validate_fetch_status() can stop publication on semantic feed failures instead of only transport exceptions.

ℹ️ Other Findings

1. 🟡 [MEDIUM] Reliability in .github/workflows/rss_source_pipeline.yml

The workflow still uploads the debug artifact only after the combined fetch/extract/build shell step finishes. If extract_source_mentions.py or build_tracker.py crashes on malformed fetched data, the job exits before Upload RSS source artifact, so the run loses the very pre-gate evidence this PR says it preserves. (line 97)

Suggestion: Split fetch/status generation into its own step and upload data/output/rss_source_pipeline/ with if: always() immediately after that step, before running extraction/tracker or the completeness gate.


Review by Codex PR Review bot • PR

Co-Authored-By: Codex <noreply@openai.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b0748fe27b

ℹ️ 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".

path: data/output/rss_source_pipeline/
if-no-files-found: error
- name: Validate feed completeness
run: python scripts/validate_fetch_status.py --status data/output/rss_source_pipeline/source_fetch_status.json

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Fix the known failing canonical feed before gating

In scheduled/main runs that use the canonical config/free_rss_feeds.csv, this new unconditional completeness gate will block publication whenever any configured feed fails; the repository’s last committed data/live/source_fetch_status.json already records cisa-news from config/free_rss_feeds.csv:10 as HTTP Error 403 with failed_feed_count: 1, so if that source still behaves the same the RSS workflow will keep stopping here and never refresh data/live. Remove or fix the failing feed, or make known non-blocking sources non-fatal, before making failed_feed_count fatal for live publication.

Useful? React with 👍 / 👎.

Comment on lines +145 to +148
git config --local http.https://github.com/.extraheader "AUTHORIZATION: bearer ${GITHUB_TOKEN}"
cleanup_git_auth() { git config --local --unset-all http.https://github.com/.extraheader >/dev/null 2>&1 || true; }
trap cleanup_git_auth EXIT
git push origin HEAD:refs/heads/main

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Use HTTPS git credentials that pushes accept

When COMMIT_OUTPUTS=true and there are live-output changes, this push now depends on a manually added Authorization: bearer header after checkout credentials were disabled with persist-credentials: false; GitHub’s HTTPS Git docs describe authenticating Git operations by using the token as the password, and the actions/checkout README’s push example relies on its persisted credentials, so this custom Bearer header can leave scheduled live publication unable to push even after a complete fetch. Use checkout’s persisted token or a Basic/credential-helper based Git credential for this scoped push.

Useful? React with 👍 / 👎.

@Pigbibi

Pigbibi commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

PERMANENT_FREEZE/CLOSE_RESLICE: terminal CRITICAL/HIGH review failure after absolute-final closure. Preserve branch, commits, reviews, checks, and tests; no further code action.

@Pigbibi Pigbibi closed this Jul 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant