Grade the Coach turn that went looking and found nothing - #825
Merged
Conversation
Three characterisation tests for grounding gaps found while verifying that
every figure the Coach states comes from the record.
The numeric grounding guard rewrites any figure the turn's Grounding Ledger
cannot account for, and it holds: removing the strip reddens the existing
cross-turn ledger test. What is not covered is its activation. On the tool
path the guard runs only when `loop.toolResults` is non-empty, and a tool that
found nothing resolves to `{ present: false, reason: "none" }` with no
`available` payload, which the loop drops. So the guard disarms itself exactly
when the record is empty: ask about a metric with no readings, and an invented
figure is persisted and streamed verbatim, with no withheld-figure marker to
tell the reader it was never checked. The no-tools snapshot fallback is
populated only in the non-tool-mode branch, so the tool path has nothing to
fall back to, and every cloud provider runs the tool path.
Separately, a rollup bucket outlives the rows it was folded from. The read-swap
falls back to live SQL only on an empty band, `ensureUserRollupsFresh` repairs
the DAY tier over the trailing 90 days only, and it keys off
`Measurement.updatedAt`, which a hard delete does not bump. The integration
test reproduces the hard `deleteMany` in the WHOOP body sync against a
MONTH-band bucket: the deleted value keeps reaching the Coach's coarse tail
while the all-time extremes beside it report only the surviving value, and the
block is stamped current because `asOf` is derived from raw reading age and
knows nothing about rollup age.
All three assert current behaviour and carry a docblock saying so. Closing
either gap will redden them, which is the point.
The numeric grounding guard keyed its activation off `loop.toolResults` alone.
A tool that finds nothing resolves to `{ present: false, reason: "none" }` with
no `available` payload, `runCoachToolLoop` drops that shape, and the no-tools
snapshot fallback is populated only in the non-tool branch — so the guard sat
out every turn where the record was empty, which is precisely the turn where a
fabricated figure is likeliest and does the most harm. Ask about a metric you
have never logged and an invented number was persisted and streamed verbatim,
with no withheld-figure marker to say it had never been checked.
`toolTrace` is the discriminant, because it records the pure misses the results
array drops. Tools ran and every one missed: grade the reply — an empty ledger
there is not missing information, it is the positive statement that nothing can
be cited, so `findUnverifiedCoachNumbersInLedger` takes an explicit opt-in to
grade against it rather than no-opping. The model answered without calling a
tool: still dormant, unchanged, because the tool-mode base prompt carries no
pre-computed figures and flagging a legitimately recalled one was the v1.32.1
regression. The inventory counts ride along as a widener so a plain count
restatement stays grounded.
Eliding the digits is not enough on an empty record. "Your sleep averaged […]
minutes, up from […]" still asserts a series, an average and a direction for a
metric with no readings; only the precision goes, the false claim stays. So a
turn that missed every tool and lost a figure to the guard is replaced with the
honest answer, in all six locales. Scoped so a reply whose figure reconciles
against a prior turn or an inventory count is left alone.
A rollup band outlives the rows it was folded from. The read-swap falls back to live SQL only when a band is EMPTY, so a bucket that is merely wrong is served as current; `ensureUserRollupsFresh` repairs the DAY tier over the trailing 90 days only, and keys off `Measurement.updatedAt`, which a hard delete does not bump. The WHOOP body sync hard-deletes a WEIGHT row with no invalidation, so a MONTH bucket keeps reporting a value the record no longer holds. The block was handing the model two irreconcilable accounts of the same record and marking neither: the coarse tail said one thing, the all-time extremes read live beside it said another, and `asOf` stamped the whole thing current because it is derived from raw reading age and knows nothing about rollup age. Reconcile them where they are assembled. A bucket mean is an average of real rows, so it cannot fall outside those rows' own all-time extremes — when it does, the band is describing readings the live read cannot see. That is an invariant rather than a heuristic, so the disputed tail is dropped and `asOf.coarseHistoryWithheld` records that it went. Narrating a ghost is worse than narrating less. `currentForTodayClaims` stays keyed to raw recency, which is what it means. The surviving readings really are from today, and suppressing a true present-tense statement to punish a stale history band would trade one dishonesty for another. The bucket on disk is still wrong, and a wide event now says so. The deeper fix is to make the tier notice — compare a bucket's `computedAt` against the type's in-window MAX(`updatedAt`), or invalidate on the hard-delete path — which is a larger change than this repair.
Three places where the copy promised more than the handler delivers. The hero's personal-baseline line rendered unconditionally while both its siblings in the same meta row gate on their own data. It claims the subtitle above it was drawn from the reader's own last 90 days, but with no briefing the subtitle is fixed fallback copy computed from nothing — so an account with no briefing at all was told its generic sentence rested on a personal baseline. Gate it on the briefing, like the rest of the row. The pulse sub-page promised a personalised Karvonen target band. Karvonen is not computed anywhere: the band comes from CDC/NCHS population percentiles by age and sex, and falls back to a fixed 60-100 for a reader with no age on file. The copy now says that in all six locales, and the fallback's own source label says it is a general adult range rather than a personal one, because that arm is the same band for everyone. Karvonen itself stays unimplemented; whether to build it is a product decision, not a copy fix. The Coach's `allTime` window returns 365 days, identical to `lastYear`, and it is the default. "year so far" was wrong too — a rolling 365 days is not year-to-date. Both labels now say what the reads do. The two options remain functionally identical and one of them should probably go, but removing a default is a product decision.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The Coach checks its own numbers against a ledger of what it retrieved, and rewrites any figure the ledger cannot account for. That check does not run when every retrieval tool comes back empty, which is the case where an invented figure is both likeliest and most harmful.
What was proven, and how
A tool that finds nothing resolves to
{ present: false, reason: "none" }with no payload, and the tool loop drops that shape. So an all-missed turn leavestoolResultsempty, the guard's activation condition fails, and the reply is streamed and persisted unedited. The quiet "some figures could not be checked" marker is absent too, so the answer is indistinguishable from a verified one.Three route-level tests establish it. With one present tool result,
"Your sleep averaged 432 minutes last week."persists as"Your sleep averaged […] minutes last week."withunverifiedFigures: 1. With every tool missed, the same reply persists verbatim andunverifiedFiguresis undefined.The guard itself is sound and was break-proofed before anything changed: replacing its assignment with a no-op reddens the ledger test on the figure it should have removed. The defect is the activation condition, not the enforcement.
The fix
toolTraceis the discriminant, because it records the pure misses thattoolResultsdrops. Tools ran and all missed, so the turn is graded. The model called no tool at all, so the check stays dormant and the v1.32.1 contract is untouched.Two gates had to move. Widening the route was not enough: the ledger walker short-circuits on an empty ledger, and on an all-missed turn the ledger often is empty. It now takes an explicit opt-in that only the route passes, because there an empty ledger is not missing information, it is the positive statement that nothing can be cited.
Eliding the digits was not enough either. "Your sleep averaged […] minutes, up from […]" still asserts a series, an average and a direction for a metric with no readings. Only the precision goes; the false claim stays. So a turn that missed every tool and lost a figure is replaced with an honest "nothing recorded" message, in all six locales, following the existing REPLACE policy of the outbound screen. It is scoped so a figure that reconciles against a prior turn or an inventory count is left alone, and there is a test for exactly that. The accepted trade-off is written at the site: a reply mixing one good recall with one fabrication is replaced whole.
Five mutations, each reverted after: reverting the activation, dropping the opt-in at the call site, restoring the short-circuit, removing the honest replacement, and over-widening to no-tool turns. All five red.
A rollup bucket that outlived its rows
Proven separately against a real database. The hard delete that runs on every WHOOP body sync removes measurement rows without touching the rollups folded from them, and the read-swap falls back to live SQL only when a band is empty. The result reaches the Coach as one block holding two accounts of the same record: a monthly timeline at 95 kg beside all-time extremes of 80, with a freshness stamp saying today.
A bucket mean is an average of real rows, so it cannot sit outside those rows' own extremes. That is an invariant, not a heuristic. When it does, the coarse band is dropped and the snapshot says it was withheld, with a wide event because the row is still wrong on disk.
currentForTodayClaimsis deliberately left true. It means the freshest reading is recent enough for present tense, and it is: the surviving rows are from today. Suppressing a true statement to punish a stale history band would not reliably have stopped the trend claim that was the actual harm. Removing the disputed band removes the harm at its source.Three promises the code did not keep
The Insights hero rendered "Based on your last 90 days" unconditionally while both its siblings were gated, so it painted even with no briefing behind it. The existing test rendered a null briefing and expected the line, which pinned the defect; it is inverted with the reason beside it.
The pulse page promised a "personalised Karvonen target band". There is no Karvonen computation anywhere: the code returns CDC/NCHS age and sex percentiles, or a flat 60 to 100 whenever age is unknown. The copy now says what it computes in all six locales, three of which were carrying untranslated English, and the flat arm labels itself a general adult range rather than a personal one. Karvonen stays unimplemented; the only mention left in the tree is a comment explaining why percent of maximum is used instead.
The Coach window labelled "all time" returns 365 days, identical to the option labelled "year so far", which is itself a rolling year. Both labels now say what they compute.
One decision left open
allTimeandlastYearare byte-identical in behaviour. The labels are honest now, but it is a default option that means nothing distinct. Removing it is a product decision rather than a fix, so it stays.Not in this branch
Six hypotheses that could not be demonstrated are written up out of the tracked tree rather than acted on, each with the run that would settle it. Among them: whether the recovery-score attribution matches its copy when a provider-native row wins, and whether a correlation chip labelled thirty days is served a twenty-eight-day window.