Correct the Ward Flow Phase 5 documents that outlived the merge, and record how to go faster - #2405
Merged
Merged
Conversation
…record how to go faster PR #2390 merged on 2026-08-26 as squash commit ea5482b with every check green, but six documents still described Phase 5 as unfinished. A session resuming from them on 2026-08-27 re-derived a settled bundle-budget decision, prepared to fix an already-green check, and began re-running a flake experiment that had already been performed ten times. None of that was a tooling problem: the stale instruction and the true record were in different documents, and the stale one was the one being read. Two of the stale statements were live hazards rather than untidiness: - `docs/ward-flow-complete-ledger.md` recorded the day-boundary release-banding finding as REJECTED, and recorded a premise ("there is no next operating day in this prototype") that the shipped code contradicts. It was in fact re-implemented narrowly and shipped. A session trusting the ledger would have reverted working code. - The binding spec's D5 said only "Now - already released", which does not describe the shipped `releaseBand()` and would have licensed that same revert. Clarified to match the code; the code and the clause now agree. Also corrected: the handover's status banner, its resume block (which pointed at a merged branch), its dead-SHA gate receipts, the whole of section 6 (marked historical, with the decision that was actually taken written down), section 9 (the flake experiment is answered, not pending), the roadmap's status line, and the kickoff prompt (marked obsolete, with the one instruction in it that is still live called out). New: `docs/development-speed-playbook.md` - how to go faster in this repository without weakening safety, quality, gates or the product. It is built from mechanisms that already exist - the gate arbiter and its record-ci path, gate receipts, fail-closed narrow selection, worktree dependency reuse, CI scope routing - plus what this session measured directly, including that run-playwright.mjs builds a full production app per invocation, that a bundle baseline set to the measured value passes by arithmetic rather than needing a rebuild, and that piping a long run through a bare tail hides both progress and the exit code. Section 12 states what must never be traded for speed, and nothing in the document asks for a gate to be skipped or an assertion weakened. Ledger inbox: - New P2 issue: data/outstanding-issues-snapshot.json's ledger_revision was rolled backwards by commit ca37696, from a 2026-08-25 revision to a 2026-08-22 one, by a regenerator running from a stale base. Regenerating with the repo's own tool in this change moved it forward again, which confirms the diagnosis; the issue asks whether the generator should refuse a backwards move so the class becomes self-detecting. - Cancelled request ecabc51d: it would have filed "build Ward Flow Phase 5" as an open P2 task for work that is merged. Verification. Change scope classifies docs_only, so the docs route is the correct gate set and no lint, typecheck, test or build is selected for it. Run and green: docs:check-index ("all 62 repository roots/modules/routes and all schema tables are indexed"), docs:check-inventory ("275 script files, 279 npm scripts"), docs:check-links ("3729 repo path references resolve"), sitemap:check (exit 0), and prettier --check on every changed file ("All matched files use Prettier code style!"). check:outstanding-issues initially reported the snapshot behind at pending 12 vs 14; regenerated with scripts/generate-outstanding-issues-snapshot.mjs, never by hand. check:ledger-write-discipline refused to report a verdict until the new inbox requests were committed, which is that gate working as designed; it is re-run after this commit. No product code changes. No gate skipped, no assertion deleted, no test loosened, no tolerance lowered. No provider-backed gate run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Contributor
Warning Review limit reachedNext included review available in 57 minutes. View limit detailsLimit details: You’ve used the included review currently available. Your 95 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 (10)
📒 Files selected for processing (2)
Comment |
Contributor
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_376f5b0f-bedd-4365-b93d-f0849a97238d) |
BigSimmo
enabled auto-merge (squash)
August 27, 2026 03:08
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:77d52620cd
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
…e-5-doc-correction # Conflicts: # data/outstanding-issues-snapshot.json
Automated review on PR #2405 was right and the text was wrong. Both the new playbook and the Phase 5 handover repeated long-standing wording that scripts/run-playwright.mjs "exits 0 when tests fail and when it refuses to run". Reading the script shows otherwise: it exits 75 with a DATABASE_HEAVY_RUN_ADMISSION_BUSY marker on admission contention, propagates Playwright's own exit status on test failure, and exits 1 on a wrapper error. That wording was not merely imprecise, it was harmful: it told callers to discard a reliable signal and parse logs instead, which destroys the distinction between blocked (retry) and red (fix). Both documents now say to check the non-zero status AND the decisive output line, and say what each one tells you. The separate observation stands and is kept, because it is independent of the exit code: a run can exit 0 having produced no "N passed" line at all, which proves nothing ran. That was observed here on 2026-08-27. Also tightened the neighbouring point about piping through a bare tail, which is the mechanism by which the exit code gets discarded in practice: a pipeline's status is its last command's. AGENTS.md carries the same stale wording and is deliberately NOT edited here - that is a policy surface, and changing it would widen this PR's risk classification. Filed as a P2 issue instead, which also asks whether a contract test should pin the script's exit codes so the guidance cannot drift from the code again. Verified: prettier --check clean on all changed files; docs:check-links "3728 repo path references resolve"; check:outstanding-issues "in step (91 open, 22 pending)". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #14097 (failure). 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 the "Repo awareness snapshot" step: data/repo-awareness-snapshot.json did not list docs/development-speed-playbook.md. Regenerated with the repository's own tool (npm run snapshot:repo-awareness), never hand-edited. The diff is exactly the new document's entry plus the two counts it moves (documents 466 -> 467, catalogued 102 -> 103) and the generator's own commit pointer. Verified: check:repo-awareness-snapshot "in step with data/repo-awareness-snapshot.json (195 pages, 467 documents, 2630 reviews)"; prettier --check clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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 freeto 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.
Summary
Documentation only. No product code changes.
PR #2390 merged on 2026-08-26 as squash commit
ea5482b9with every check green, but six documents still described Ward Flow Phase 5 as unfinished. A session resuming from them on 2026-08-27 re-derived a settled bundle-budget decision, prepared to fix an already-greenBuildcheck, and started re-running a flake experiment that had already been performed ten times and tabulated in a different document.That was not a tooling problem. In each case the stale instruction and the true record lived in different documents, and the stale one was the one being read.
Corrected two live hazards, not just untidiness.
docs/ward-flow-complete-ledger.mdrecorded the day-boundary release-banding finding as REJECTED, together with a premise ("there is no next operating day in this prototype, so 'Released today' never needs to empty") that the shipped code contradicts — the demo clock does roll days. The finding was in fact re-implemented narrowly and shipped. Separately, the binding spec's D5 said only "Now — already released", which does not describe the shippedreleaseBand(). Either document, read on its own, would have led a later session to revert working code. Both now match the code and say so explicitly.Corrected the handover (
docs/ward-flow-phase-5-handover.md): a status banner at the top, a resume block that no longer points at a merged branch, gate receipts no longer pinned to a SHA unreachable frommain, §6 marked historical with the decision that was actually taken written down, and §9 rewritten to give the flake experiment's recorded answer instead of prescribing it again.Corrected the roadmap's status line and marked the kickoff prompt obsolete, while calling out the one instruction inside it that is still live (flag spec D14 to the owner before Phase 6).
Added
docs/development-speed-playbook.md— how to go faster in this repository without weakening safety, quality, gates or the product. Built from mechanisms that already exist (the gate arbiter and itsrecord-cipath, gate receipts, fail-closed narrow selection, worktree dependency reuse, CI scope routing) plus what this session measured directly:run-playwright.mjsbuilds a full production app per invocation, so looping it N times pays N builds; a bundle baseline set to the measured value passes by arithmetic and needs no rebuild to verify; and piping a long run through a baretailhides both progress and the inner exit code. §12 states what must never be traded for speed. Nothing in it asks for a gate to be skipped or an assertion weakened.Ledger inbox. New P2 issue:
data/outstanding-issues-snapshot.json'sledger_revisionwas rolled backwards by commitca376969b, from a 2026-08-25 revision to a 2026-08-22 one, by a regenerator running from a stale base. Regenerating with the repo's own tool in this change moved it forward again, which confirms the diagnosis; the issue asks whether the generator should refuse a backwards move so the class becomes self-detecting. Also cancelled requestecabc51d, which would have filed "build Ward Flow Phase 5" as an open P2 task for work that is already merged.Verification
scripts/ci-change-scope.mjsclassifies this change asdocs_only: true—static_heavy_changed,coverage_changed,ui_changedandbuild_changedare all false — so the documentation route is the correct gate set, and lint, typecheck, the unit suite and the build are not selected for it.Run locally, all green:
Verification run. Results below, each quoted from the gate.
npm run docs:check-index— "docs/codebase-index.md coverage OK: all 62 repository roots/modules/routes and all schema tables are indexed."npm run docs:check-inventory— "Docs inventory current: 275 script files, 279 npm scripts."npm run docs:check-links— "docs link check passed: 3729 repo path references resolve."npm run sitemap:check— exit 0.npx prettier --checkon every changed file — "All matched files use Prettier code style!" Two files failed this on the first pass and were fixed with--writebefore committing.npm run check:outstanding-issues— initially reported the snapshot behind atcounts.pending12 vs 14; regenerated withscripts/generate-outstanding-issues-snapshot.mjs, never by hand.npm run check:ledger-write-discipline— refused to report a verdict until the two new inbox requests were committed, which is the gate working as designed; re-run after the commit.No gate was skipped, no assertion deleted, no test loosened, no tolerance lowered. No provider-backed gate was run (
verify:release,eval:*,check:supabase-project,test:liveall untouched).Risk and rollout
Risk: low - documentation, two merge-safe ledger inbox records and one regenerated snapshot; no runtime code is touched, so no user-facing behaviour can change.
Documentation and two merge-safe ledger inbox records. The only behavioural surface touched is the binding spec's D5 clause, which was clarified to match already-shipped code rather than changed to require anything new — the code is unchanged by this PR.
Rollback: revert the single commit. It touches no runtime code, so there is nothing to roll out and nothing to roll back beyond the documents and two ledger records.
One judgement worth a reviewer's eye: the ledger's item 4 and spec D5 both previously licensed reverting a shipped fix. I have rewritten both to say the fix shipped and must not be reverted. If the original ruling that rejected the fix is still the owner's preferred behaviour, then the code on
mainis what needs revisiting, not these documents — say so and I will raise it separately rather than let the documents and the code disagree again.Clinical Governance Preflight
Completed because
scripts/pr-policy.mjsclassifies this change asclinicalRisk: true. The trigger is the single pathdata/outstanding-issues-snapshot.json, which is a generated bookkeeping snapshot of the outstanding-issues ledger and carries no clinical content; every other changed path classifies false. The section is completed in full rather than waved away, because the classifier's judgement is the one that governs the merge.Clinical KB Database(sjrfecxgysukkwxsowpy)Each item holds trivially: this PR changes documentation plus two merge-safe ledger inbox records and one regenerated snapshot. No ingestion, answer-generation, retrieval, ranking, source-rendering, document-access, privacy or production-environment code is touched, no Supabase configuration is touched, no key handling changes, and no clinical decision-support behaviour changes — so the SaMD classification is unaffected. Ward Flow itself remains a synthetic, offline prototype reachable only from the administrator-gated developer hub, holding no real patient data, and is not clinical decision support.
Notes
data/outstanding-issues-snapshot.jsonchange was produced byscripts/generate-outstanding-issues-snapshot.mjs, never hand-edited. Regenerating it also movedledger_revisionforward from the 2026-08-22 revision back to the 2026-08-25 one, repairing the backwards roll described in the summary.Note
Low Risk
Docs, inbox JSON, and a generated snapshot only; no auth, data paths, or clinical runtime behavior change.
Overview
Documentation and ledger bookkeeping only—no product or runtime code changes.
Ward Flow Phase 5 is marked complete and merged (PR #2390,
ea5482b9) across the handover, kickoff prompt, roadmap, complete ledger, and binding spec D5. Stale “resume this branch / fix Build / rerun flake experiment” instructions are replaced with historical record and pointers to what is still owed (spec D14 clinician validation, Phase 6/7). The ledger and spec now agree that day-boundary release banding shipped narrowly (releaseBand()/ “Released today”) and must not be reverted to unconditional"now".New
docs/development-speed-playbook.mddocuments existing speed levers without weakening gates: verify state via git/PR before handovers, gate arbiter +record-ci, gate receipts, narrowtest:focused, worktree reuse, deliberate Playwright/build cost, bundle-budget arithmetic, lock contention, and CI scope routing. Linked fromdocs/README.md.Outstanding-issues ledger: inbox cancels the request that would re-file Phase 5 work; adds P2 issue for
ledger_revisionrolling backwards when the snapshot regenerator ran from a stale base; regeneratesdata/outstanding-issues-snapshot.json(pending 12→14, revision pointer forward again).Reviewed by Cursor Bugbot for commit 77d5262. Configure here.