Skip to content

Eval: ingest email items at their report date, not the wall clock (#346) - #347

Merged
gapilongo merged 1 commit into
mainfrom
fix/phishing-cassette-clock
Nov 9, 2026
Merged

gapilongo merged 1 commit into
mainfrom
fix/phishing-cassette-clock

Conversation

@gapilongo

Copy link
Copy Markdown
Owner

Closes #346

Root cause

The evaluation runner pushed each email item through IngestionService.ingest_email, which stamped it with datetime.now(UTC) (src/lg_sotf/ingestion/service.py:296). That time becomes the alert's occurred_at. Triage's rule score then adds 5 to an alert that occurs outside 06:00-18:00 UTC (src/lg_sotf/agents/triage/base.py:384-386, _score).

The phishing_v1 cassettes were recorded at 08:23 UTC. After 18:00 UTC, phish-003, -004, -006 and -008 built a triage prompt with rule_assessment.confidence 100 instead of 95, so the input hash changed and the replay raised CassetteMiss. phish-001, -002, -005 and -007 were already capped at 100, so they still matched.

I confirmed this by diffing the dumped triage prompts with the clock shifted back to the recording time against the prompts built now. The confidence field was the only difference.

Fix

  • ingest_email takes an optional received_at. It still defaults to now, so production behaviour is unchanged.
  • EvalRunner._ingest passes email_received_at(mime): the report's top-level Date header in UTC, or a fixed weekday noon (EMAIL_FALLBACK_RECEIVED_AT) when the header is missing or unparseable. It never uses the wall clock.
  • The committed cassettes replay unchanged, so I did not re-record anything.

Tests

  • tests/integration/eval/test_cassettes.py::test_the_phishing_cassettes_replay_at_any_hour_of_the_wall_clock replays the phishing_v1 cassettes with the ingestion clock frozen at 03:30 and at 21:00 UTC. Both times are after hours. Without the fix, both cases fail.
  • tests/unit/eval/test_email_received_at.py covers the Date parsing, the fallback, and the runner passing the report time to ingest_email.
  • make lint, make typecheck, lint-imports and scripts/changelog.py check pass.
  • pytest tests/unit tests/deploy tests/invariants tests/integration/eval: 5114 passed, 67 skipped.

Residual

The email normalizer still clamps an occurred_at that is more than 15 minutes in the future against the wall clock. A dataset dated in the future would therefore still depend on when it runs. No current dataset is.

🤖 Generated with Claude Code

https://claude.ai/code/session_01JmdYVXkgpNHoygVJCrUeZc

The runner pushed each email item through ingest_email, which stamped it with datetime.now(UTC). That time became the alert's occurred_at, and triage's rule score adds 5 to an alert outside 06:00-18:00 UTC (agents/triage/base.py _score). The phishing_v1 cassettes were recorded at 08:23 UTC, so after 18:00 UTC phish-003/-004/-006/-008 built a triage prompt with rule confidence 100 instead of 95 and missed. (The other four were already capped at 100.)

ingest_email takes an optional received_at; the runner passes the report's top-level Date header, or a fixed noon when there is none. Production still uses the arrival time. The committed cassettes replay unchanged. A new integration test replays them with the ingestion clock frozen at 03:30 and 21:00 UTC; it fails without the fix.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JmdYVXkgpNHoygVJCrUeZc
@claude

claude Bot commented Nov 9, 2026

Copy link
Copy Markdown

Review

The fix looks right. Production behaviour is unchanged because received_at defaults to now. The eval runner now derives the time from the report, so replays no longer depend on the hour they run. The root-cause write-up is clear, and the tests cover both after-hours sides of the 06:00–18:00 window.

I didn't run the tests myself, and the repo has no top-level CLAUDE.md, so I judged style from the surrounding code.

Suggestions (non-blocking)

  1. Future-dated Date headers. You already list this under Residual. The wall clock can still leak in through the normalizer's 15-minute future clamp. A cheap guard would be for email_received_at or the dataset loader to reject or warn on a future Date, so the dependence is not silent. You could also add a unit test that pins today's behaviour.
  2. Fixed fallback date. EMAIL_FALLBACK_RECEIVED_AT is 2026-01-05 and is in the past. If the normalizer ever adds a maximum-age check, the fallback would break. A short comment, or a test showing a fallback item ingests successfully, would protect that.
  3. Frozen-clock test. The test only patches ingestion_service.datetime. If another module (the normalizer or triage) reads the wall clock for scoring, the test would not exercise it. Consider also asserting that the occurred_at fed to triage equals the Date header, which would be a more direct regression check than CassetteMiss absence. The FrozenClock subclass is also somewhat heavy. If the repo already has freezegun or a clock helper, use that.
  4. Hour dependence in the test. tenant_id=f"clock-{frozen:%d%H}" is fine. It would still be worth confirming the test fails without the fix, as the PR description says, which a short CI note could document.
  5. Minor. received_at or datetime.now(UTC) is fine for datetimes. received_at if received_at is not None else ... is more explicit and avoids truthiness surprises. The except tuple in email_received_at is broad but reasonable given parsedate_to_datetime behaviour across Python versions.

Security and performance

  • No concerns. Header parsing only touches headers (HeaderParser), and the Date value is untrusted dataset input that is only parsed, never executed.
  • The runner is a test and eval path, and production ingestion still uses server time. A client-supplied received_at is not exposed on any API.

Test coverage
Good coverage of the Date parsing, the naive -0000 zone, the fallback cases, the runner wiring, and the cassette replay at two hours. A test for a timezone-offset date that crosses the 06:00 or 18:00 boundary after conversion to UTC would be a nice addition.

Approving in spirit. The suggestions are optional.

🤖 Generated with Claude Code

@gapilongo
gapilongo merged commit 4998e5a into main Nov 9, 2026
16 of 17 checks passed
@gapilongo
gapilongo deleted the fix/phishing-cassette-clock branch November 9, 2026 19:02
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.

[BUG] phishing_v1 cassette replay is time-dependent: 4 items miss after some hours

1 participant