docs(#585): regenerate ADR-0005 evidence with the flaky-test fix; fix LOC-classification drift - #625
Merged
Conversation
…'s file split Regenerating evidence with the WebKit flaky-test fix in place surfaced a second, unrelated bug: the deletion-estimate went from -157 to -192 LOC, the wrong direction. Root cause: classifyFunctionRangesFromSource's boundary regex only ever matched export-prefixed top-level declarations. Phase 1 (PR #621) restructured ch-client.ts and moved chUrl (+ the progress-line stream loop) into a new src/net/clickhouse-http-transport.ts, and several other classified helpers (isAbort, errMessage, isCurrentEpoch, staleEpochAbort, querySystemAware, loadDataLakeCatalogTableNames) lost their export keyword along the way. None of this was tracked: symbols before the file's first remaining export boundary were silently dropped from every bucket, and symbols between two boundaries were misattributed to the earlier one's bucket, with no error raised in either direction. The boundary regex now matches top-level function/const declarations whether or not they're exported (still anchored to true top-level, never nested). A new symmetric drift guard throws when a classification-table entry stops matching anything, mirroring the existing throw for an unclassified symbol found in the source — this is exactly the failure mode that let the drift go unnoticed. That guard immediately caught a second, pre-existing latent bug (OfficialConnection, a type-only interface, was never actually matched by either regex and had been a dead table entry all along) and surfaced two more real symbols (transportFor, DOC_PROBE_TABLE_NAMES in ch-client.ts; classifyError, flattenHeaders in official-adapter.ts) that were always non-exported and always invisible to the old mechanism. src/net/clickhouse-http-transport.ts is now classified on its own (HTTP_TRANSPORT_CLASSIFICATION: chUrl/streamLines/createHttpTransport, all delete-after-cutover per the ADR's ownership boundary) and combined into the net-deletion formula alongside ch-client.ts's own bucket. Corrected estimate (via computeDeletionEstimate(), no live-matrix rerun needed for this part): currentGenericLoc 131, estimatedOfficialAdapterLoc 190, acceptedBridgeGuardLoc 95, netExecutableDeletion -154 — close to the original -157 (the corrected pre-Phase-1 baseline), confirming the earlier Rejected conclusion on this gate holds for the right reasons, not the transient miscounting introduced by Phase 1. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MqDRfSfy6w1FJnFibYY44F
…he ADR Regenerates docs/evidence/585/ from a full run-matrix.mjs pass (all 4 server rows, both browsers) with the WebKit flaky-test fix and the deletion-estimate LOC-classification fix both in place, then reconciles docs/ADR-0005-clickhouse-web-client.md's prose and quoted evidence mirrors to match. Corrected outcomes: - Browser-matrix gate: fail -> pass (16/16, reproduced across two independent full-matrix reruns). - Proposed ClickHouse minimum: 26.6.2.160 -> 26.3.16.10001.altinitystable (that row no longer excluded now that its browser-matrix cell passes). - Net production-code deletion estimate: -157 -> -154 (131 eligible - 190 adapter - 95 bridge/guard), now including src/net/clickhouse-http- transport.ts's own bucket alongside ch-client.ts's. The Rejected decision is unchanged: the supported-server-matrix gate (confirmed-live 24.8.x incompatibility) fails independently of both corrections. Added an "Evidence correction addendum" section to the ADR documenting both fixes and their provenance; updated the CHANGELOG's existing #585 entries to match. docs/evidence/585/**'s own self-consistency validator (npm run check:client-spike:evidence) reports 92 findings against this regenerated evidence vs. 94 against the original (confirmed via git stash) -- the only two findings that disappeared are exactly the browser-matrix-flake ones this change resolves. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MqDRfSfy6w1FJnFibYY44F
29 tasks
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 free
to 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.
What & why
Follow-up to #624 (the WebKit flaky-test fix). Regenerates
docs/evidence/585/for issue #585 / ADR-0005 with that fix in place, and fixes a second, unrelated evidence-generation bug found along the way, then reconcilesdocs/ADR-0005-clickhouse-web-client.mdto match.Corrected outcomes:
fail→pass(16/16, reproduced across two independent full-matrix reruns — no retries even needed).26.6.2.160→26.3.16.10001.altinitystable(that row is no longer excluded now that its browser-matrix cell passes).-157→-154physical LOC (131 eligible − 190 adapter − 95 bridge/guard).The Rejected decision is unchanged. The supported-server-matrix gate (a confirmed-live, genuine ClickHouse 24.8.x incompatibility) fails independently of both corrections above.
The second bug (deletion-estimate LOC classification)
Regenerating evidence with #624's fix in place surfaced the net-deletion figure moving the wrong direction first (-157 → -192). Root cause:
run-matrix.mjs'sclassifyFunctionRangesFromSourceonly ever matchedexport-prefixed top-level declarations. #585 Phase 1 (PR #621) restructuredsrc/net/ch-client.tsand movedchUrl(+ the progress-line stream loop) into a newsrc/net/clickhouse-http-transport.ts, and several other already-classified helpers (isAbort,errMessage,isCurrentEpoch,staleEpochAbort,querySystemAware,loadDataLakeCatalogTableNames) lost theirexportkeyword along the way — none of this was tracked, so some symbols went uncounted entirely and others were misattributed to the wrong bucket, with no error raised in either direction.Fixed: the boundary regex now matches top-level
function/constdeclarations whether or not they're exported (still anchored to true top-level, never nested), and a new symmetric drift guard throws when a classification-table entry stops matching anything — mirroring the existing throw for an unclassified symbol found in the source. That guard immediately caught one more pre-existing latent bug (OfficialConnection, a type-only interface, was never actually matched by either regex and had been a dead table entry all along) and surfaced two more real, always-unexported symbols the old mechanism had always missed (transportFor,DOC_PROBE_TABLE_NAMESinch-client.ts;classifyError,flattenHeadersin the spike'sofficial-adapter.ts).clickhouse-http-transport.tsis now classified on its own and summed into the net-deletion formula.The corrected
-154lands close to the originally-recorded-157— the Rejected conclusion on this gate held for the right reasons all along, not the transient miscounting Phase 1's file split introduced.Changes
tests/spike/clickhouse-client/run-matrix.mjs/run-matrix.test.ts: the classification-drift fix above (commit 1).docs/evidence/585/**: regenerated from a fullrun-matrix.mjsrun (all 4 server rows, both browsers) with both fixes in place (commit 2).docs/ADR-0005-clickhouse-web-client.md: reconciled — updated the "Decision" gate count/bullets, the Support-minimum derivation, the Browser/origin-matrix and Decision-table mirrors, the Future-production-deletion-estimate numbers, Consequences, Alternatives considered, and the stale Limitations bullet; added a new "Evidence correction addendum" section documenting both fixes and their provenance.CHANGELOG.md: updated the existing ADR-0005### Addedentry (no longer cites the stale -157/WebKit-failure framing) and added a new### Fixedentry for this correction.Testing
npm run check:types,npm run check:arch,npm run check:schemas,npm run check:examples,npm test(100/97.14/100/100 coverage),npm run build— all green.npm run check:client-spike:evidence(the evidence's own self-consistency validator): 92 findings against this regenerated evidence vs. 94 against the original unmodified evidence (verified viagit stash) — the only two findings that disappeared are exactly the two browser-matrix-flake findings this change resolves; the remaining 92 are the expected, already-documented 24.8.x precision/live-scenario failures the ADR's supported-server-matrix gate is about.decision=Rejected) with no Docker/Playwright errors.Checklist
npm testpasses (the per-file coverage gate is non-negotiable)run-matrix.test.ts)npm run buildsucceeds (single-filedist/sql.html)tests/spike/clickhouse-client/,docs/evidence/585/,docs/ADR-0005-clickhouse-web-client.md,CHANGELOG.md; nothing undersrc/changedCHANGELOG.md([Unreleased]) updatedPart of #585 (does not close it — Phase 2-4 remain gated on a new decision; this PR corrects evidence precision, it does not reopen the decision).
🤖 Generated with Claude Code
https://claude.ai/code/session_01MqDRfSfy6w1FJnFibYY44F