fix(prompt-surface): let the generator own the A1 golden's hash baseline - #292
Conversation
Closes cortexkit#290. prompt-surface-a1-golden.md says it is generated by export-agent-surface.ts, but the script emitted only sections 1 and 2. Section 3 (the system-prompt hash baseline) was hand-maintained, so running the script the way its own usage line documents deleted it -- exit 0, no warning -- and system-prompt-hash.test.ts then threw "Malformed A1 primary guidance golden". The generator now emits section 3. The table is DERIVED, not transcribed: the hash handler persists the MD5 of output.system.join("\n"), and in the no-host-prefix baseline each guidance section IS the whole system array, so the hash is just the MD5 of the guidance bytes already emitted in section 1. Verified by regenerating over the committed golden -- the output is byte-identical apart from the date stamp, including all four hash rows. Also hardens the two consumers that slice on that heading (tool-registry.test.ts and pi-plugin/src/tools/index.test.ts). Both did document.slice(indexOf("## 2. Tool surface"), indexOf("## 3. ...")) which yields -1 for a missing heading, so the slice silently returned "" and the golden parsed as ZERO tools -- and the tests still PASSED, comparing empty to empty. That was the worse half of the trap: one loud error while every other reader quietly degraded. Both now resolve offsets through a sectionOffset() helper that throws a named error. Red-checked: with section 3 stripped from the golden, both call sites now fail with `A1 golden is missing the "## 3. System-prompt hash baseline" section heading`. On master the same mutation leaves them green. Gates: plugin 3609/3, pi 740/0, cli 296 (2 skip), typecheck 0 x3, lint clean. The 3 plugin failures are pre-existing @OpenTui TDZ errors in tui-compiled-runtime-imports.test.ts, unrelated to this change (it touches no TUI file) and reproducible on a clean upstream/master worktree.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 3 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…ed module Addresses cubic's P3 on cortexkit#292: the sectionOffset() guard was duplicated verbatim across tool-registry.test.ts and pi-plugin/src/tools/index.test.ts, both parsing the same golden. New src/shared/prompt-surface-a1-golden.ts owns three things that were previously copy-pasted per package: the two section-heading strings, the document read, and the offset guard. Pi already consumes plugin/src through the @magic-context/core/* tsconfig alias, so this is the established seam rather than a new dependency. The module resolves the golden relative to ITSELF, which removes the per-package `../..` arithmetic (`../shared/...` on OpenCode, `../../../plugin/src/shared/...` on Pi). That arithmetic was the more dangerous half of the duplication: it is invisible to the type checker and rots silently when files move. Scope note: the two readA1PrimaryGuidance() readers are deliberately left alone. They regex the whole document rather than slicing between headings, so they were never exposed to the -1 bug this PR fixes, and folding them in would widen an unrelated PR. The heading constants are exported and ready if a later change wants them. Re-verified the red-check through the shared helper -- with section 3 stripped, both consumers still fail with the named error: A1 golden is missing the "## 3. System-prompt hash baseline" section heading Gates: plugin 3609/3, pi 740/0, cli 296 (2 skip), typecheck 0 x3, lint clean. Same 3 pre-existing @OpenTui TDZ failures as the parent commit; this touches no TUI file.
|
@cubic-dev-ai Valid — fixed in New One thing your report understated, and it is the better half of the fix: the duplicated path arithmetic was the more fragile part. The two readers resolved the same file as Deliberately left out of scope: the two Re-ran the red-check through the shared helper — with section 3 stripped from the golden, both consumers still fail with the named error: Gates unchanged: plugin 3609/3, pi 740/0, cli 296, typecheck 0 ×3, lint clean. The 3 are the pre-existing |
@iceteaSA Right on both counts — the shared module is the stronger fix, and One thing worth a quick sanity check on your side, since your Otherwise the scoping is exactly what I'd want: the heading constants exported for later consolidation, the red-check re-run through the shared helper, and no widening into the regex-based readers. Tip: get faster answers by chatting with cubic’s review copilot in the review UI |
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/plugin/src/shared/prompt-surface-a1-golden.ts">
<violation number="1" location="packages/plugin/src/shared/prompt-surface-a1-golden.ts:19">
P3: The `A1_TOOL_SECTION_HEADING` constant doesn't mirror the actual golden heading — the file declares `## 2. Tool surface (description + parameters as serialized to the provider)`, while the constant only carries the `## 2. Tool surface` prefix. It works only because `indexOf` does a prefix/substring match, which contradicts the module's own stated contract (that these heading strings make a golden format change a one-file edit). If the golden ever gains a section whose heading starts with `## 2. Tool surface`, the offset guard would silently resolve to the wrong section. Suggest storing the exact on-disk heading so the constant truly reflects the contract it documents.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| * exactly the kind of thing that rots silently when files move. | ||
| */ | ||
|
|
||
| export const A1_TOOL_SECTION_HEADING = "## 2. Tool surface"; |
There was a problem hiding this comment.
P3: The A1_TOOL_SECTION_HEADING constant doesn't mirror the actual golden heading — the file declares ## 2. Tool surface (description + parameters as serialized to the provider), while the constant only carries the ## 2. Tool surface prefix. It works only because indexOf does a prefix/substring match, which contradicts the module's own stated contract (that these heading strings make a golden format change a one-file edit). If the golden ever gains a section whose heading starts with ## 2. Tool surface, the offset guard would silently resolve to the wrong section. Suggest storing the exact on-disk heading so the constant truly reflects the contract it documents.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/plugin/src/shared/prompt-surface-a1-golden.ts, line 19:
<comment>The `A1_TOOL_SECTION_HEADING` constant doesn't mirror the actual golden heading — the file declares `## 2. Tool surface (description + parameters as serialized to the provider)`, while the constant only carries the `## 2. Tool surface` prefix. It works only because `indexOf` does a prefix/substring match, which contradicts the module's own stated contract (that these heading strings make a golden format change a one-file edit). If the golden ever gains a section whose heading starts with `## 2. Tool surface`, the offset guard would silently resolve to the wrong section. Suggest storing the exact on-disk heading so the constant truly reflects the contract it documents.</comment>
<file context>
@@ -0,0 +1,41 @@
+ * exactly the kind of thing that rots silently when files move.
+ */
+
+export const A1_TOOL_SECTION_HEADING = "## 2. Tool surface";
+export const A1_HASH_BASELINE_HEADING = "## 3. System-prompt hash baseline";
+
</file context>
Third union since the fork-lane move; migrations.ts and storage-db.ts merged with zero conflicts again. upstream <= 75 byte-identical to upstream fork 10000-2 skill-memory P1 / P2 / P3a fork 10100 external-memory recall snapshot LATEST_SUPPORTED_VERSION = 75 Upstream merged our cortexkit#292 (639d80e), so the section-3 generator fix and the shared prompt-surface-a1-golden.ts module are now upstream's. Both feature branches dropped their local ports during rebase and took upstream's version; skill-memory-pr re-applies only its two tool registrations on top. Of the 11 conflicts, 8 had byte-identical parents versus the previous union and were restored verbatim. STRUCTURE.md was the only genuine merge (3 TAKE-BOTH regions: features overview, inject-compartments, and the per-file lists), because upstream also added dreamer/provider-output-failure.ts to the same lists this cycle -- so it is a three-way take, not two. Budget fixture: kept the previous union's numbers (baseline 4193, ceiling 2096) after confirming they still match this tree's measurement exactly. The staged merge result had silently taken skill-memory-pr's 4087, which measures a tree that does not exist -- neither parent's fixture is valid for the union, which is why this file gets re-measured rather than picked every cycle. Gates: plugin 3908/2, pi 745/0, cli 298 (2 skip), typecheck 0 x3, lint clean, budget check green, fork-lane coexistence 25/0, tui-compiled regenerated, marker sweep clean, externalRecallHash still 0x inside mustMaterialize. The 2 plugin failures are upstream's own @OpenTui TDZ errors.
2 participants
Closes #290. Both items you asked for, in one commit off clean
master@77c51da9.1. The generator owns section 3
export-agent-surface.tsemitted only sections 1 and 2, so the hash baseline table was hand-maintained and regenerating the golden deleted it.The table is now derived rather than transcribed:
Buffer.byteLength(text, "utf8")andcreateHash("md5").update(text)over the samevariantsarray section 1 already walks.Verified by regenerating over the committed golden — output is byte-identical apart from the date stamp, all four hash rows included. One detail worth flagging: the committed prose contains
output.system.join("\\n")(escaped backslash-n, not a newline). My first pass emitted\nand the diff caught it; the emitted string now reproduces the committed bytes exactly. The golden itself is untouched in this PR — the fix is the generator, and a date-only churn would just be noise.2. Both slice call sites now throw
tool-registry.test.tsandpi-plugin/src/tools/index.test.tsboth did:-1for a missing heading, so the slice returned"", the golden parsed as zero tools, and the tests passed comparing empty to empty. Agreed this is the half that hides the next regression. Both resolve offsets through asectionOffset()helper that throws a named error.Red-check
With section 3 stripped from the golden:
Same mutation on
master: both green. That gap is the bug, and it is now closed on both hosts.Gates
† Pre-existing
@opentuiTDZ errors intui-compiled-runtime-imports.test.ts(Cannot access 'TreeSitterClient' before initialization). Unrelated to this change — it touches no TUI file — and reproducible on a cleanupstream/masterworktree with a fresh install. Your CI is green on master, so this looks local to my environment rather than something you need to act on; mentioning it only so the count is not mistaken for a regression here.Files:
export-agent-surface.ts,tool-registry.test.ts,pi-plugin/src/tools/index.test.ts.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Owns the A1 golden’s “System-prompt hash baseline” in the generator and makes tests fail when the section is missing. Prevents silent golden truncation and false green runs; also centralizes golden accessors to remove duplicated parsing and path math.
Bug Fixes
packages/plugin/scripts/export-agent-surface.tsnow emits section 3 by deriving the table from the guidance text (bytes viaBuffer.byteLength(..., "utf8"), MD5 viacreateHash("md5")). Output matches the current golden (date-only diff).packages/plugin/src/plugin/tool-registry.test.tsandpackages/pi-plugin/src/tools/index.test.tsnow throw when required section headings are missing, replacing the silentslice(...)behavior that produced empty tool sets.Refactors
packages/plugin/src/shared/prompt-surface-a1-golden.tsexportingA1_TOOL_SECTION_HEADING,A1_HASH_BASELINE_HEADING,readA1GoldenDocument, anda1GoldenSectionOffset. Both test suites now import these (Pi via@magic-context/core/shared/prompt-surface-a1-golden) to dedupe logic and ensure consistent guards.Written for commit ceb4cb0. Summary will update on new commits.
Greptile Summary
The PR makes the prompt-surface generator produce the system-prompt hash baseline and prevents malformed golden sections from silently yielding empty comparisons.
Confidence Score: 5/5
The PR appears safe to merge.
No blocking failure remains.
Important Files Changed
Reviews (2): Last reviewed commit: "refactor(prompt-surface): extract the A1..." | Re-trigger Greptile