Skip to content

test(qwen): pin the orphaned-record dedup key shape - #936

Draft
ozymandiashh wants to merge 1 commit into
getagentseal:feat/core-extractionfrom
ozymandiashh:fix/qwen-dedup
Draft

test(qwen): pin the orphaned-record dedup key shape#936
ozymandiashh wants to merge 1 commit into
getagentseal:feat/core-extractionfrom
ozymandiashh:fix/qwen-dedup

Conversation

@ozymandiashh

Copy link
Copy Markdown
Collaborator

The smallest item from the review pass, and the one that says the most about the extraction's test coverage.

The pre-extraction decoder built dedup keys with template interpolation, so a record missing sessionId or uuid produced the literal qwen:undefined:<uuid>. Phase 8.1 changed the spelling to qwen::<uuid> by coalescing both fields to an empty string.

The intent was right — a missing identifier should contribute nothing rather than a fake value. But the change shipped with nothing behind it: no test, no fixture, no golden pinned either spelling, and the CLI parity golden only exercises fully-formed records, where the two are byte-identical. Nobody could tell the change from a defect. That is what this PR fixes — not the shape, the absence of a contract.

The test pins the orphan spellings, asserts the collapsed key still dedups identical records, and fails against the pre-migration spelling (verified red: qwen:undefined:a-orphan against the pinned qwen::a-orphan). The decode-site comment records the reasoning so nobody "restores" the old one.

Two caveats, worth knowing rather than discovering

  • Records with an explicit null coalesce the same way as missing ones, so two previously-distinct keys now collapse into one.
  • Keys persisted by pre-8.1 builds for orphaned records will not match the new spelling, so such a record can be counted once more across the upgrade. The qwen corpus is not in the frozen golden set, so no shipped fixture moves.

The pre-extraction decoder built dedup keys with template interpolation, so
a record missing sessionId or uuid produced the literal `qwen:undefined:<uuid>`.
Phase 8.1 changed the spelling to `qwen::<uuid>` by coalescing both fields to
an empty string. The intent was right — a missing identifier should contribute
nothing rather than a fake value — but the change shipped with nothing behind
it: no test, fixture or golden pinned either spelling, and the CLI parity
golden only exercises fully-formed records, where the two are byte-identical.
Nobody could tell the change from a defect.

This makes the corrected shape a tested contract. The test pins the orphan
spellings, asserts the collapsed key still dedups identical records, and fails
against the pre-migration spelling. The decode-site comment records why, so
nobody "restores" the old one.

Two caveats worth knowing rather than discovering. Records with an explicit
null coalesce the same way as missing ones, so two previously-distinct keys
now collapse into one. And keys persisted by pre-8.1 builds for orphaned
records will not match the new spelling, so such a record can be counted once
more across the upgrade — the qwen corpus is not in the frozen golden set, so
no shipped fixture moves.
@ozymandiashh
ozymandiashh marked this pull request as draft August 5, 2026 10:04
@ozymandiashh

Copy link
Copy Markdown
Collaborator Author

Converted to draft: this was opened with only a self-review by the model that wrote it, which is not the review bar this batch is held to. Independent review by two other reviewers is running now; I will mark it ready once both have passed and any findings are applied. Apologies for the noise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant