From 6c18d91748a816e3fea12b18fff4eb369c1a92a9 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 11:15:10 +0000 Subject: [PATCH] issues: close #120 as not reproducible on current main MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `verify:phone-chrome` exits 1, not 0, when Playwright cannot launch. Reproduced the row's own failure condition on main at fcd9041 — the container ships Chromium 1194 against the repo's required 1234, giving the same 13 launch failures the row describes — and captured OVERALL_EXIT=1. The gate stopped at the focused-browser stage and never reached full-ui. The exit status propagates at verify-phone-chrome.mjs:72-73 and run-playwright.mjs:271-273, and childProcessExitCode returns 1 on a non-integer status. None of those files has changed since 7581cfc, which predates the row, so a later fix is ruled out and the original exit-0 reading is what is in doubt. The leading hypothesis — the status was read through a pipe — is recorded as explicitly unverified, along with the zero-selected-test case this reproduction did not cover. Closed as not reproducible rather than as fixed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018K7sEKH35KZkWxvCnQcNN2 --- docs/outstanding-issues.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/outstanding-issues.md b/docs/outstanding-issues.md index 5ddaa6d1a7..1247f4c880 100644 --- a/docs/outstanding-issues.md +++ b/docs/outstanding-issues.md @@ -143,7 +143,6 @@ removed after current-main verification; it is not missing recommended work. | #106 | P2 | rec | Ingestion worker and indexing agent are verified by grepping their own source | **Outcome:** the ingestion worker and indexing agent are verified by executing code, not by asserting on their own source text. **Detail:** measured 2026-07-29 via `npm run test:coverage` — `worker/main.ts` (2,015 lines) and `supabase/functions/indexing-v3-agent/index.ts` (1,966 lines) each report **0% executed lines**; no test imports either module. Both are covered only by `readFileSync` + `toContain` assertions in `worker-safe-logging.test.ts`, `worker-visual-capture.test.ts` and `document-metadata-merge.test.ts`, which pass whenever a string is present and break on harmless refactors; `document-metadata-merge.test.ts` additionally reimplements the SQL deep-merge in TypeScript and tests the reimplementation rather than the worker. Area totals: `worker/` 18.6% lines, `supabase/functions/` 4.5%. **Next:** continue the extraction pattern that already works here — `indexing-v3-agent/behavior.ts` (167 lines, 96%) and `ingestion-worker/auth.ts` (30 lines, 90%) — pulling the highest-risk decision points out of `worker/main.ts` (job claim/retry, generation commit, failure classification) into importable modules with executing tests, retiring the matching source-text assertion as each lands. Roughly cost-neutral: each extracted test replaces a grep assertion. **Stop:** do not try to make the 2,000-line entrypoint importable in one pass; extract incrementally and keep each step green. | session 2026-07-29 test-coverage analysis | 2026-07-29 | | #107 | P2 | rec | Component state matrices are the largest untested surface | **Outcome:** loading / empty / error / disabled states on interactive components are covered by executing tests, not only by E2E happy paths. **Detail:** measured 2026-07-29 — production components (excluding mockups) sit at **38.2% lines / 22.8% branch** across 12,602 lines, with **83 of 208 files at zero executed lines**; there are 51 `.dom.test.tsx` files against 195 components. Playwright does visit these routes, so they are smoke-covered, but branch coverage is where the state matrix lives and smoke journeys rarely reach it. Worst by uncovered lines: `global-search-shell.tsx` (7%), `mode-action-popup.tsx` (21%), `answer-content.tsx` (27%), `document-search-results.tsx` (32%), `universal-search-command-surface.tsx` (39%), `master-search-header.tsx` (43%). A concrete first target with clinical meaning: `calculator-ui.tsx` now covers all exported scoring logic, but `seedCheckboxDefaults`, `toggleCheckboxAnswer` and `selectOptionAnswer` stay uncovered because they are module-private and only reachable through React event handlers — `seedCheckboxDefaults` is what makes an all-negative CAGE / SAD PERSONS screen read as a valid 0 rather than incomplete, so a regression there is a false-negative risk. **Next:** treat as a per-PR convention rather than a backfill push — `docs/testing.md` already prescribes the state matrix, so the gap is enforcement. Start with `global-search-shell.tsx`, which `docs/search-chrome-behaviour.md` treats as a contract surface. Keep additions in the jsdom tier (measured ~0.54s per file) instead of new Playwright journeys (~231 production journeys already run serially at `workers: 1` against a 45-minute CI budget). **Stop:** do not chase the coverage percentage by backfilling low-risk components; the re-ratcheted broad floor in `vitest.config.mts` holds the line. | session 2026-07-29 test-coverage analysis | 2026-07-29 | | #110 | P3 | task | Design-system project token manifest lags its stylesheet | **Outcome:** the claude.ai/design token panel matches the shipped stylesheet. **Detail:** PR #1375 pushed a recompiled `_ds_bundle.css` (Clinical Sky, `--e0`–`--e4`, 4px radius grid, `--tracking-eyebrow`/`--leading-display`/`--leading-prose`) plus the four changed guideline docs to project `08d6f126`, but `_ds_manifest.json` is converter-generated and still advertises `--text-4xs: 0.5rem`, the old `--radius-lg/xl/2xl` values, and `--tw-leading`/`--tw-tracking` entries scoped to the retired `.leading-[…]` / `.tracking-[0.08em]` utilities. Rendering is correct; only the token inventory lags. Hand-editing was rejected — `kind`/`scope`/`annotation` are converter heuristics and a wrong panel is worse than a stale one. **Next:** in a session with the `/design-sync` skill, `npm ci`, then `npm install --prefix .ds-sync --no-save --package-lock=false esbuild ts-morph @types/react @tailwindcss/cli geist`, read `.design-sync/NOTES.md`, and run `resync.mjs --remote` so bundle and manifest regenerate together. **Stop:** do not hand-author `_ds_manifest.json`; the converter is not a published npm package and ships with the skill. | PR #1375; `.design-sync/NOTES.md`; project `08d6f126` (`_ds_needs_recompile` marker present) | 2026-07-29 | -| #120 | P2 | issue | `verify:phone-chrome` exits 0 while reporting failed browser tests | **Outcome:** the phone-chrome gate cannot report success when no test executed. **Evidence 2026-07-30 (PR #1396):** `npm run verify:phone-chrome` finished with **exit code 0** while its own output ended `13 failed`. Every one of the 13 failed at browser launch (`browserType.launch: Executable doesn't exist ... chrome-headless-shell`), so zero assertions ran, yet the gate returned success. This is the green-when-broken case `AGENTS.md` warns about ("Exit code 0 alone is not proof") realised in a gate that is supposed to be the proof. **Next:** make the runner propagate the Playwright exit status, and fail loudly on a launch error rather than treating a zero-test run as a pass. Stop: do not paper over it by grepping output in the caller — the runner owns the status. | `scripts/verify-phone-chrome.mjs`; `scripts/run-playwright.mjs` | 2026-07-30 | | #121 | P3 | issue | Container Playwright browser build lags the pinned client | **Outcome:** browser gates run in remote sessions without hand-patching. **Evidence 2026-07-30:** the repo's Playwright client resolves headless-shell build `1234`; the container image provides `1194` at `/opt/pw-browsers`, so every browser test fails at launch. Worked around in-session by symlinking `chromium_headless_shell-1234/chrome-headless-shell-linux64/chrome-headless-shell` to the `1194` `headless_shell` binary plus its sibling resources — container-local, nothing committed, and it disappears with the session. `PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD=1` means the mismatch cannot self-heal. **Next:** decide whether the image pins the browser build or the repo pins a client matching the image; until then any remote session claiming browser proof must state which it used. | `docs/testing.md`; container `/opt/pw-browsers` | 2026-07-30 | | #122 | P2 | issue | `ci/circleci: verify` fails on every branch and its log needs operator access | **Outcome:** the CircleCI status is trustworthy signal again, or it stops reporting. **Evidence 2026-07-30:** `ci/circleci: verify` was `failure` on every open PR sampled — #1396, #1407, #1405, and #1400, which is a **docs-only** `AGENTS.md` change — plus #1403's head. It is sharply bounded in time: #1393's head **passed** at build 638 (03:57), and builds 645 (04:09) onward all failed. The job's entire contents were mirrored locally on PR #1396's exact tip and every part is green — `format:check` clean, `lint` exit 0, `typecheck` exit 0, `npm run test` `432 passed (432)` / `4473 passed \| 4 skipped`, and the PyMuPDF-gated `tests/pdf-extractor.test.ts` (the repo's only `process.env.CI`-gated tests) `6 passed (6)` under a locally built `PyMuPDF==1.28.0` venv with `PYTHON_BIN` set exactly as `.circleci/config.yml` does. So the failure is in the job's **environment**, not repo code. Around 40 builds fired in ~40 minutes across 8 open PRs in that window, so credit/quota exhaustion is the leading hypothesis — **explicitly unverified**: the CircleCI project is private and no CircleCI token is available to any agent session, and `api/v1.1/project/gh/BigSimmo/Database/` returns `Build not found` unauthenticated. **Next:** an operator opens one failing build and reads the failing step; if it is quota, either raise it or remove the CircleCI status so it stops masking real reds. **Stop:** do not chase this from a PR branch — it is not branch-specific, and no agent can read the log. Do not go looking for a CircleCI token. | `.circleci/config.yml`; PR #1396 session 2026-07-30 | 2026-07-30 | | #125 | P3 | issue | `ui-therapy-nav-scroll.spec.ts` cites a spec file that does not exist | **Outcome:** a reader following the comment finds the coverage it names, or the comment stops naming it. **Detail:** the spec's comment points at `mode-nav-bar-anchoring.spec.ts`, left behind when PR #1390 moved `ModeNav` into the universal header; no such file exists anywhere in the repo. Harmless at runtime, but it sends the next person looking for anchoring coverage to a file that is not there, and it is the kind of stale pointer that makes a reader distrust the surrounding comments. Either repoint it at the coverage that actually exists (`ui-mode-nav-density.spec.ts`, landed in #1405) or delete the reference. | Noticed and explicitly deferred in PR #1405's body to keep that diff scoped to `#113` | 2026-07-30 | @@ -180,6 +179,7 @@ Move resolved rows here with the resolution date and a one-line outcome. Keep th | #112 | issue | `issues:next-id` has no concurrency protection | RESOLVED 2026-07-30. `npm run check:outstanding-issues` now gates this file, in `verify:cheap` and in the `static-pr` CI job (the gate-manifest check refuses a local gate that CI does not run). It fails on a duplicate id, an id present in both tables, a marker at or below the highest id, a malformed row, and a missing heading or marker — so every shape the 2026-07-29 triple collision took is now a red gate rather than a silent row loss. Verified by replaying that collision against the real file: two rows claiming `#110` produced "#110 appears 2 times (lines 151, 164)", and a lost marker bump produced "issues:next-id=113 is not above the highest id #114". The checker honours `\|` escapes — its first run against the live file flagged row #042, which is correctly escaped, and a gate with false positives is a gate people switch off. NOT fixed: the underlying race. Ids are still allocated by read-modify-write with no lock, and this file still has no `merge=union` driver; what changed is that a collision can no longer land silently. Source: session 2026-07-29 PR sweep; PR #1391 conflict resolution; `.gitattributes` | 2026-07-30 | | #113 | issue | `ModeNav` clips its labels at every phone width on `main` | RESOLVED 2026-07-30 (PR #1405, landed on `main` as `020c1260`). The two lower density bands used `grid-auto-columns: 1fr`, so the widest slot set every slot's width and the label's `truncate` hid the shortfall — nothing overflowed and nothing failed, the word was simply gone. `display: flex` at every band removes the mode rather than retuning it; thresholds went `16/26/34rem` -> `22/33/42rem` against remeasured intrinsic widths (471px for four labels, not the 394px originally budgeted). `tests/ui-mode-nav-density.spec.ts` asserts at both band boundaries and one pixel either side, registered in BOTH Playwright allowlists. **Carry-forward:** the ~8% threshold headroom is not padding — a measured-plus-one-pixel attempt (21/31rem) passed locally and failed CI by exactly 1px, because the variable face rasterises wider on `ubuntu-24.04`. A threshold calibrated to one machine's font metrics is calibrated to nothing. | 2026-07-30 | | #114 | issue | The live Web-Vitals instrument cannot measure its own noise | RESOLVED 2026-07-30 (PR #1407, landed as `a73db460`). Each route/strategy cell is measured `SAMPLES` times (default 3), the MEDIAN is graded, and a cell whose samples straddle a threshold is reported as incomplete evidence rather than resolved either way — so #017's "evidence is too noisy" stop rule can finally fire. Mid-run Chrome drift disqualifies the verdict. **Second defect found in review and fixed in the same PR:** `samples` is a free-text `workflow_dispatch` input, so `samples=1` reached the grader as a valid integer and reproduced the exact single-run matrix this work removes — verified against the pre-fix logic, it reported 0 breaches, 0 incomplete and printed the bold "Every mobile route is within" verdict. Now refused in three places (`main()` before reading any report, `renderTable`'s disqualifier list, and the workflow before spending a live measurement pass). | 2026-07-30 | +| #120 | issue | `verify:phone-chrome` exits 0 while reporting failed browser tests | RESOLVED 2026-07-30 — **closed as not reproducible, not as fixed.** The premise is contradicted by a reproduction of its own failure condition. **Evidence, `main` at `fcd9041`:** the container ships Chromium build `1194` while the repo wants `1234`, so `npm run verify:phone-chrome -- --files src/components/clinical-dashboard/mobile-composer-reserve.ts` reproduced the identical shape — the same `13 failed`, every one at `browserType.launch: Executable doesn't exist at /opt/pw-browsers/chromium_headless_shell-1234/…` — and the captured status was **`OVERALL_EXIT=1`**, not 0. The gate stopped at the `focused-browser` stage and never reached `full-ui`, which is correct; the three stages ahead of it were genuinely green (`Test Files 9 passed (9)`, `Tests 105 passed (105)`). **The code path agrees:** status propagates at `scripts/verify-phone-chrome.mjs:72-73` and `scripts/run-playwright.mjs:271-273`, and `childProcessExitCode` returns `1` when `status` is not an integer, so a launch error cannot read as 0. Neither file, nor `scripts/child-process-result.mjs`, has changed since `7581cfc`, which predates this row — so "fixed in the meantime" is ruled out and the original exit-0 reading is what is in doubt. **Leading hypothesis, explicitly unverified:** the 0 was read through a pipe, since `npm run verify:phone-chrome` piped to `tail` reports `tail`'s status rather than the gate's. The original command line was not recorded, so this cannot be confirmed. **Not covered by this reproduction:** the zero-selected-test case (a `--grep` matching nothing) was never exercised — only the launch-failure case this row describes. **If it recurs:** capture the exact command line and `echo $?` with no pipe before re-filing. | 2026-07-30 | | #012 | rec | Slim the lazy cross-mode differentials chunk | Precomputed a trimmed index (`src/data/cross-mode-differentials-index.json` via `scripts/build-cross-mode-differentials-index.mjs`) so the lazily-loaded cross-mode chunk imports a ~53 KB catalog instead of statically pulling the ~1.2 MB differentials snapshot (only that dynamic path reached it). A drift test plus `check:cross-mode-index` (in verify:cheap) lock the index to the live projection. | 2026-07-27 | | #029 | issue | Residual answer-quality fallback stubs | Closed after fixing each causal cluster independently. Active-community ED, community-home-visit, clozapine blood-threshold/typo, discharge source-gap recovery, and Best Practice Prescription now use narrowly validated, source-bound answers or auditable recovery; cited provider refusal prose can no longer masquerade as grounded, and terminal gaps retain no claim citations. The final 44-case gate reported 30/30 substantive grounded supported answers, 14/14 unsupported correct, zero review fallbacks, zero citation/numeric failures, and zero route-ceiling failures. Measurement still reports review fallback separately and denies targeting credit for echoed boilerplate. | 2026-07-27 | | #019 | issue | Preserve admission/discharge sources through comparison fallback | The actual fallback path now selects source-bound facts, preserves one admission and one discharge citation from distinct documents, and terminates at an evidence gap for qualified, negated, unrelated, title-only, single-sided, or same-document traps. Both exact live cases complete in about one second with zero provider calls; the final 44-case canary passed them with two citations each, while the 36-case retrieval canary held recall 1.0 and zero RR regressions. Retrieval scores, aliases, clamps, and comparator ordering were unchanged. | 2026-07-27 |