Add deterministic weekly workflow boundary - #36
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: Current finding remains valid. The PR still allows any already-completed manual week regardless of age: 🚫 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: 9137cd7f02
ℹ️ 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".
|
|
||
|
|
||
| def _status(value: object) -> WeeklyFeedStatus: | ||
| if not isinstance(value, Mapping) or any(key not in value for key in ("feed_count", "successful_feed_count", "failed_feed_count", "complete")): |
There was a problem hiding this comment.
The workflow passes data/output/rss_source_pipeline/source_fetch_status.json from fetch_rss_sources.py, whose write_fetch_status() only emits generated_at, feed counts, item_count, and feeds; it never writes complete. With a fully successful fetch this check therefore raises feed_status_invalid, so every Build completed weekly producer artifact step exits before uploading political-event-weekly-v1 unless the status writer is updated or this adapter derives complete from the existing counts.
Useful? React with 👍 / 👎.
| if start.weekday() != 0 or end != expected_end - timedelta(days=1): | ||
| raise _invalid("manual_period_mismatch") | ||
| return start, end |
There was a problem hiding this comment.
validate_manual_period() only checks Monday/Sunday shape, so a manual dispatch for the current or a future ISO week (for example period_start=2026-07-13, as_of=2026-07-19 on July 15, 2026) passes the pre-side-effect guard. build_weekly_artifact() later rejects the same period as generated_at_invalid because generated_at is before period_end_exclusive, but that happens after RSS fetch and rss-source-pipeline upload (and live publication when commit_outputs=true), defeating the fail-fast boundary for incomplete weeks.
Useful? React with 👍 / 👎.
| raise _invalid("generated_at_invalid") | ||
| if source_provenance != "official_rss_source_pipeline_v1" or run_mode not in {"scheduled", "manual"}: | ||
| raise _invalid("producer_contract_invalid") | ||
| raw_digest = _snapshot_digest(source_events, watchlist) |
There was a problem hiding this comment.
When the RSS snapshot contains rows outside the requested week, political_events.csv is filtered before being emitted but source_snapshot_digest is computed over the unfiltered input bytes. The five-file weekly artifact therefore does not contain the bytes needed to recompute this digest, and parse_weekly_artifact() only checks the per-file hashes, so a rewritten lock/manifest can carry an arbitrary source_snapshot_digest while still parsing successfully. For a digest-bound artifact, compute the aggregate over the emitted files or include the raw snapshot being digested.
Useful? React with 👍 / 👎.
| source_events=args.source_events.read_bytes(), | ||
| watchlist=args.watchlist.read_bytes(), | ||
| feed_status=feed_status, | ||
| source_provenance="official_rss_source_pipeline_v1", |
There was a problem hiding this comment.
workflow_dispatch still lets operators point feeds_path, aliases_path, and watchlist_path at alternate CSVs, but the weekly builder always stamps the artifact with official_rss_source_pipeline_v1 and the five files do not record the config/feed identity. If a manual test uses a reduced feed list or alternate aliases/watchlist, the upload has the same political-event-weekly-v1 name and official provenance as the default scheduled artifact, so downstream consumers cannot tell that the feed universe was changed; either gate the weekly artifact to default inputs or bind the config paths/hashes/feed IDs in the contract.
Useful? React with 👍 / 👎.
Co-Authored-By: Codex <noreply@openai.com>
|
PERMANENT_FREEZE/CLOSE_RESLICE: repeated terminal HIGH after absolute-final closure. Preserve branch, commits, reviews, checks, and tests; no further code action. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 935309f235
ℹ️ 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".
| ref: ${{ github.sha }} | ||
| persist-credentials: false |
There was a problem hiding this comment.
When COMMIT_OUTPUTS is true (scheduled runs and manual publish runs), the later publish step commits and invokes plain git push. This checkout is no longer pushable: ref: ${{ github.sha }} checks out a commit instead of the main branch, and persist-credentials: false opts out of the checkout token that actions/checkout says enables authenticated git commands (README); as soon as generated CSVs change, the push fails before either artifact upload runs. Check out a branch/push an explicit ref with token auth before keeping this hardening.
Useful? React with 👍 / 👎.
| raise _invalid("manual_period_invalid") from None | ||
| if start.weekday() != 0 or end != expected_end - timedelta(days=1): | ||
| raise _invalid("manual_period_mismatch") | ||
| if run_created_at is not None and (type(run_created_at) is not datetime or run_created_at.tzinfo != timezone.utc or expected_end > run_created_at.date()): |
There was a problem hiding this comment.
When workflow_dispatch is used for any older completed ISO week, this guard accepts it even though the workflow always fetches the current RSS feeds rather than a historical snapshot for that week. A July manual run for a January period can therefore emit a complete-looking weekly artifact with zero or partial January events, which can silently corrupt downstream experiments; restrict manual periods to the immediately previous completed week or require an explicit historical source snapshot.
Useful? React with 👍 / 👎.
| if args.run_attempt != 1: | ||
| raise WorkflowBoundaryError("run_attempt_invalid") |
There was a problem hiding this comment.
On a GitHub re-run, GITHUB_RUN_ATTEMPT increments while GITHUB_RUN_ID remains the same, so this guard exits before rebuilding the weekly artifact for attempts 2+. If the scheduled run fails after fetching or during artifact upload, operators cannot recover the same locked weekly production run with GitHub's normal re-run flow; allow reruns while binding the lock to attempt 1 evidence instead of rejecting the current attempt.
Useful? React with 👍 / 👎.
1 participant
Summary
Fresh reslice from
origin/mainimplementing the producer-side weekly workflow boundary. This does not reuse PR #35.event=schedule,run_attempt=1, and full producer SHA. The APIcreated_atdate determines the previous complete ISO week; runnerdate -uis not period authority.actions:readto existing permissions; no id-token, secrets, or new external store.rss-source-pipelineupload before dedicated weekly build, so dedicated fail-closed does not hide legacy/source evidence.political-event-weekly-v1five-file/30-day/feed-complete/digest-bound contract.Scope boundaries
No QAR consumer, Pages, publisher, monthly, identity/store/registry, migration/compatibility, trading, position, order, broker, funds, or leverage changes.
Validation
python3 -m pytest -q— 132 passeduv run pytest -q— 132 passedpython3 -m compileall -q src scripts tests— passedactionlint .github/workflows/rss_source_pipeline.yml— passedgit diff --check— passedruff— unavailable locallyExternal workflow execution is intentionally not triggered by this worker.