fix(news): guard the panel signals projection#120
Conversation
`get_latest_panel_payload` returned the wire's `signals` dict raw. That made it the one panel path with no projection, and so the one path `_alarm_if_all_dropped` was blind to — it can only see what a projection dropped. Nothing read `sentiment_score` before the browser did, where `Number(undefined).toFixed(2)` renders the string "NaN": a rename would have painted "NVDA NaN" into every chip with no drop, no exception, no badge and no log. The score -> sentiment_score rename passed within inches of this. Project `signals` to the keys the panel actually renders (sentiment, sentiment_score, url, source), drop what it can't, and run the existing alarm over it. Independent of the feed projection: a story with no sentiment read is still a story, so coupling them would let one break take out both. The alarm is appended AFTER the feed block, not folded in above: the items alarm assigns `drifted` outright rather than OR-ing it, so seeding it earlier is clobbered whenever the items feed loads — i.e. always, in prod. The guard would read correctly and fire never. test_panel_signals_drift_survives_a_ healthy_items_feed pins exactly that; the other drift tests leave `_http_get_items` unpatched and cannot see it. Behavior change: an entry missing a rendering key is now dropped rather than rendered broken, and an all-dropped signals list escalates to `degraded`. Suite 978 passed / 4 skipped. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pbs5wnQZhTi9CQ3pb14vDo
Self-review with mutation testing: dropping `sentiment`, `url` or `source` from _PANEL_SIGNAL_KEYS left the whole suite green — only `sentiment_score` was actually pinned. A guard against renamed fields whose own key set is unpinned against renaming is the same bug one level up. One failing case per key now. The isinstance branch was likewise untested, and the obvious test for it is vacuous: a `str` entry substring-matches (`"sentiment" not in "not-a-dict"` is just True), so it drops via the missing-key branch and passes with the guard deleted. Parametrized on None/42, which actually reach the TypeError. Also widen _alarm_if_all_dropped's `projected` hint to Union[list, dict] — the new call site passes a dict; only len()/truthiness are used, so this was cosmetic, but the hint said otherwise. Suite 984 passed / 4 skipped. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pbs5wnQZhTi9CQ3pb14vDo
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note on CI: Run locally against this branch in the meantime: |
The items alarm assigned `drifted` while the other two OR-ed it, so the panel-signals alarm had to be appended last and a comment kept it there. That is the guard's own failure mode one level up: read correctly, fire never, held off by a comment and a line number. All three now OR, so each projection sits with its own alarm and order carries no meaning. Behaviour is identical — `drifted` starts False, so the old bare assign and the OR agree. OR alarm-first (`_alarm(...) or drifted`), never `drifted or _alarm(...)`: `or` short-circuits, so the reversed form stops calling later alarms once anything has drifted and swallows the ERROR they exist to emit. It reads identically and passed all 48 tests, so test_every_concurrent_drift_gets_its_own_alarm pins it. Also: state the projection-independence invariant once, at _PANEL_SIGNAL_KEYS where someone would edit it, and drop the incident retelling _alarm_if_all_dropped already carries. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Accumulation made uniform. The items alarm assigned Worth knowing: the obvious tidy-up — Also: independence invariant stated once (at Reviewed and rejected: generalizing the three projection+alarm pairs into one mechanism (three different trigger conditions and raw types — it'd move complexity, not remove it); deriving Follow-ups, not in this PR:
Suite 998 passed / 2 skipped. |
1 participant
Closes #119.
The panel's
signalsdict reached the browser raw from the wire — the one panel path with no projection, and so the one path_alarm_if_all_droppedwas blind to, since it only ever sees what a projection dropped. Nothing server-side readsentiment_score; the browser did, whereNumber(undefined).toFixed(2)renders the string"NaN". A rename would have paintedNVDA NaNinto every chip with no drop, no exception, no badge, no log. The feed keeps building (different keys), so the existing alarm could not fire.Project
signalsto the keys the panel renders, drop what it can't, run the existing alarm over it. Independent of the feed projection: a story with no sentiment read is still a story, so coupling them would let one break take out both.The alarm is appended after the feed block, not folded in above. The items alarm assigns
driftedoutright rather than OR-ing it, so seeding it earlier is clobbered whenever the items feed loads — i.e. always, in prod. The guard would read correctly and fire never.test_panel_signals_drift_survives_a_healthy_items_feedpins that; the other drift tests leave_http_get_itemsunpatched and cannot see it.Behavior change: an entry missing a rendering key is dropped rather than rendered broken, and an all-dropped signals list escalates to
degraded. Verified against the producer that this cannot fire on a legitimate payload:_PANEL_SIGNAL_KEYSis a subset of the vendored v2 schema's per-entryrequiredlist, the producer builds all four unconditionally, and_PUBLIC_STRIPis top-level-only.Second commit is self-review: mutation testing showed 3 of the 4 keys in the new guard were themselves unpinned (dropping
sentiment/url/sourceleft the suite green). Also the obvious non-dict test is vacuous — astrsubstring-matches, so it passes with the guard deleted; parametrized onNone/42instead.Stacked on #117 (same files). Suite 984 passed / 4 skipped.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Pbs5wnQZhTi9CQ3pb14vDo