feat: add weekly source contract foundation - #25
Conversation
Co-Authored-By: Codex <noreply@openai.com>
🤖 Codex PR Review✅ Merge allowed: blocking findings were cleared by independent Codex arbitration ⚖️ Codex Review Arbitration✅ clear: The cumulative diff demonstrably fixes both prior blockers. In ℹ️ Other Findings1. 🟡 [MEDIUM] Security 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: f0be26b8ec
ℹ️ 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".
| "period_end_exclusive": contract.period_end_exclusive.isoformat(), | ||
| "generated_at": contract.generated_at.isoformat(timespec="microseconds").replace("+00:00", "Z"), | ||
| "run_mode": contract.run_mode, "producer_ref": contract.producer_ref, "source_provenance": contract.source_provenance, | ||
| "source_artifacts": [{"path": item.path, "sha256": item.sha256, "row_count": item.row_count} for item in contract.source_artifacts], |
There was a problem hiding this comment.
When producers build WeeklySourceContract directly with multiple source_artifacts, this preserves the tuple's input order even though parse_weekly_contract normalizes artifacts by path and the contract promises deterministic output. The same logical weekly contract can therefore produce different byte streams depending on artifact discovery order, which breaks downstream digest/signature comparisons; serialize the normalized/sorted artifact list instead of the raw tuple order.
Useful? React with 👍 / 👎.
| "generated_at": contract.generated_at.isoformat(timespec="microseconds").replace("+00:00", "Z"), | ||
| "run_mode": contract.run_mode, "producer_ref": contract.producer_ref, "source_provenance": contract.source_provenance, | ||
| "source_artifacts": [{"path": item.path, "sha256": item.sha256, "row_count": item.row_count} for item in contract.source_artifacts], | ||
| "feed_status": {"feed_count": contract.feed_status.feed_count, "successful_feed_count": contract.feed_status.successful_feed_count, "failed_feed_count": 0, "stale_feed_count": 0, "missing_feed_count": 0, "complete": True}, |
There was a problem hiding this comment.
When a producer constructs WeeklyFeedStatus directly with any nonzero failed/stale/missing count, this payload overwrites those fields with zero before parse_weekly_contract(payload) validates it. If successful_feed_count still equals feed_count, serialization emits a complete-looking contract instead of failing closed for the partial feed state; include the actual counts in the payload and let the parser reject invalid statuses.
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: 315a389a03
ℹ️ 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".
| if ( | ||
| type(path) is not str or not path or not path.isascii() or unicodedata.normalize("NFC", path) != path | ||
| or canonical is None or canonical.is_absolute() or path != str(canonical) or path.endswith("/") | ||
| or "//" in path or any(part in {"", ".", ".."} for part in canonical.parts) or "\\" in path | ||
| ): |
There was a problem hiding this comment.
When an artifact path contains ASCII control characters such as \u0000 or \n, this guard still accepts it: path.isascii() passes, PurePosixPath preserves it as a relative part, and none of the later checks rejects it. A contract with source_artifacts[0].path = "data/live/a\u0000.csv" therefore parses and serializes even though any later digest/row-count step that opens base / artifact.path will fail with an embedded-null ValueError instead of failing closed for an unsafe artifact path.
Useful? React with 👍 / 👎.
|
PERMANENT_FREEZE/CLOSE_RESLICE: actionable path-control-character finding remains after final closure. Preserve branch/worktree/commits/review threads/tests as evidence. |
1 participant
Scope
Pure producer-side
political_event_weekly.v1contract foundation only. No fetcher, workflow, artifact publication, QAR consumer, Pages, publisher, monthly, new data source, credential, or dependency changes.Contract
as_ofis the Sunday date.generated_atis explicit actual UTC build time and must be at/after the exclusive period boundary.schema_version,contract_version, cadence, run mode, producer SHA, provenance, source artifact digests/row counts, and complete feed status are strict and deterministic.Source-of-truth decision: weekly period uses complete UTC ISO week and Sunday
as_of.