You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[finding] classifyIndexFailure's dialect arm reads only the OUTER message — a wrapped dialect refusal loses the unsupported verdict, and with it overlay-index's fallback index #6848
Recorded while implementing #6699 (PR #6845). Deliberately out of that card's lane — its ruling scoped the migration to the FIRST arm and said the dialect arm stays as-is — so this is filed rather than fixed.
What
packages/metadata-protocol/src/migrations/partial-index-probe.ts, after #6699:
The two arms now read different depths. isUniqueViolationError follows error.cause up to four levels, because pool and query-builder layers re-throw with the original attached. indexFailureText deliberately does not: it returns err.message and stops.
So a dialect refusal arriving behind a wrapper — outer prose Write failed or pool query failed, the actual near "WHERE": syntax error one step down cause — is classified failed instead of unsupported.
Why it is observation-class, not a live defect
Nothing in the repo produces that shape today. IndexExec is driver.raw(sql) or driver.execute(sql), and the drivers actually shipped hand knex's own error straight back with the dialect text on the outer message, which is why every existing test case matches on the first read. The asymmetry is dormant, not wrong.
It is also not a regression from #6699. Before that change the classifier received err.message as a string, so the dialect arm read exactly the same single channel. What #6699 changed is that the first arm now reads deeper, which is what makes the difference visible by contrast.
Consequence if a wrapping driver ever lands
Not merely a wording difference, which is why it seems worth writing down. The two consumers of the verdict diverge:
view-definition-active-index.ts — unsupported and failed both keep the previous index and both log at error. Only the wording differs (a named dialect limitation versus a cause we do not understand).
overlay-index.ts — the unsupported branch additionally builds the composite fallback lookup index (buildOverlayFallbackIndexSql), offered precisely because a dialect that cannot take the partial form should still get the lookup. Under a failed verdict that branch never runs, so the fallback is silently not attempted and fallback is reported as not-attempted rather than ensured / refused.
Possible dispositions
Give the dialect arm the same bounded cause walk the predicate has, so both arms read the same depth. Cheap, symmetric, and testable with a wrapped error.
Leave it, and say in the doc comment that the second arm is outer-message-only by design — the current state, just undocumented, so the next reader does not have to re-derive whether the asymmetry is deliberate.
No recommendation offered here; the choice depends on whether the platform expects to take a wrapping raw-SQL driver at all, which is a question for triage rather than for this finding.
Related: #6699 / PR #6845 (the first arm's migration onto @objectstack/types), #6250 (the shared predicate), #6418 (where the classifier and the probe-first order moved to).
Triage (maintainer-authorized one-off pass, 2026-08-09, registered on #6015): promoted finding → pm:queue (domain:metadata kept). The two arms of one classifier reading different wrap-depths is a concrete misclassification path with a named consequence (a wrapped dialect refusal grades failed instead of unsupported, costing overlay-index its fallback). #6699's ruling scoped that PR away from the dialect arm; it did not rule the arm permanent. Deliverable: give indexFailureText the same cause-walking depth isUniqueViolationError already has, plus a wrapped-refusal fixture.
Dispatching on the triage deliverable of 2026-08-09 06:50Z: give indexFailureText the same bounded cause walk isUniqueViolationError already has, plus a wrapped-refusal fixture. Triage's framing is carried into the dispatch verbatim — #6699's ruling scoped that PR away from the dialect arm, it did not rule the arm permanent, so disposition 2 (document the asymmetry as deliberate) is not the ruled path.
File surface: packages/metadata-protocol/src/migrations/partial-index-probe.ts (indexFailureText / classifyIndexFailure) plus its tests and a changeset. Anchored to function names, not line numbers — this family's anchors have moved twice (#6418 relocated the classifier; #6699 changed classifyIndexFailure's parameter from message: string to error: unknown), so the dev re-measures the current address before editing.
Round-7 siblings, all region-disjoint: #6190 (saveMetaItem two-tier write gate, protocol.ts ~:7xxx) and #6924 (assertSortFieldsExist ~:4745). This card is in a different file entirely.
Implemented disposition 1 per the triage ruling of 2026-08-09 06:50Z: indexFailureText now walks error.cause to the same bounded depth isUniqueViolationError uses, plus the wrapped-refusal fixture the deliverable names.
What I measured before editing
Anchored on function names, not line numbers, as the dispatch required.
PM assumption 2 — falsified in the direction of the fallback branch. There is no reusable message-collecting helper in @objectstack/types to prefer. It exports isUniqueViolationError and uniqueViolationColumn only; its two chain walkers (matchesUniqueViolation, findUniqueViolationColumn) are private, and each answers its own question rather than handing back text. Hoisting one would widen that package's contract for a single consumer, and editing @objectstack/types is a declared STOP for this card — so the walk is local, pinned to the same bound, with the reasoning recorded in the doc comment.
PM assumption 3 — confirmed, and the issue's "four levels" is accurate.MAX_CAUSE_DEPTH = 4 in packages/types/src/unique-violation.ts, guard depth > MAX_CAUSE_DEPTH, top-level call at depth 0 — so the thrown value plus four wrapper levels below it (five objects read). Mirrored exactly and counted the same way.
PM assumption 4 — the real answer is the opposite of the expected one, and is reported rather than tidied.matchesUniqueViolation keeps no visited set; the depth bound is its only cycle protection. "Match its behaviour" therefore meant not adding a visited-set guard, because a bound terminates a cycle just as well and a guard the predicate lacks would be a second asymmetry in place of the one being closed. A test pins that a self-referential cause returns a verdict instead of exhausting the stack.
The dialect regex is untouched — only the text fed to it, as scoped.
Changed, file by file
packages/metadata-protocol/src/migrations/partial-index-probe.ts — added MAX_CAUSE_DEPTH and the private collectIndexFailureText; indexFailureText now joins the chain. classifyIndexFailure's body is unchanged — the walk did not need it; only its doc comment gained a "Why both arms walk cause" note carrying the consumer-divergence rationale.
packages/metadata-protocol/src/migrations/partial-index-probe.test.ts — six new cases: wrapped refusal, arm order across the chain, depth parity, newline separator, cycle termination, end-to-end through probeThenReplaceIndex.
Deviation from the declared file surface, declared in the PR body too:packages/metadata-protocol/src/migrations/overlay-index.test.ts — one added test, test-only, no source change. It pins the consequence the card names (a pooled wrapper reaching fallback: 'ensured' instead of not-attempted). Without it the PR proves the verdict moved but not that the fallback index is actually reached again.
.changeset/index-failure-dialect-cause-walk.md — patch for @objectstack/metadata-protocol; classifyIndexFailure is a public export, so the change is user-visible.
Not touched, per the dispatch: protocol.ts, packages/spec, @objectstack/types, content/docs/releases/. No STOP was reached.
One judgement call worth a reviewer's eye
Levels join with a newline, never a space. Two dialect alternatives are multi-word (where clause, near "where"), so a space would synthesise a refusal across a wrapper boundary that no driver ever wrote. There is a test that goes red if the separator is changed to a space. Level-0 behaviour is byte-identical to the old body for every previously reachable shape, so no existing verdict moves.
Reverse verification — predictions written BEFORE running, then measured
Method: git checkout origin/main -- …/partial-index-probe.ts (fix out, new tests kept), re-run, restore. No git stash at any point.
× grades a WRAPPED dialect refusal `unsupported`, not `failed` (#6848)
× reads the dialect arm to exactly the depth the conflict arm reads (#6848)
× terminates on a `cause` chain that loops, exactly as the predicate does (#6848)
× the probe classifies a wrapped refusal, and leaves `detail` the OUTER prose (#6848)
× a POOLED wrapper around the dialect refusal still reaches the fallback index (#6848)
Tests 5 failed | 39 passed (44)
The parity pin's output states the defect most directly — the dialect arm reaching depth 0 only, against the conflict arm's 0..4:
Two assertions were recorded up front as green-in-both-directions and are reported as guards, not evidence — they constrain choices inside the new walk, which do not exist before the fix:
the newline-separator test (green before, vacuously — the cause is never read);
"arm order holds across the chain" (green before, because arm 1 already walked). This is the inversion-risk assertion: it would go red after the fix if widening arm 2 had stolen the verdict from arm 1. It stayed green, which is the direction that needed checking.
One prediction went unmeasured and is not claimed. The parity test's second assertion (literal profile [T,T,T,T,T,F,F]) never executed on the revert run — the parity equality above it failed first and short-circuited the test. Green with the fix in; its predicted red is unverified.
CI — per-job conclusions on 41d9415
All 25 check runs completed. No red anywhere; the two non-successes are path-filter skips.
job
status
conclusion
ESLint (carries the family gates)
completed
success
TypeScript Type Check
completed
success
Check Changeset
completed
success
Build Core
completed
success
Test Core (1/3, 2/3, 3/3)
completed
success
Dogfood Regression Gate (1/3, 2/3, 3/3)
completed
success
Temporal Conformance (live PG + MySQL)
completed
success
Build Docs / Console Pin Gate
completed
skipped (path filter)
Labels read back after the bots settled: documentation, size/m, tests, tooling. skip-changeset deliberately not applied — this PR ships a changeset and Check Changeset passed on it.
pnpm exec eslint on all three changed files — clean, exit 0.
node scripts/check-nul-bytes.mjs — OK, 6467 files; plus a self-scan of the changed files across the wider control-byte range, no hits.
Consumption radius swept: classifyIndexFailure / ensureOverlayStateIndex / PartialIndexStatus have no code consumers outside metadata-protocol; the three hits in packages/metadata are prose in comments.
Could not verify / flagged rather than omitted
tsc --noEmit on metadata-protocol reports 159 errors, 0 of which name any file in this PR — all pre-existing debt in unrelated seed-loader-* / sys-metadata-repository.* tests. That is why the package carries no typecheck script and sits in the type-check-debt ledger, so no per-package tsc gate reads my files directly; the type coverage they do get is the tsup DTS build (green) and the repo-wide TypeScript Type Check job (green). Recorded so the gap is visible rather than implied by a green tick.
Out-of-scope findings
None. Nothing outside the card's surface was tripped over, so nothing was filed.
Activity
os-project-manager commentedon Aug 9, 2026
Triage (maintainer-authorized one-off pass, 2026-08-09, registered on #6015): promoted
finding→pm:queue(domain:metadatakept). The two arms of one classifier reading different wrap-depths is a concrete misclassification path with a named consequence (a wrapped dialect refusal gradesfailedinstead ofunsupported, costing overlay-index its fallback). #6699's ruling scoped that PR away from the dialect arm; it did not rule the arm permanent. Deliverable: giveindexFailureTextthe same cause-walking depthisUniqueViolationErroralready has, plus a wrapped-refusal fixture.Generated by Claude Code
os-zhuang commentedon Aug 9, 2026
Claim: PM loop round 7 (
domain:metadataseat, sticker #6367)Session:
session_01W6bLax4KMrSfnE1ydFU8DwBranch:
claude/issue-6848-dialect-arm-cause-walkWorktree:
objectstack-issue-6848Domain:
domain:metadataBase:
origin/main@30283263cor newerDispatching on the triage deliverable of 2026-08-09 06:50Z: give
indexFailureTextthe same boundedcausewalkisUniqueViolationErroralready has, plus a wrapped-refusal fixture. Triage's framing is carried into the dispatch verbatim — #6699's ruling scoped that PR away from the dialect arm, it did not rule the arm permanent, so disposition 2 (document the asymmetry as deliberate) is not the ruled path.File surface:
packages/metadata-protocol/src/migrations/partial-index-probe.ts(indexFailureText/classifyIndexFailure) plus its tests and a changeset. Anchored to function names, not line numbers — this family's anchors have moved twice (#6418 relocated the classifier; #6699 changedclassifyIndexFailure's parameter frommessage: stringtoerror: unknown), so the dev re-measures the current address before editing.Round-7 siblings, all region-disjoint: #6190 (
saveMetaItemtwo-tier write gate,protocol.ts~:7xxx) and #6924 (assertSortFieldsExist~:4745). This card is in a different file entirely.Generated by Claude Code
os-zhuang commentedon Aug 9, 2026
os-dev report
Issue: #6848 · PR: #6989 (draft) · Branch:
claude/issue-6848-dialect-arm-cause-walk· Base:origin/main@30283263cStatus: done · Premise still valid: yes
Implemented disposition 1 per the triage ruling of 2026-08-09 06:50Z:
indexFailureTextnow walkserror.causeto the same bounded depthisUniqueViolationErroruses, plus the wrapped-refusal fixture the deliverable names.What I measured before editing
Anchored on function names, not line numbers, as the dispatch required.
classifyIndexFailure(error: unknown)and the privateindexFailureTextare both inpackages/metadata-protocol/src/migrations/partial-index-probe.ts, with exactly the asymmetry the issue describes. Nothing had moved again since [finding] classifyIndexFailure is a FIFTH private unique-violation vocabulary, missed by #6250's inventory — and it reads only the message channel #6699.@objectstack/typesto prefer. It exportsisUniqueViolationErroranduniqueViolationColumnonly; its two chain walkers (matchesUniqueViolation,findUniqueViolationColumn) are private, and each answers its own question rather than handing back text. Hoisting one would widen that package's contract for a single consumer, and editing@objectstack/typesis a declared STOP for this card — so the walk is local, pinned to the same bound, with the reasoning recorded in the doc comment.MAX_CAUSE_DEPTH = 4inpackages/types/src/unique-violation.ts, guarddepth > MAX_CAUSE_DEPTH, top-level call atdepth 0— so the thrown value plus four wrapper levels below it (five objects read). Mirrored exactly and counted the same way.matchesUniqueViolationkeeps no visited set; the depth bound is its only cycle protection. "Match its behaviour" therefore meant not adding a visited-set guard, because a bound terminates a cycle just as well and a guard the predicate lacks would be a second asymmetry in place of the one being closed. A test pins that a self-referentialcausereturns a verdict instead of exhausting the stack.Changed, file by file
packages/metadata-protocol/src/migrations/partial-index-probe.ts— addedMAX_CAUSE_DEPTHand the privatecollectIndexFailureText;indexFailureTextnow joins the chain.classifyIndexFailure's body is unchanged — the walk did not need it; only its doc comment gained a "Why both arms walkcause" note carrying the consumer-divergence rationale.packages/metadata-protocol/src/migrations/partial-index-probe.test.ts— six new cases: wrapped refusal, arm order across the chain, depth parity, newline separator, cycle termination, end-to-end throughprobeThenReplaceIndex.packages/metadata-protocol/src/migrations/overlay-index.test.ts— one added test, test-only, no source change. It pins the consequence the card names (a pooled wrapper reachingfallback: 'ensured'instead ofnot-attempted). Without it the PR proves the verdict moved but not that the fallback index is actually reached again..changeset/index-failure-dialect-cause-walk.md— patch for@objectstack/metadata-protocol;classifyIndexFailureis a public export, so the change is user-visible.Not touched, per the dispatch:
protocol.ts,packages/spec,@objectstack/types,content/docs/releases/. No STOP was reached.One judgement call worth a reviewer's eye
Levels join with a newline, never a space. Two dialect alternatives are multi-word (
where clause,near "where"), so a space would synthesise a refusal across a wrapper boundary that no driver ever wrote. There is a test that goes red if the separator is changed to a space. Level-0 behaviour is byte-identical to the old body for every previously reachable shape, so no existing verdict moves.Reverse verification — predictions written BEFORE running, then measured
Method:
git checkout origin/main -- …/partial-index-probe.ts(fix out, new tests kept), re-run, restore. Nogit stashat any point.Predicted red: wrapped-refusal fixture, depth-parity pin, cyclic-chain refusal case, end-to-end probe case, overlay-index fallback case. Measured: exactly those five, no others.
The parity pin's output states the defect most directly — the dialect arm reaching depth 0 only, against the conflict arm's 0..4:
Two assertions were recorded up front as green-in-both-directions and are reported as guards, not evidence — they constrain choices inside the new walk, which do not exist before the fix:
One prediction went unmeasured and is not claimed. The parity test's second assertion (literal profile
[T,T,T,T,T,F,F]) never executed on the revert run — the parity equality above it failed first and short-circuited the test. Green with the fix in; its predicted red is unverified.CI — per-job conclusions on
41d9415All 25 check runs
completed. No red anywhere; the two non-successes are path-filter skips.Labels read back after the bots settled:
documentation,size/m,tests,tooling.skip-changesetdeliberately not applied — this PR ships a changeset and Check Changeset passed on it.Local verification
pnpm --filter @objectstack/metadata-protocol test— 65 files, 820 tests passed (814 pre-existing + 6 new).pnpm exec eslinton all three changed files — clean, exit 0.node scripts/check-nul-bytes.mjs— OK, 6467 files; plus a self-scan of the changed files across the wider control-byte range, no hits.classifyIndexFailure/ensureOverlayStateIndex/PartialIndexStatushave no code consumers outsidemetadata-protocol; the three hits inpackages/metadataare prose in comments.Could not verify / flagged rather than omitted
tsc --noEmitonmetadata-protocolreports 159 errors, 0 of which name any file in this PR — all pre-existing debt in unrelatedseed-loader-*/sys-metadata-repository.*tests. That is why the package carries notypecheckscript and sits in the type-check-debt ledger, so no per-package tsc gate reads my files directly; the type coverage they do get is thetsupDTS build (green) and the repo-wide TypeScript Type Check job (green). Recorded so the gap is visible rather than implied by a green tick.Out-of-scope findings
None. Nothing outside the card's surface was tripped over, so nothing was filed.
Generated by Claude Code