chore(issues): close #170, and correct #309 to partially delivered - #1925
Conversation
Both rows describe work that has since landed, and left open they would send the next session to rebuild finished code. Closed with evidence taken from merged source on main 2d27039, not from PR state: #170 — services (service-facets.ts, scope segment on a `scope` URL param, quick filters evicted to composer suggestions), factsheets (SegmentedControl + counts), therapy-compass (filter-sheet.tsx deleted, converged in #1885/#1889), documents (converged in #1910 with meterContent/footerOverride). Contract also spread to dsm, forms, tools and differential-stream. #309 — the dense tier now lives in the shared ResultFilterSheet: find-a-filter plus collapse-by-default per facet group, ported UP from documents by PR F rather than duplicated, which is what the row's stop rule required. Queued as inbox requests rather than canonical edits. I first wrote straight to docs/outstanding-issues.md via scripts/outstanding-issues.mjs and check:ledger-write-discipline correctly rejected it — `npm run issues:done` routes through scripts/ledger-inbox.mjs, which is the merge-safe path. Reverted and redone. Run npm run issues:reconcile from a fresh ledger branch once this lands. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011btGFwWKYFWDs5McQkqz9J
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 51 seconds Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
Comment |
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011btGFwWKYFWDs5McQkqz9J
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b7ac0f244b
ℹ️ 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".
check:ledger-write-discipline compares a committed range, so an uncommitted edit to docs/outstanding-issues.md yields an empty range and a pass having evaluated nothing. Hit live in this session: I made a forbidden canonical edit, the gate said green, and only committing and re-running surfaced the real failure. Also records that scripts/outstanding-issues.mjs (canonical, reconcile side) and npm run issues:done (inbox, branch side) are different tools with nothing at the call site saying so. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011btGFwWKYFWDs5McQkqz9J
… band The Codex reviewer on PR #1925 is right and my closure was wrong. Verified on main 2d27039: result-filter-control.tsx computes const dense = facetGroups.length > 3 || totalFacetOptions > 20 so formulation — one facet group of nine derived domains, the exact case that opened #309 — evaluates dense=false and still renders a wrapping chip row. The 6-20 full-width tier with its right-aligned count column does not exist, and neither does the nine-option DOM assertion the row asked for. What PR F (#1910) delivered is the upper tier: find-a-filter and collapse for >3 groups or >20 options, which is what documents needed. Section 5 has two thresholds; I conflated them and would have archived the row whose specific band is unbuilt. Replaces the done request with an update recording the partial delivery, what remains, and an explicit stop rule against closing on the strength of the upper tier. #170 is unaffected — mode adoption did complete. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011btGFwWKYFWDs5McQkqz9J
BigSimmo
changed the title
2 participants
Summary
Two ledger rows were stale in opposite directions.
#170describes work that has shipped and would send the next session to rebuild finished code.#309describes work that is half shipped, and my first attempt at this PR wrongly closed it — corrected in review, see below.#170— shared-band filter adoption · closedEvidence read out of merged source on
main, not from PR state:src/lib/service-facets.tsexists; navigator importsresultFilterFacetGroup; scope segment is aSegmentedControlbacked by ascopeURL param; the six query-replacing quick filters now feedAnswerSuggestionChipsas composer suggestions rather than the sheetSegmentedControlwith per-option countssrc/components/therapy-compass/filter-sheet.tsxdeleted;screens/search-screen.tsxuses the shared control (#1885, #1889)meterContent/footerOverrideso its progress meter and "Browse all sources" footer survive (#1910)The contract also spread past the original six:
dsm-search-page,forms-search-results-page,tools-search-results-pageanddifferential-stream-workspaceare on the shared sheet.One correction recorded in the outcome: the earlier plan claimed factsheets' category presets discarded the query and needed evicting to the composer. Measured false —
searchHrefpreservesqand addscategory— so no eviction was needed and none was done.#309— facet density · stays open, updated to partially deliveredThis PR originally closed
#309and that was wrong. The Codex reviewer caught it and was right. Verified againstmain:Formulation passes one facet group with
formulationDomainsInUse.length === 9, so neither condition fires,denseisfalse, and it still renders a wrapping chip row — the exact case that opened the row.§5 has two thresholds. PR F (#1910) delivered the upper one (find-a-filter + collapse for >20 options or >3 groups), because that is what documents needed. The 6–20 full-width band with its right-aligned count column is unimplemented, as is the nine-option DOM assertion the row itself requested. I conflated "the dense tier landed" with "this row's band landed" and would have archived a row whose specific case is unbuilt.
The
donerequest is replaced with anupdaterecording which band shipped, which did not, the line and measurement proving it, and a stop rule against closing on the upper tier's strength. It preserves the reviewer's alternative: amend §5 to drop the middle band deliberately and record the contract as reversed rather than satisfied.Verification
npm run check:ledger-write-discipline—Ledger write discipline passed for f2abf5baf3f4..HEADnpm run check:outstanding-issues—Outstanding-issues guard passed: 310 rows (114 open, 196 archived), unique ids, next-id=313 above the highest, no merge driver, no ids deleted from base f2abf5baf3f4npx prettier --check docs/outstanding-issues-inbox/*.json—All matched files use Prettier code style!mainby grep before writing it, including thedenseexpression and formulation's group/option counts.Two process notes worth recording.
First, I wrote the closures straight into
docs/outstanding-issues.mdviascripts/outstanding-issues.mjsandcheck:ledger-write-disciplinerejected it.npm run issues:doneroutes throughscripts/ledger-inbox.mjs, which queues a merge-safe request instead — the raw writer is the reconcile-side tool, not the branch-side one. The two entry points are not interchangeable.Second, and worse: that gate reported a pass while the edit was uncommitted. It compares a committed range, so a dirty tree gives an empty range and a green verdict having evaluated nothing. Captured as a new
/issuesrequest in this PR.Not run, with reasons:
verify:ui,build,check:bundle-budget— no source file is touched.classifyPullRequestFilesreturnsclinicalRisk: false,operationalRisk: false,ragRanking: false, so no Clinical Governance Preflight orRAG impact:line is required.Risk and rollout
docs/outstanding-issues-inbox/.Notes
Still open and deliberately not closed here:
#275(the shared filter trigger's arbitrary spacing values, now leaked across five files) anddocs/filter-contract.md§7 (SearchScopeFilters— 20 server-side keys editable from no panel), which §7 itself records as not a prerequisite.npm run issues:reconcilefrom a fresh ledger branch applies these once this lands.