Skip to content

docs(issues): queue two spec closures backed by a first-hand test run, plus a count correction - #2235

Merged
BigSimmo merged 3 commits into
mainfrom
claude/ledger-spec-closures
Aug 21, 2026
Merged

docs(issues): queue two spec closures backed by a first-hand test run, plus a count correction#2235
BigSimmo merged 3 commits into
mainfrom
claude/ledger-spec-closures

Conversation

@BigSimmo

Copy link
Copy Markdown
Owner

Summary

Queues three inbox requests that preserve evidence the reconciliation in pull request #2229 would otherwise have discarded. Three additive JSON files under docs/outstanding-issues-inbox/; the canonical ledger is not edited and nothing else is touched.

The Codex review on #2229 pointed out that the transaction cancelled two done requests whose evidence was exactly the re-measurement #SZGPAH and #4TBHS8 were waiting for, and then applied everything without preserving it — leaving both rows open and still claiming the spec had not been re-run. That is correct, and the cancellation authors said the same thing themselves: each cancellation reason ends "recommend a fresh done request be queued against the current row citing this same run so the evidence is not lost." The cancellations were only a fingerprint race — an already-merged branch had edited both rows after those requests were fingerprinted.

Rather than close the rows on another session's reported run, the spec was re-run here first-hand:

node scripts/run-playwright.mjs --project=chromium-mockups tests/ui-tools-search-mode-mockup.spec.ts
16 passed (1.5m) exit 0

Both previously-failing tests are green by name — test 1, desktop uses universal search and keeps results beside the selected-tool panel (spec line 24, 10.9s), and test 9, phone filter sheet follows the shared local-filter behavior (spec line 194, 4.5s). This independently reproduces the run at a341832af that the cancelled requests cited, so both rows close on a reproduced measurement rather than on a third-party claim.

The third request corrects #VTEW3W. Its earlier update recorded therapyBtn at 21 occurrences across 8 files; the request cancelled as c6a9756d recorded 13 call sites across 7, and that cancellation flagged the contradiction for human re-measurement rather than guessing. Re-measured per file, excluding each file's import line and the export in controls.ts: brief-screen 1, compare-screen 1, detail-screen 1, pathways-screen 1, recommend-screen 1, sheets-screen 7, therapy-card 1 — 13 call sites across 7 .tsx files. The 21/8 figure was wrong: it counted raw textual occurrences, including one import per consuming file, against a baseline expressed in call sites, and so overstated the remaining work. Against the 30 call sites measured on 2026-08-18, the real reduction is 17, not the 9 the bad arithmetic implied.

These are requests rather than a reconciliation on purpose: check:ledger-write-discipline rejects a branch that both creates a request and applies it, since the request never existed as queued on the base for anyone to review. They will be applied by the next serialized reconciliation.

Verification

  • node scripts/run-playwright.mjs --project=chromium-mockups tests/ui-tools-search-mode-mockup.spec.ts16 passed (1.5m), exit code 0, with both named tests green. This is the evidence the two closures rest on.
  • npm run check:outstanding-issuesLedger inbox check passed: 3 pending request(s), 485 applied. and Outstanding-issues guard passed: 423 rows (67 open, 356 archived), unique display and durable ids, collision-free allocation enabled, deprecated next-id marker ignored, no merge driver, no ids deleted from base 91681abdd16d.

UI verification not run beyond the spec above: this change adds no executable product code, route, component, style or browser behaviour. The Playwright run here is evidence for the ledger rows, not verification of this diff.

Risk and rollout

  • Risk: Very low. Three additive, immutable request files. Nothing reaches the canonical ledger until a later serialized reconciliation runs, and this change cannot alter any existing row. The substantive risk is a wrong factual claim inside a request, which is why each one names its command, its run, and the individual tests or per-file counts behind it.
  • Rollback: git revert this pull request, or delete the request files before reconciliation runs.
  • Provider or production effects: None.
  • RAG impact: none — no file under src/lib/rag/, no retrieval RPC, no ranking configuration, no eval harness and no golden fixture is touched.

Notes

The Codex findings on #2229 were posted in the review body with file permalinks rather than as inline review threads, so there was no thread to reply to or resolve; this pull request is the response to them.

Worth recording separately: the #VTEW3W error was mine, in a request landed earlier the same day, and it was caught only because another session measured the same thing differently and refused to reconcile the two numbers mechanically. That disagreement is the reason the ledger now holds the right figure.

🤖 Generated with Claude Code

…, and a count correction
Codex review on PR #2229 flagged that the reconciliation cancelled two done
requests whose evidence was exactly the re-measurement #SZGPAH and #4TBHS8
were waiting for, without preserving it. The cancellation authors said the
same thing and recommended fresh done requests be queued.
Rather than close on their claim, the spec was re-run here: 16 passed
(1.5m), exit 0, with both previously-failing tests green by name.
Also corrects #VTEW3W, whose earlier update counted raw text occurrences
(21 across 8 files) against a call-site baseline. Measured properly it is
13 call sites across 7 files.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@supabase

supabaseBot commented Aug 21, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project sjrfecxgysukkwxsowpy because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@coderabbitai

coderabbitaiBot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your current included review allowance is based on your included PR review attempts over the past 7 days.

Next review available in:15 minutes

Limit details: You’ve used the included review currently available. Your 87 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.

How can I continue?

Wait for the limit to reset, then comment @coderabbitai review or push new commits to the PR.

An organization admin can change what happens after included review limits in Billing.

How do review limits work?

CodeRabbit enforces per-developer PR review limits within each organization.

For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5ac2ffdc-61c3-43fd-a542-4f0a279f8391

📥 Commits

Reviewing files that changed from the base of the PR and between 59504e5 and d7771f3.

📒 Files selected for processing (3)
  • docs/outstanding-issues-inbox/51b7e3d4-9c38-4fd7-9aa8-70d46086fde9.json
  • docs/outstanding-issues-inbox/5d987a3d-3869-4864-8af0-de0a6145b682.json
  • docs/outstanding-issues-inbox/c1f99542-526a-4218-b8e6-87bf136d7567.json

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connectorchatgpt-codex-connectorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:37cd0a06a2

ℹ️ 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".

@BigSimmo
BigSimmo enabled auto-merge (squash) August 21, 2026 14:26
Codex review correctly noted that applyRequest/updateIssue only
touches fields present in the payload, so an update carrying detail
alone would leave the row's summary column still reading "30 raw
controls across 8 files" after reconciliation, contradicting the
corrected 13-across-7 detail. Added the matching summary field.
@BigSimmo
BigSimmo merged commit 3c94d55 into mainAug 21, 2026
32 checks passed
@BigSimmo
BigSimmo deleted the claude/ledger-spec-closures branch August 21, 2026 14:37
Sign up for freeto 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.

2 participants

@BigSimmo@claude