Add concrete weekly producer artifact - #35
Conversation
Co-Authored-By: Codex <noreply@openai.com>
🤖 Codex PR Review🚫 Merge blocked: 2 serious issue(s) found in high-risk files 🚫 Blocking IssuesThese issues must be fixed before this PR can be merged: 1. 🟠 [HIGH] Logic in
|
![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: d828439af7
ℹ️ 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".
| raise _invalid("weekly_artifact_invalid") from None | ||
| files: dict[str, bytes] = { | ||
| PERIOD_LOCK_NAME: period_lock, | ||
| EVENTS_NAME: source_events, |
There was a problem hiding this comment.
For scheduled/manual builds where the fetched RSS output contains items outside period_start through as_of, this copies the entire upstream source_events.csv into political_events.csv. The workflow passes the raw extract_source_mentions output directly, and _csv_snapshot only checks shape, so a Monday run can publish current-Monday or older feed entries while the manifest declares the previous complete ISO week. This can corrupt downstream weekly analyses; filter or reject rows whose event_date is outside the locked period before hashing and writing the artifact.
Useful? React with 👍 / 👎.
| raise _invalid(code) from None | ||
| if not rows or tuple(rows[0]) != expected_header: | ||
| raise _invalid(f"{code}_header") | ||
| if len(rows) < 2 or any(len(row) != len(expected_header) for row in rows[1:]): |
There was a problem hiding this comment.
When RSS fetching succeeds but no fetched item matches any aliases, extract_source_mentions writes a header-only source_events.csv; this check treats that valid no-event snapshot as events_csv_invalid_rows, so the weekly artifact fails to upload for otherwise complete weeks with zero political events. The manifest already records row counts, so the events CSV should be allowed to have zero data rows while still enforcing the header.
Useful? React with 👍 / 👎.
| test -n "${PERIOD_START}" && test -n "${AS_OF}" | ||
| ARGS+=(--period-start "${PERIOD_START}" --as-of "${AS_OF}" --run-mode manual) | ||
| fi | ||
| python scripts/build_weekly_artifact.py "${ARGS[@]}" |
There was a problem hiding this comment.
When any configured feed fails, the fetch step still succeeds because this workflow invokes it with --continue-on-feed-error, but build_weekly_artifact rejects the resulting nonzero failed_feed_count; since this new step runs before Publish live CSV outputs, the job exits here and stops the existing live CSV commit and RSS artifact upload as well. This changes the documented RSS behavior from publishing partial-but-marked live outputs to publishing nothing whenever one feed is down, so isolate the dedicated weekly artifact failure from the legacy live-output path.
Useful? React with 👍 / 👎.
| value["failed_feed_count"], | ||
| value.get("stale_feed_count", 0), | ||
| value.get("missing_feed_count", 0), | ||
| True, |
There was a problem hiding this comment.
If the producer status file marks the snapshot as incomplete with complete: false while the numeric counters are otherwise zero-failure, this hardcodes complete back to True and publishes a manifest claiming the feeds were complete. That bypasses the fail-closed feed-health signal already modeled by WeeklyFeedStatus, so pass through and validate the incoming complete value instead of overwriting it.
Useful? React with 👍 / 👎.
Co-Authored-By: Codex <noreply@openai.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 33fe72d291
ℹ️ 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".
| test -n "${PERIOD_START}" && test -n "${AS_OF}" | ||
| ARGS+=(--period-start "${PERIOD_START}" --as-of "${AS_OF}" --run-mode manual) | ||
| fi | ||
| python scripts/build_weekly_artifact.py "${ARGS[@]}" |
There was a problem hiding this comment.
In the current ordering I checked, the weekly builder still runs before Upload RSS source artifact; when any feed fails, the fetch step continues because of --continue-on-feed-error, but build_weekly_artifact rejects the incomplete status and this command exits before the legacy rss-source-pipeline artifact is uploaded for operators to inspect. Fresh evidence versus the prior comment is that live publication was moved earlier, but the RSS source artifact upload remains behind the weekly failure boundary.
Useful? React with 👍 / 👎.
| --output-dir data/output/political-event-weekly-v1 | ||
| ) | ||
| if [ "${GITHUB_EVENT_NAME}" = "schedule" ]; then | ||
| ARGS+=(--scheduled-today "$(date -u +%F)" --run-mode scheduled) |
There was a problem hiding this comment.
If the Monday cron run is queued long enough to start after 00:00 UTC on Tuesday, this passes Tuesday's wall-clock date to completed_week_period, whose helper rejects any non-Monday with scheduled_period_invalid; the scheduled run would then produce no weekly artifact even though the intended completed ISO week is still unambiguous. Compute the previous completed week from the actual start date without requiring it to still be Monday, or pass an explicit scheduled period.
Useful? React with 👍 / 👎.
| source_snapshot_digest, | ||
| source_provenance, | ||
| ( | ||
| SourceSnapshotArtifact(EVENTS_NAME, _sha256(period_events), event_rows), |
There was a problem hiding this comment.
When source_events contains any rows outside the locked week, source_snapshot_digest is computed from the unfiltered CSV, but this source_artifacts entry records the SHA and row count from period_events after filtering. That makes period_lock.json describe two different snapshots, so later retry or consumer code that uses lock.source_artifacts to verify the original source snapshot will compare against the wrong bytes; keep the lock metadata on the raw source snapshot and leave filtered output metadata to the weekly contract/manifest.
Useful? React with 👍 / 👎.
|
PERMANENT_FREEZE/CLOSE_RESLICE: closure exhausted; preserve branch, commits, review threads, checks, and tests as evidence. No further code action in this PR. |
1 participant
Summary
Implements the concrete producer-side
political-event-weekly-v1artifact for the RSS source pipeline.period_start/as_offor manual runs and fails closed on reruns (run_attempt != 1).period_lock.json,political_events.csv,political_watchlist.csv,political_event_weekly.json, andweekly_manifest.json.This is producer-side only. QAR consumer integration, Pages/publisher, monthly, legacy/compatibility/migration, identity/store/registry, and production permissions are out of scope.
Validation
python3 -m pytest -q— 127 passeduv sync --locked --extra test && uv run pytest -q— 127 passedpython3 -m compileall -q src scripts tests— passedactionlint .github/workflows/rss_source_pipeline.yml— passedgit diff --check— passedruff— unavailable in the local environmentContract gate
The dedicated artifact is only emitted after the pure builder/readback validator succeeds. Existing broad
rss-source-pipelineupload remains separate and does not include the dedicated artifact directory.