Repository navigation
[PR] fix(storage): normalize roleless rows so unified ingest stops throwing on node:sqlite (#4407) - #5261
[PR] fix(storage): normalize roleless rows so unified ingest stops throwing on node:sqlite (#4407)#5261ranxianglei wants to merge 2 commits into
Conversation
ranxianglei
changed the title
30e4196 to
6e39a95
Compare
📦 Built Package ArtifactBranch: Latest build — install this exact versionnpm install -g billion-context@0.0.9178-pr.5261.5d22108This exact version is immutable and reproducible. The rolling Recent builds (commit → version)
Download artifact
tar xzf billion-context-pr5261.tgz
npm install -g packageInstall instructions are refreshed on each push. |
3ff9e40 to
af27bc4
Compare
af27bc4 to
cca1f46
Compare
(ai: vllm-qwen/qwen3.8-27b) Conflict resolution complete — new head What I did this run:
七问:
收工判定:冲突已清、可合并状态已恢复,等 CI 全绿后即可由人工合并。 |
(ai: vllm-qwen/qwen3.8-27b)
What I did this run:
七问:
收工判定:修复已推、本地门禁全绿,远端新一轮 check 进行中(当前 0 红);若后续再红由下一次唤醒继续处理。 |
(ai: vllm-qwen/qwen3.8-27b) Closing status — both blockers cleared, full matrix green. Live mirror-cache read this run (head Session total for this PR:
Note: merge state currently reads 收工判定:冲突已清、CI 全绿(30/30),PR 处于可合并状态,等人工合并。 |
(ai: vllm-qwen/qwen3.8-27b) Progress report after forced process restart (session had gone idle after completing the work — nothing was mid-flight; no lost progress). Re-verified LIVE in this run (mirror cache fetched 15:44Z):
No open work, no blockers, nothing waiting on me. PR is ready for human merge. 收工判定:进程重启后已重新实测——CI 全绿(30/30)、无冲突,仅因 master 前进呈 behind 状态(不阻塞合并);等人工合并即可。 |
…g on node:sqlite (#4407) A stored message whose view row carries no `role` field reached the sqlite insert as undefined and threw "Provided value cannot be bound to SQLite parameter 6", latching the process into degraded mode after three saves on the dsh web lane. Normalize the role column at the source (both ingesters) and at every bind site (sqlite + mysql insert/restore): a roleless row stores the sentinel "unknown" in the NOT NULL column while its verbatim content bytes stay untouched — every wire/render/recovery reader re-parses content and drops non-string-role rows, so the sentinel never reaches the model line. The verify projection compares the re-parsed sentinel against the stored one so roleless rows are not flagged corrupt. Insert errors now carry session_id/seq/msg_id so the degrade relay names the failing row. Regression test: tests/issue4407-roleless-bind.test.ts — red pre-fix with the exact param-6 TypeError, green post-fix.
d8be2b8 to
155b375
Compare
(ai: vllm-qwen/qwen3.8-27b) Merge-state blocker handled — branch rebased onto current master, new head What I found and did this run:
The previously-green run on 七问:
收工判定:分支已对齐当前 master、内容经多重验证,等新一轮 CI 跑完即可合并;若再红由下次唤醒处理。 |
There was a problem hiding this comment.
(ai: vllm-qwen/qwen3.8-27b)
Verdict: APPROVE — posted as a comment review because GitHub refuses a formal approval from the PR’s own account (author == reviewer here).
Independent end-to-end review of head 155b375cf8 (the branch sits directly on current master c26c31ace4; no base drift). Mirror cache at this sha: 30/30 checks, 0 failed (12 skipped by design).
Verified myself rather than taking the author's word:
- Red→green reproduced locally: pre-fix sources fail the new regression test with the byte-exact field error (
TypeError: Provided value cannot be bound to SQLite parameter 6.); PR head passes 1/1.tsc --noEmitclean; unused-exports gate clean (12 waived). - Root cause, not a mask: both planners normalize
parsed.roleat the source; all six messages bind sites (sqlite+mysql ingest/restore) and both verify-projection comparisons use the SAME helper, so the stored column and the re-parsed content can never disagree. The issue's suggestedrole ?? nullwas correctly rejected —roleisNOT NULLin both DDLs (schema.ts:162, mysql.ts:160), so null would just trade the TypeError for a constraint failure. - Sentinel never reaches the model line: independently confirmed both reader guards — src/server/snapshot-render.ts:127 and src/persist.ts:4404 drop non-string-role rows after re-parsing the verbatim content; the test pins that content bytes stay role-less.
- Fail-fast preserved: insert errors rethrow with row identity, original message kept as a suffix; no production consumer matches the old error string (shadow dedup/lastError gets strictly better context).
- No residual same-class gap: blocks start_ref/end_ref are planner-normalized to
number | null(ingest.ts:422-423); the new sqliteref/refNum/kinddefaults align it with the guards mysql already had (MessageRow types these non-optional, so typed callers see zero change); the MySQL NOT NULL DDL means no legacy NULL-role rows can exist for verify to misflag. - The test has teeth: real builtin node:sqlite engine, real
planIngest → openBackend → ingestSession; asserts no throw, both rows land, verbatim content (no role key), sentinel column, real role preserved, verifyProjections ok. A regression of this bug class goes red.
One accuracy note (non-blocking, no code impact): the PR body's "latches off until restart" describes the reporter's 0.1.204 build. On current master the amplifier already changed (#5627 removed self-disable — pinned by tests/issue5627-storage-failure-forensics.test.ts:116), so on master the user-visible delta is: every save of an affected session rolled back its ENTIRE sidecar transaction (unified-only features dead for that session — the cold-load ~436k-token replay from #4407 floor 2) → those saves now commit. Root cause and fix are identical either way; the disclosure stands for the reported version.
Scope/conventions: host-side src/storage only — no kernel/, no config surface, no schema/wire/persistence-format change, both compression modes unaffected (storage sits below the wire), clean diff (4 files, +131/−17, every line on-purpose), single linear history on current master. §7.4 auto-merge criteria satisfied; human merges as usual.
[bot] (ai: vllm-qwen/qwen3.8-27b) Review complete — verdict: APPROVE, posted on GitHub PR #5261 as a comment review (review link). One mechanical caveat: GitHub refuses a formal approval from the PR's own account (author == reviewer here), so What I did independently (not reusing prior reviews):
No small findings needed an on-the-spot fix commit; nothing routed to needs-decision. One accuracy note on the PR body (non-blocking, no code impact): its "latches off until restart" wording describes the reporter's 0.1.204 build. On current master the failure amplifier already changed (#5627 removed self-disable — pinned by tests/issue5627-storage-failure-forensics.test.ts:116), so on master the user-visible delta is narrower but real: every save of an affected session rolled back its ENTIRE sidecar transaction (unified-only features dead for that session — the cold-load ~436k-token replay from #4407 floor 2) → those saves now commit. Root cause and fix are identical either way; the disclosure stands for the reported version. 八问
收工判定:代码通过、CI 30/30 绿、无冲突、无待决策项——PR 处于可合并状态,等 owner 人工合并(GitHub PR #5261)。 |
Closes #4407
What
On the dsh web lane, a stored message whose view row has no
rolefield made the unified store'smessagesinsert bindundefinedinto aNOT NULLcolumn. node:sqlite rejects that withTypeError: Provided value cannot be bound to SQLite parameter 6.; three consecutive save failures then latched the process into "unified store auto-disabled" (degraded) until restart — same amplifier as #4341, different trigger (wrong-type bind, not a lock).Fix
Normalize the role column at the source (both ingesters) and at every bind site (sqlite + mysql insert/restore/verify): a roleless row stores the sentinel
"unknown"in the NOT NULL column while its verbatim content bytes stay untouched. Every wire/render/recovery reader re-parses content and drops non-string-role rows, so the sentinel never reaches the model line. Insert errors now carry session_id/seq/msg_id so the degrade relay names the failing row.Behavior change (old → new)
role="unknown"in the metadata column (content verbatim unchanged); it is dropped by every reader exactly as before, and the store no longer latches. No currently-working session is affected (they have no roleless rows).Tests
tests/issue4407-roleless-bind.test.ts: red pre-fix with the exact param-6 TypeError, green post-fix. Asserts no throw, sentinel column, verbatim content, real roles preserved, and projection verify passes.Out of scope
Per-row quarantine instead of the whole-store latch (suggestion #3 in the issue) changes shared failure-amplifier semantics with #4341 and is a separate behavior decision — not changed here.