Make WardFlowProvider's pinned clock read the instant it was given - #2436
Conversation
… given `initialNow` is documented as pinning the clock at that instant, but its value was discarded: the render body computed `elapsed = 0` when pinned and then `now = NOW_ANCHOR + elapsed + clockOffsetMinutes`, so every pinned provider served NOW_ANCHOR (642, 10:42) whatever it was passed. The prop was only ever a "do not tick" flag plus a seed for a checkpoint the pinned path never reads. The defect was latent, not active: all 40 `initialNow=` call sites in tests pass NOW_ANCHOR, where `NOW_ANCHOR + 0` and `initialNow` agree by coincidence. All 40 remain green. But it made every time-of-day branch unreachable through the real provider, which is what forced the pure-function workaround in Ward Flow Phase 6's morning page (spec D5's pre-08:00 handover branch). Fix: when pinned, `now` derives from `initialNow + clockOffsetMinutes`. The unpinned path is unchanged. Four tests added, three of which fail against the old code with "Expected 450, Received 642". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ranch The fix (62f798c) is proven but lives only on this local branch, and the branch that actually needed it — Phase 6's morning page, on claude/ward-flow-phases-6-7-design — still carries both the defect and the workaround built around it. That branch moved twice during the session that wrote this, so it is live, not idle. The handover records what the defect was, what is proven and by what evidence (including the mutation test's decisive line), the three outstanding items and where they must be done, what must not be re-opened, and the two gate traps this work hit — a lint that exited 0 twice without running, and a fresh worktree's empty node_modules. Adds a SCOPED_ALLOWLIST entry to check-docs-links for the two Phase 6 paths the handover deliberately names, using the gate's own documented mechanism for a document that intentionally cites a path not on this branch. It is scoped to this one document and carries its own removal condition. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… follow-through Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J2yCaWLCy1XZ6vUH1erQwr
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Warning Review limit reachedNext included review available in 15 minutes. View limit detailsLimit details: You’ve used the included review currently available. Your 102 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (5)
Comment |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_739dcfd8-a533-421c-95ff-f87e83e11aed) |
CI triageCI failed on this PR. Automated classification of the 3 failed job(s):
Compared with main CI run #14332 (failure). That run's conclusion is an aggregate and did not exercise Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
Static PR checks failed on `check:outstanding-issues-snapshot`: the committed snapshot recorded `counts.pending: 3` while the ledger plus inbox now hold 4. This branch's queued `add` request is the fourth, so the snapshot went stale the moment that request was committed. Regenerated with `scripts/generate-outstanding-issues-snapshot.mjs` rather than hand-edited, per the generated-file rule. The diff is exactly the count and the one new request record. Regenerated AFTER merging `origin/main`, deliberately: this branch's own ledger was 27 commits behind and reported 98 open items where CI's merge result reported 81, so regenerating on the stale base would have written a snapshot that disagreed with the ledger CI actually evaluates. Local now reports the same 81 open / 431 archived that CI does. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Static PR checks failed on check:repo-awareness-snapshot with "documentation differs from the repository": this branch adds docs/ward-flow-pinned-clock-handover.md, which the committed snapshot's document inventory did not list. Regenerated with npm run snapshot:repo-awareness rather than hand-edited. The diff is exactly the new document entry and the counts it moves (documents 481 -> 482, uncatalogued 374 -> 375), plus captured_revision, which the checker documents as deliberately not compared. Found by running the job's steps locally rather than by another CI round trip. The rest of that job's steps were swept the same way and pass: runtime, ci-scope, verification-plan, outstanding-issues, ledger-write-discipline, branch-review-ledger, type-scale, icon-scale, assets, design-system-contract, owner-scope, migration-role, function-grants, maintainability-budgets, therapy-data-index, cross-mode-index, mha-act-sections, upload-limit-parity, medication-lexicon-report, brand:check, and all four docs checks. check:knip could not be evaluated locally: it dies in oxc-parser with "RangeError: Array buffer allocation failed", an out-of-memory condition on this workstation rather than a finding. This diff cannot affect what knip inspects (dependencies, unlisted, unresolved, duplicates) - it adds a document, a JSON snapshot, a comment-only allowlist entry, and a two-line change inside an existing component. Reported as unverified locally, not as passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_7232d8db-4701-471e-8f72-a50272425050) |
PR #2435 and PR #2436 each queued one inbox request and each bumped counts.pending from 3 to 4. Merged, the true count is 5. Regenerated rather than hand-edited. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JiaghcKco9xE3vLS4PJHdd
# Conflicts: # data/outstanding-issues-snapshot.json
Uh oh!
There was an error while loading. Please reload this page.
Summary
WardFlowProvider's pinned clock actually read the instant it was given. TheinitialNowprop is documented as pinning the clock at that instant, but its value was silentlydiscarded: the render body forced
elapsed = 0wheneverinitialNowwas defined and thencomputed
now = NOW_ANCHOR + elapsed + clockOffsetMinutes, so a pinned provider always servedNOW_ANCHOR(642 — 10:42) whatever instant it was handed. The prop was only ever a "do not tick"flag plus a seed for a clock checkpoint the pinned path never reads again. Now
nowderives frominitialNow + clockOffsetMinuteswhen pinned; the unpinned path, including Phase 3'smidnight-rollover accumulator, is untouched.
three red on
Expected element to have text content: 450 / Received: 642— the provider servingthe hardcoded anchor while the test pinned 07:30. The fourth (a pinned provider never starts its
tick interval) passes either way by design; it guards the fix against costing the prop its other
job, and is not a detector of this bug.
docs/ward-flow-pinned-clock-handover.md. The work this unblocks lives on a differentbranch, so the handover records the defect, the evidence, the outstanding follow-through and
where it must be done, what must not be re-opened, and two gate traps this work hit.
check-docs-linksallowlist entry. The handover deliberately names two Phase 6paths that exist on
claude/ward-flow-phases-6-7-designand not on this branch. This uses thegate's own documented
SCOPED_ALLOWLISTmechanism for a document that intentionally cites a pathit cannot resolve, scoped to that one document, with its own removal condition written in.
add, P2 task) recording the Phase 6follow-through. No canonical ledger edit; reconciliation is a separate serial branch.
The defect was latent, not active. All 40
initialNow=call sites in the suite passNOW_ANCHOR, whereNOW_ANCHOR + 0andinitialNowagree by coincidence, and the live app neverpasses the prop at all. Nothing was rendering a wrong time. What it did cost was testability: no
test could obtain any clock other than the one instant the fixture is authored around, which made
every time-of-day branch unreachable through the real provider. That is what forced a pure-function
workaround in Ward Flow Phase 6's morning page, where spec D5's pre-08:00 "the 08:00 handover has
not been taken for this day" branch is currently proven at the pure-function level only, not through
the rendered page.
Verification
npm run verify:pr-localVerification not run: verify:pr-local— the repository run coordinator was occupied by otherworktrees for the whole session, and the broad wrapper could not be admitted. Every failure class it
would have covered for this diff was proven directly instead, each read from its own output rather
than from an exit code:
tests/ward-flow-provider.dom.test.tsx—Tests 9 passed (9).initialNowcall site (the regression risk): all 17 files that pin theclock —
Test Files 17 passed (17),Tests 92 passed (92). A correct fix had to be a no-op forall 40 call sites, and it was; none was relying on the bug.
Tests 519 passed (519).tests/check-docs-links.test.ts—Tests 2 passed (2).npm run typecheckpassed on the source change. Not re-run for the three latercommits, deliberately:
tsconfig.typecheck.jsonincludes only**/*.ts,**/*.tsx,**/*.mts,so a
.md, a.jsonand a.mjsare invisible to it and the earlier verdict stands unchanged onthis content.
npm run lintpassed repo-wide on the source change;scripts/check-docs-links.mjslinted directly at the pushed content (
--max-warnings 0, exit 0).docs link check passed: 3804 repo path references resolve.check:outstanding-issuespassed (512 rows, 98 open, no ids deleted from base);check-ledger-write-disciplinepassed forbe65b8a1bc00..HEAD.Two CI failures were found and fixed after opening, each diagnosed from its own log rather than
guessed at:
Static PR checks→check:outstanding-issues-snapshot:counts.pending: committed 3 vs regenerated 4. This branch's queued ledger request was the fourth. Regenerated withscripts/generate-outstanding-issues-snapshot.mjs(never hand-edited); the diff is exactly thecount and the one new request record. Now
[snapshot] in step with data/outstanding-issues-snapshot.json (81 open, 4 pending).PR policy: that regeneration putdata/**in the diff, flippingclinicalRisktotrueandmandating the governance section now present below. Re-validated locally against
evaluatePullRequestPolicy—ok: true, no errors, no warnings.npm run verify:uiwhen UI, routing, styling, browser behavior, reduced-motion, or forced-colors behavior changedUI verification not run: no rendered behaviour changes for any reachable route.The provider isreached only from
/mockups/ward-flow, which 404s in production, and the only code path this diffalters is the one taken when
initialNowis supplied — which no production call site does. Thelive app's unpinned path is byte-for-byte unchanged in behaviour. The 519-test ward suite covers the
rendered surfaces.
npm run verify:releasebefore release or handoff confidence claimsNot applicable — no release or handoff confidence is claimed here, and it is provider-backed.
Two verification traps worth recording, both of which reported success while doing nothing:
npm run lintexited0twice without running — once holding off for another worktree'sheavy-run lease (
DATABASE_HEAVY_RUN_ADMISSION_BUSY), once on a WindowsEPERMcreating the lockdirectory. A third attempt genuinely ran and passed. Piping a gate through
tailmasks its realexit code, so both looked green.
output. That gap was closed manually against the pushed content rather than left standing.
Risk and rollout
production call site takes. The blast radius was measured before the change (40 call sites, all
passing
NOW_ANCHOR) and re-verified after (all 40 green). The remaining files are a document, aqueued ledger request, one scoped allowlist entry in a docs gate, and a regenerated ledger-count
snapshot.
62f798c2aalone for the behaviour change, or the whole branch for all of it. Nothingdepends on it yet — the Phase 6 branch that will consume it has not taken it.
access at any point; every check above is local and offline.
classifyPullRequestFilesreportsragRanking: falseandoperationalRisk: false. It reportsclinicalRisk: true, but solely on thedata/**path of the regenerated issue-ledger snapshot; the governance section below addressesthat on its merits.
Clinical Governance Preflight
This section is present because regenerating
data/outstanding-issues-snapshot.json— required toclear the earlier
Static PR checksfailure — puts adata/**path in the diff, andclassifyPullRequestFilestreats all ofdata/**as clinical-risk. That is the gate failing closed,and correctly so: it cannot know from the path alone that this particular file is an issue-ledger
count rather than clinical content. Every item below is attested on its merits, not ticked to pass.
The diff contains no clinical content, no ingestion, retrieval, answer-generation, source-rendering,
document-access, privacy or production-configuration change.
Clinical KB Database(sjrfecxgysukkwxsowpy)Notes on the last item specifically: no clinical decision-support behaviour changed, so the
TGA SaMD classification is untouched. The only runtime behaviour this PR alters belongs to a
synthetic prototype reachable solely from
/mockups/ward-flow, which 404s in production.Notes
main(merge commit2717fc583). It was originally leftbehind deliberately, but the snapshot regeneration below had to be computed against the ledger CI
actually evaluates: this branch reported 98 open items where CI's merge result reported 81, so
regenerating on the stale base would have written a snapshot that disagreed with it. After the
merge, local and CI agree at 81 open / 431 archived. The merge tree was confirmed clean
(
git merge-tree, exit 0) before merging, and the full ward suite re-run afterwards:Test Files 49 passed (49),Tests 521 passed (521).this branch, so the D5 page-level test and the removal of the workaround it replaces must happen
on
claude/ward-flow-phases-6-7-design. That branch was actively advancing while this was written(
a53a3a994→0d29dd734within one session), so whoever picks it up should confirm ownershipbefore editing it.
docs/ward-flow-pinned-clock-handover.md§4 carries the full checklist, and thequeued inbox request records it durably.
removal: once Phase 6 lands on
main, both paths resolve normally and the entry should go.Note
Low Risk
Behavior change is limited to the prototype Ward Flow provider when
initialNowis supplied (tests/mockups only); production mockup layout does not pass the prop.Overview
Fixes
WardFlowProviderso a pinnedinitialNowactually setsnow(plus in-app clock offset) instead of always servingNOW_ANCHORwhile only suppressing ticks. The unpinned live-app clock path is unchanged.Adds four DOM tests that pin clocks before/after the fixture anchor, layer
ADVANCE_CLOCKon a pinned instant, and assert no interval when pinned—intended to catch regressions that would again make time-of-day branches untestable through the real provider.Also adds
docs/ward-flow-pinned-clock-handover.md(evidence, Phase 6 follow-through on another branch, traps), a scoped docs-link allowlist for Phase 6 paths cited but not on this branch, a queued P2 inbox request, and regenerateddata/outstanding-issues-snapshot.json/data/repo-awareness-snapshot.jsoncounts.Reviewed by Cursor Bugbot for commit c6e0800. Configure here.