Skip to content

H2A: add pure feed status decision foundation - #46

Closed
Pigbibi wants to merge 2 commits into
mainfrom
feat/pert-feed-status-decision-h2a
Closed

H2A: add pure feed status decision foundation#46
Pigbibi wants to merge 2 commits into
mainfrom
feat/pert-feed-status-decision-h2a

Conversation

@Pigbibi

@Pigbibi Pigbibi commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Scope

Fresh H2A from latest origin/main@294e30b8eed36f070299b1dc01d34955fb262918; PR #45 is closed/frozen contract-conflict evidence and was not reused.

This is a pure typed decision foundation over already-validated per-feed primitive outcomes:

  • returns canonical StatusEvidence plus explicit ProducerDecision (success, quarantine, hard_fail);
  • all-quarantined and accepted+quarantined return incomplete/non-eligible evidence with a quarantine decision;
  • any failed outcome returns incomplete evidence plus hard-fail decision without raising inside the builder, so callers can write evidence before acting;
  • malformed shape/state/kind/rows/error/digest inputs raise sanitized contract errors; BaseException propagates.

No raw XML/RSS/Atom parsing, I/O writer, workflow, publish, weekly, QAR, permission, or automation changes are included.

Validation

  • focused pytest: 7 passed
  • full pytest: 129 passed
  • compileall: passed
  • git diff --check: passed
  • ruff: unavailable in environment

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: 1 serious issue(s) found in high-risk files

🚫 Blocking Issues

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

1. 🟠 [HIGH] Bug in src/political_event_tracking_research/feed_status_decision.py

StatusEvidence.status is recursively converted to MappingProxyType and tuples by _freeze(). That makes the returned status payload no longer recursively JSON-safe: json.dumps(result.evidence.status) will fail on nested mappingproxy objects, and callers expecting ordinary dict/list wire data will hit a runtime break even though status is advertised as the canonical evidence object. (line 123)

Suggestion: Keep status as plain JSON-native dict/list data and enforce immutability through a separate wrapper or copy-on-read API, or expose only canonical_bytes as the serialized artifact while documenting that status is not the wire payload.

ℹ️ Other Findings

1. 🟡 [MEDIUM] Reliability in src/political_event_tracking_research/feed_status_decision.py

build_status_decision() eagerly materializes any Iterable with list(outcomes) and has no upper bound on feed count. A buggy or unbounded generator can therefore hang or exhaust memory before any sanitized contract error is raised, which defeats the PR’s goal of always being able to build/write evidence before acting on failures. (line 209)

Suggestion: Require a bounded concrete sequence, or iterate incrementally while enforcing a maximum feed count and aborting with a sanitized contract error once the limit is exceeded.


Review by Codex PR Review bot • PR

Co-Authored-By: Codex <noreply@openai.com>
@Pigbibi Pigbibi closed this Jul 15, 2026

@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: 5641e948f3

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

Comment on lines +136 to +137
def _row_key(row: _Row) -> tuple[str, str]:
return row.published_at, row.item_id

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 Make row digests deterministic for tied row keys

When an accepted feed contains two rows with the same published_at and item_id but different content, this sort key leaves their order tied, so Python's stable sort preserves the incoming row order and both row_digest and aggregate_row_digest change if the same rows arrive in a different order. Since this module is producing canonical evidence, include the remaining row fields in the key or reject duplicate row keys before hashing.

Useful? React with 👍 / 👎.

Comment on lines +102 to +105
def _row(value: object) -> _Row:
data = _mapping(value, frozenset(_ROW_KEYS), "row_invalid")
values = [_string(data[key], "row_invalid", allow_empty=key == "author") for key in _ROW_KEYS]
return _Row(*values)

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 Validate accepted row timestamps before success

Accepted rows are accepted after only _string checks, so an outcome with published_at like not-a-date still returns DecisionKind.SUCCESS and eligible_for_live_publication=True. The existing source-items consumer derives event dates from this field and calls parse_date, so a malformed timestamp can be marked live-eligible here and then fail the pipeline when a matching item is extracted; validate the ISO/Z timestamp before counting the row accepted.

Useful? React with 👍 / 👎.

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