refactor(core): tail migrations — crush, zcode, zed, forge, goose (phase 8, sqlite batch 1) - #832
Merged
Merged
Conversation
…ase 8, sqlite batch 1)
Category B (sqlite) variant of the bridge migration: the sqlite driver and every
SQL query stay CLI-side. Each provider's `readRecords` opens the database, runs
the same queries as before, and hands the resulting rows (blob and all) to a
pure core decoder; `toProviderCall` maps the rich, cost-free decode back onto
ParsedProviderCall, where cost re-enters via the parser.ts pricing pass.
Per provider:
- crush: session row + dominant-model query -> one combined record. Crush stores
cost in dollars, so a row with cost > 0 carries `measuredCostUSD` (costBasis
'measured'); a zero-cost row falls back to token estimation, arm order intact.
- zcode: model_usage + tool_usage row sets -> one composite record. Each turn's
tools still attach to the first non-skipped usage row of that turn only.
- zed: threads rows handed over compressed; zstd decompression, JSON parsing and
per-request/cumulative-remainder accounting are pure. The Node >= 22.15 zstd
capability check stays host-side.
- forge: conversation row handed over with `context` still serialized; JSON
parsing and per-message decode are pure. Bash base-name extraction (and its
strip-ansi dependency) stays CLI-side over the decoder's raw command strings.
- goose: session + assistant tool-message + first-user-message rows, BLOB
columns pre-converted to text host-side, bundled into one composite record.
Validator fixes (original behavior is the authority):
- forge: the draft replaced the pre-migration `mapToolName` switch with an
object-literal lookup. Tool names come straight from conversation JSON, so
names colliding with Object.prototype members ("constructor", "toString",
"__proto__", "hasOwnProperty") resolved to inherited Functions / the prototype
object and were pushed into `tools` as non-strings instead of falling through
to the identity default. Restored the switch and pinned the arm in the fixture.
- zed: the draft routed the "skipped N unreadable Zed threads" notice into
record diagnostics, which the bridge discards, silently dropping a warning the
pre-migration decode printed. Re-emitted host-side from the diagnostics count
and pinned with a stderr assertion.
- Fixture coverage extended for the arms that were regression-blind: forge's
prototype-named tool calls, zed's aggregate stderr line, and goose's
single-turn `toolSequence` omission plus the unparseable-timestamp fallback.
Parity was verified independently of the bridge tests with a git-show harness
that runs the same fixtures through the pre-migration provider files and asserts
field-for-field equality, including the extra arms above.
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.
Phase 8 sqlite batch 1 (tracking #809): migrates the row→call decode for crush, zcode, zed, forge, goose into
@codeburn/coreper the recipe's sqlite variant — the sqlite driver,openDatabase, and all SQL queries stay CLI-side inreadRecords; only pure row decode, row types, and tool-name maps move.Two-tier protocol: cheap-tier draft, then independent arm-by-arm validation against the pre-migration originals with a git-show harness. The validator caught and fixed two real defects the green suites missed:
switchinto aRecordlookup, introducing a prototype-member leak (constructor/toString/__proto__resolved to inherited values, landing non-strings intools). Restored the originalswitchverbatim; pinned with a hostile fixture.skipped N unreadable Zed threadsstderr warning was silently dropped (bridge discards decode diagnostics). Re-emitted host-side via a decode wrapper; byte-identical to the original.Adjudicated deviations (accepted, output-equivalent, harness-verified): goose's clock fallback moved to the host boundary (
''from core,|| new Date()intoProviderCall); crush'smeasuredCostUSDcarrier with exact original arm order and> 0threshold.Goldens: full-object
toEqualpins captured from the unmodified providers, independently re-derived via the git-show harness (8/8), including arms the draft fixtures didn't exercise (crush cost-only/child-session rows, zcode skip-slot semantics, zed remainder/no-model rows, forge positional-stableId fallbacks).Gates: core 233/233; CLI suite (from packages/cli) and root
npm testboth 2327 passed, 5 skipped;tsc --noEmitclean in both packages. Arch-gate allowlist: zed/forge/goose only (verified crush/zcode carry no userMessage). No parse-version changes.Known follow-up (pre-existing, untouched):
gooseToolNameMaphas the sameRecordprototype hazard as the forge bug, but it predates this migration.