Skip to content

Add compatible strict source freshness evidence#53

Closed
Pigbibi wants to merge 2 commits into
mainfrom
feat/pert-source-freshness-evidence-compat
Closed

Add compatible strict source freshness evidence#53
Pigbibi wants to merge 2 commits into
mainfrom
feat/pert-source-freshness-evidence-compat

Conversation

@Pigbibi

@Pigbibi Pigbibi commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Scope

Fresh producer fetch/freshness boundary reslice from latest main; no reuse of frozen PR #52.

  • Preserves legacy fetch_url(url) -> bytes as body-only and does not inspect freshness headers.
  • Adds opt-in fetch_url_with_metadata for Date/Last-Modified, rejecting duplicate headers.
  • Strict per-kind parsing: Atom updated uses timezone-qualified RFC3339; HTTP Date/Last-Modified use IMF-fixdate; RSS lastBuildDate uses timezone-qualified RFC822/2822. No generic fromisoformat fallback, naive, date-only, ISO HTTP, or obs-date acceptance.
  • Present-empty/duplicate high-priority signals fail closed without fallback.
  • Canonical evidence read is bounded to 64 KiB before decode/JSON; existing priority, 8-day, 5-minute, digest, and tamper contracts remain.
  • No workflow, weekly artifact, QAR, permissions, network behavior, or source-config changes.

Validation

  • focused: 27 passed
  • full: 142 passed
  • compileall and diff-check: passed
  • ruff: unavailable in environment

Review mode: preflight then one concentrated review; no merge/rerun/poll requested.

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

github-actions Bot commented Jul 16, 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] Logic in src/political_event_tracking_research/source_freshness.py

The strict HTTP/RSS date parsers do not verify that the weekday token matches the calendar date. datetime.strptime() in _http_date() and email.utils.parsedate_to_datetime() in _rfc2822() will happily parse strings like Mon, 12 Jul 2026 ... even though July 12, 2026 is a Sunday, so malformed producer timestamps can be accepted as valid freshness evidence and drive an incorrect eligible decision. (line 71)

Suggestion: After parsing, re-render the timestamp into the canonical wire format (or compare the computed weekday explicitly) and require it to exactly match the original input before treating the signal as valid.

ℹ️ Other Findings

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

fetch_url_with_metadata() only translates OSError, TimeoutError, TypeError, ValueError, and URLError. A truncated HTTP response can raise http.client.IncompleteRead (or another HTTPException) from response.read(), which will leak out as a raw exception instead of the sanitized feed_fetch_failed error. That breaks the stated fail-closed API contract and can crash callers that only handle FeedXmlError/ValueError. (line 121)

Suggestion: Catch http.client.IncompleteRead or http.client.HTTPException in the same translation block and convert it to FeedXmlError("feed_fetch_failed").


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: 2f6ca8418e

ℹ️ 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 +27 to +29
_RFC2822_RE = re.compile(
r"^(?:Mon|Tue|Wed|Thu|Fri|Sat|Sun), \d{1,2} (?:Jan|Feb|Mar|Apr|May|Jun|Jul|Aug|Sep|Oct|Nov|Dec) "
r"\d{4} \d{2}:\d{2}:\d{2} (?:[+-]\d{4}|UT|GMT|EST|EDT|CST|CDT|MST|MDT|PST|PDT)$"

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 Allow weekday-less RFC 2822 RSS dates

For RSS feeds whose lastBuildDate is a valid RFC 822/2822 timestamp without the optional weekday, such as 12 Jul 2026 11:00:00 GMT, this regex marks the signal invalid because it requires a Mon,/Tue, prefix. build_freshness_evidence() then fails any present-but-invalid RSS freshness signal before considering Last-Modified, so those feeds cannot produce freshness evidence even though the producer supplied a standards-compliant timezone-qualified date.

Useful? React with 👍 / 👎.

if type(value) is not str or not _IMF_RE.fullmatch(value):
return False, None
try:
parsed = dt.datetime.strptime(value, "%a, %d %b %Y %H:%M:%S GMT").replace(tzinfo=dt.UTC)

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 Reject IMF-fixdate headers with mismatched weekdays

When Last-Modified is the first available freshness signal and its weekday does not match the calendar date, such as Mon, 12 Jul 2026 11:00:00 GMT, strptime still returns July 12 and this code treats the header as valid. That lets malformed HTTP freshness metadata mark a feed eligible despite the strict IMF-fixdate contract; compare the parsed weekday back to the header before returning True.

Useful? React with 👍 / 👎.

@Pigbibi

Pigbibi commented Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

PERMANENT_FREEZE: the final closure is exhausted and strict provider-date parsing remains underspecified (weekday consistency and RSS optional-weekday grammar). Preserving this PR as evidence. No second closure, rerun, thread bypass, or merge; provider-content freshness will be re-arbitrated before any successor.

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