Skip to content

Agent guidance contains verbatim aichat-matrix prompts — the eval is measuring recall of planted examples, not generalization - #1763

Merged
malibio merged 7 commits into
mainfrom
worktree-issue-1760-decontaminate-guidance
Jul 28, 2026
Merged

malibio merged 7 commits into
mainfrom
worktree-issue-1760-decontaminate-guidance

Conversation

@malibio

@malibio malibio commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Closes #1760

…LEVANT ENTITY TYPES (closes #1760)

The agent's compiled guidance contained verbatim prompt strings from
scripts/aichat-matrix.ts alongside the exact tool call each should
produce, so the eval could not distinguish generalization from recall
of a planted example.

Guidance (packages/agent/src/agent_guidance.rs):
- Replaced the three memorized examples (scenarios 5, 6, 8d) and the
  "contacts database" echo of 8b with rules that teach the shape.
- Removed the hardcoded node_type="invoice" literal. It was also simply
  wrong: schemas named from "an invoice tracking database" normalize to
  invoice_tracker, so the model copied the example and the lookup missed.
  Had the literal been correct, scenario 6 would have gone green while
  proving nothing.
- Added NODE_TYPE COMES FROM RELEVANT ENTITY TYPES: node_type arguments
  must be copied exactly from the injected entity-types block. No rule
  previously stated where those values come from, which is the actual
  root cause behind scenario 6 rather than the bad literal alone.

Eval (scripts/aichat-matrix.ts):
- Reworded every scenario off the invoice/book/contact domains to
  re-establish an independent signal. Scenario IDs and expectations are
  unchanged so baseline diffing still works.

Mechanical check:
- guidance_is_not_contaminated_by_eval_prompts parses prompt literals out
  of both eval scripts at test time and fails if any shares more than
  four consecutive words with a guidance constant, catching near-verbatim
  reuse rather than only exact substrings. It reads the real files rather
  than a copy so it cannot silently drift, and asserts it parsed a
  non-empty prompt set so it cannot pass vacuously. Verified to fail on a
  re-planted example. Runs under rust:test, which the pre-push gate
  enforces.

scripts/results-e4b.json still records 11/12 from a contaminated run and
is left untouched; it needs re-recording against a running daemon before
it is comparable, and ADR-056's scenario 5 claim revisited with it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DkqjrfV1G6kugsEjvyU9wU
@malibio

malibio commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

Pragmatic Code Review — PR #1763

Review Type: Initial Review
Scope: packages/agent/src/agent_guidance.rs, scripts/aichat-matrix.ts (2 files, +246/-17)


Summary

This is a net-positive change that correctly diagnoses a subtle and genuinely damaging problem. Contaminated evals are worse than no evals — they produce confident numbers that measure the wrong thing — and the analysis in both the issue and the commit message is unusually sharp. The observation that scenario 6 failed because the planted answer was wrong, and that fixing the literal to invoice_tracker would have turned it green while proving nothing, is exactly the right read. The NODE_TYPE COMES FROM RELEVANT ENTITY TYPES rule addresses the actual root cause rather than the symptom, and adding a mechanical guard rather than relying on discipline is the correct instinct for a failure mode that recurs precisely when you're least likely to notice it.

The guard's design shows real care: reading the eval files at test time rather than keeping a copy, matching on shared word-runs rather than exact substrings, the vacuity assertion, and a pinned regression test against the original contamination. That is a higher standard than most anti-regression tests are held to.

My concerns are about completeness of the guard's coverage, not its correctness. The central finding: the guard omits skill_pipeline.rs, and there is live, verifiable contamination sitting there right now that this PR does not catch. Because the guard is the mechanism that is supposed to make this class of bug impossible going forward, a coverage gap in it is worth more than the guidance rewrite itself. There is also a scenario prompt that currently sits exactly at the tolerance threshold with zero headroom.


Requirements Check

# Acceptance Criterion Status Notes
1 No eval scenario prompt appears verbatim/near-verbatim in agent_guidance.rs or skill_rules.rs ⚠️ Mostly met Met for the three named overlaps (lines 22/49/53) and for skill_rules.rs. But scenario 2's prompt "What can you do?" still shares 4 consecutive words with TOOL_STRATEGY_RULES (capability questions (\"what can you do?\"), line 37) — sitting exactly at MAX_SHARED_RUN, passing only because the comparison is > not >=. See Finding 2.
2 TOOL_STRATEGY_RULES teaches node_type resolution as a rule referencing RELEVANT ENTITY TYPES, no hardcoded literal ✅ Met The new rule is well written — it names all four consuming tools, says why the user's noun is not the id, and covers the "not listed ⇒ doesn't exist" case. tool_strategy_rules_bind_node_type_to_relevant_entity_types pins it.
3 Eval scenario prompts reworded so they don't share distinctive phrasing with guidance ✅ Met for aichat-matrix.ts All 12 rewritten; measured max shared run against guidance is now 3 words (excl. scenario 2). routing-eval.ts was not reworded and still contains the original "Create an invoice tracking database" and "Add an invoice for $500 due next Friday" — arguably out of scope for AC-3 as written, but it is in the guard's scan set, which makes Finding 1 fire.
4 An automated check fails the build if an eval prompt reappears in guidance text ⚠️ Partially met The check exists, runs under rust:test (pre-push enforced), and was verified to fail on a re-planted example. But it does not cover all guidance that reaches the model — see Finding 1, which is a live miss, not a hypothetical one.
5 Matrix re-run, new score recorded as honest baseline ⏸ Deferred Documented in the commit message and on the issue; requires a live daemon + model run. Acknowledged as a known deferral. Flagging only so it isn't lost: until it happens, scripts/results-e4b.json records a number that this PR has just invalidated, and ADR-056's scenario 5 claim still rests on it. Worth a tracking issue rather than trusting the commit message to be re-read.

Code Review Findings

🔴 Critical

1. The guard misses skill_pipeline.rs — and there is live contamination there today

📁 packages/agent/src/agent_guidance.rs:194-212 (guidance_corpus())
📁 packages/agent/src/skill_pipeline.rs:305 (the actual contamination)

guidance_corpus() covers the three agent_guidance constants plus skill_rules imperative fields. It does not cover the markdown_content guidance blocks in skill_pipeline.rs, which are seeded into the DB as prompt nodes and injected as skill instructions — unambiguously guidance that reaches the model, by the same definition the corpus doc-comment uses ("Every guidance string that reaches the model").

The Node Creation skill contains:

EXAMPLE — create an invoice for $500 due next Friday (schema_metadata type_id="invoice"):
{
  "node_type": "invoice",
  ...
}

That is routing-eval.ts:187's prompt ("Add an invoice for $500 due next Friday") verbatim, paired with the exact create_node call it should produce — a 7-word shared run, well over the threshold. It is the same defect pattern this PR was opened to eliminate, including the same hardcoded node_type="invoice" literal the PR correctly removed from TOOL_STRATEGY_RULES, and it is arguably worse because it also plants the property values (amount: 500, due_date) rather than just the tool name.

I confirmed by reimplementing the guard's algorithm and running it with skill_pipeline.rs added to the corpus:

run=7 prompt='Add an invoice for $500 due next Friday' shared='an invoice for 500 due next friday'

Recommendation: add skill_pipeline.rs's rendered guidance to guidance_corpus() (the seed_skill_nodes() / *_guidance() builders return String, so they can be called directly from the test — no parsing needed), then fix the resulting failure by rewriting the Node Creation example onto a domain that appears in no eval. Also consider prompt_assembler.rs:269/280/318 inline markdown_content seeds for completeness.

Principle: A guard that covers a subset of the invariant's surface provides false assurance, which is worse than no guard — it converts "we should check this" into "the build checks this." The corpus doc-comment ("Add new guidance constants here") is a manual-discipline instruction protecting against a manual-discipline failure, which is the same class of mistake being fixed. Where possible, derive the corpus from the thing that is actually assembled into the prompt rather than a hand-maintained list.


🟡 Important

2. Scenario 2 is at the threshold with zero headroom — > vs >= is load-bearing

📁 packages/agent/src/agent_guidance.rs:266 (MAX_SHARED_RUN), :37 (the guidance line), scripts/aichat-matrix.ts:227

TOOL_STRATEGY_RULES line 37 reads capability questions (\"what can you do?\"), and scenario 2's prompt is "What can you do?" — a 4-word shared run against a 4-word prompt. It is 100% of the prompt, planted next to the assertion (noTools) it's meant to produce. It passes only because the check is run > MAX_SHARED_RUN rather than >=.

This is a real instance of the contaminated pattern that the guard, as tuned, structurally cannot catch: a prompt of ≤4 words can never trip a >4 threshold, no matter how completely it is quoted in guidance. Scenarios 1 and 2 ("Hi there", "What can you do?") are both in this blind spot, and scenario 1's intent is also quoted (Greetings (\"hi\", \"hello\")).

Mitigating: these two scenarios test tool abstention, so contamination is far less consequential than for the CRUD scenarios — a model that says "no tools for greetings" because guidance told it so is arguably behaving correctly. That is why I'm rating this Important rather than Critical.

Recommendation: either (a) add a proportional rule alongside the absolute one — flag when the shared run covers the whole prompt (e.g. run == prompt_words.len() && run >= 3), which catches short-prompt contamination without touching the long-prompt tolerance; or (b) if these two are a deliberate accepted exception, encode that as a named allowlist with a comment explaining why abstention scenarios are exempt, so the exemption is a decision rather than an artifact of an off-by-one. Silently relying on > at an exact tie is the one outcome to avoid.

Principle: A threshold that a real case sits exactly on is not a tuned threshold — it's an accident waiting for someone to "clean up" a comparison operator. Make intent explicit.

3. eval_prompts() fails open on several plausible TS shapes

📁 packages/agent/src/agent_guidance.rs:216-243

I emulated the parser against realistic variants. Results:

Input shape Parsed Consequence
prompt: \Mark the $500 invoice as paid`` (template literal) (nothing) 🔴 silently skipped — contamination invisible
prompt: "She said \"hi\" now" (escaped quote) She said \ 🟡 truncated → weakened detection
prompt: "Mark the " + "$500 invoice as paid" (concat) Mark the 🟡 truncated → weakened detection
// prompt: "Mark the $500 invoice as paid" (commented) full string 🟢 harmless false-positive direction
prompt : "..." (space before colon) (nothing) 🟡 skipped

Every miss fails open (guard stays green while contamination exists), which is the wrong direction for a safety check. Template literals are the realistic one — a future author parameterizing a prompt (prompt: \Add ${n} invoices``) silently removes that scenario from the guard, and nothing signals it. The vacuity assertion does not help: it only requires the set to be non-empty, so losing 11 of 12 prompts still passes.

Recommendation: minimally, accept backticks in the quote match (one added char in the match) and count parsed prompts against the number of prompt: occurrences that look like literals, failing if they diverge. Stronger and cheaper long-term: have aichat-matrix.ts export its GROUPS and emit the prompt list as JSON via a tiny bun invocation or a checked-in generated fixture, so the Rust side consumes structured data instead of re-implementing a TS lexer. Hand-rolled parsers of another language's syntax drift by construction.

Principle: Safety checks must fail closed. When a parser cannot represent the full input grammar, assert on coverage (did I parse everything I should have?) rather than only on non-emptiness.

4. Scenario 6's rewritten prompt may no longer exercise resolve_query's reason for existing

📁 scripts/aichat-matrix.ts:247-251

Old: "Mark the $500 invoice as paid" → new: "The 2400 one came back — set it to returned", paired with instance-creation prompt "Log a laser cutter checked out on the 12th, replacement cost 2400".

This is a legitimately good rewrite for the indirect reference property the resolve_query rule describes — "a bare value such as an amount or code, without naming which field it belongs to" fits 2400 precisely, and it's arguably a harder, more honest test than the original.

Two risks worth weighing before treating the new score as the baseline:

  • "set it to returned" presumes the schema the model invented in scenario 3 has a status-like field with a returned value. Nothing constrains that — scenario 3's prompt is "I want to keep a record of the equipment my team checks out", which does not imply a returned state. If the model's schema has no such field, update_node may fail for a reason unrelated to the search-then-write behavior scenario 6 asserts. The old prompt had the same coupling (paid on an invoice schema) but "paid" is far more strongly implied by "invoice tracking" than "returned" is by "equipment checkout". Worth checking on the first honest run whether failures here are behavioral or schema-shape artifacts.
  • "The 2400 one" is a bare number with no unit. For a small model, disambiguating it from "the 12th" in the prior turn is a genuine additional load. That may be fine — but note the eval is now measuring something slightly harder than before, so a score drop won't be cleanly attributable to decontamination alone.

Recommendation: no change required, but on the honest re-run (AC-5), record why each scenario failed, not just pass/fail — otherwise the new baseline can't distinguish "decontamination revealed real weakness" from "the reworded prompt is harder." That distinction is the entire point of the re-run.

Principle: When you change both the measurement and the thing measured in one step, the delta is uninterpretable. Since AC-5 is deferred anyway, the mitigation is cheap: capture failure reasons on the first honest run.

Scenario 7 ("Do we have anything worth 90000 sitting out?") I checked and it looks sound — noRetry only requires search_nodes not to repeat, and an amount with no match reliably produces the empty result the scenario wants.


🟢 Suggestions

5. Nit: scenario 8's labels now actively contradict their prompts

📁 scripts/aichat-matrix.ts:259-283

"8c. Instance: book" has prompt "Put down Kind of Blue, it's by Miles Davis"; "8d. Instance: contact" creates a venue. The added header comment (lines 49-52) explaining that sub-labels name the role not the entity is a reasonable mitigation, and I agree with keeping the IDs stable for baseline diffing.

Still, a label that says "book" next to a prompt about an album is a small, permanent tax on every future reader, and the explanation lives ~200 lines away from the contradiction. Consider renaming to role-descriptive IDs (8c. Instance: first type, 8d. Instance: second type) in the same commit that re-records the baseline for AC-5 — at that point the baseline is being rewritten anyway, so ID stability costs nothing. Not worth churning the baseline for on its own.

Principle: A comment that explains why a name is wrong is weaker than a name that is right. Acceptable when renaming has a real cost; here that cost disappears at AC-5.

6. Nit: longest_shared_run is O(n·m) over full guidance text

📁 packages/agent/src/agent_guidance.rs:253-272

Nested loops with an inner walk across ~30 prompts × ~15 corpus entries × a few thousand words each. Fine in practice — it's a unit test and the inputs are small — but if the corpus grows (which Finding 1 will do), a suffix-automaton or even a simple 5-gram HashSet intersection would be both faster and simpler to read: build the set of 5-grams from guidance once, then check whether any prompt 5-gram hits it. That formulation also makes the threshold self-documenting (MAX_SHARED_RUN + 1-gram = violation). Optional.

7. Positive note: the guidance rewrite reads as behaviorally sound

📁 packages/agent/src/agent_guidance.rs:20-24, 50-54

I compared old vs new line by line for semantic drift, since removing concrete examples is the central risk of this change for a small model like Gemma E4B. My read is that the rewrites hold up, and in two places improve:

  • "DATABASE" = SCHEMA: loses the "a contacts database" → name="Contact" worked example but gains an explicit derivation rule ("singular form, stripped of the tracking wording itself") that the example only implied. That rule is what actually would have prevented the invoice_tracker mismatch, so this is a genuine upgrade, not just a redaction.
  • INSTANCE vs TYPE: replacing three example phrasings with the abstract cue ("supplies the particulars of ONE record — a name, an amount, a title, a date") is the riskiest single edit here, since pattern-matching on example phrasings is exactly what small models do well. It's partly compensated by the added closing clause ("Only a request to start tracking a KIND of thing calls for create_schema"), which sharpens the contrast the rule turns on. Watch scenarios 4/8c/8d on the honest re-run — if any regress, this line is the first suspect, and the fix is a fresh example on a domain absent from both eval files, not restoring the old one.
  • LIST EVERY NODE OF A TYPE: the added "the empty query is what makes it return the whole type, so do NOT invent a keyword to put there" is strictly better than the old (e.g. "list all my invoices", "show all tasks"). It states the mechanism instead of relying on the model to induce it from two samples.
  • resolve_query: the abstract characterization of indirect reference is faithful to the original, and the replacement of Use update_task_status ONLY for built-in task nodes — NOT for custom types like invoice, contact, book with for every custom schema type, use update_node is a strict improvement — the old form enumerated three types and left the general rule to inference.

No drift found that I'd ask you to revert.

8. Positive note: the supporting tests are well chosen

📁 packages/agent/src/agent_guidance.rs:133-158, 320-351

contamination_guard_detects_a_planted_example is the one that matters — testing that a detector fires is the assertion most guards omit, and pinning it against the specific historical contamination gives it durable meaning. eval_prompt_parser_extracts_literals correctly covers the two shapes the parser is meant to handle plus the fixture.prompt skip. tool_strategy_rules_bind_node_type_to_relevant_entity_types sensibly asserts both presence (the rule, all four tools) and absence (the literal).

One gap consistent with Finding 3: neither parser test covers a shape the parser fails on, so the failure modes are unpinned. Adding a #[test] documenting current template-literal behavior would at least make the limitation visible to the next reader.


Recommendation

REQUEST CHANGES — for Finding 1 only.

To be clear about the weighting: the guidance rewrite (AC-1/2/3) is good work and I'd merge it as-is. The reason I'm not approving is narrow and concrete — the guard is the durable deliverable here, it is the thing that makes AC-1 stay true after everyone forgets this PR existed, and it currently has a hole with live contamination already sitting in it (skill_pipeline.rs:305, a verbatim routing-eval.ts prompt beside its expected tool call and property values). Landing a guard that is green while the defect it targets exists elsewhere in the same crate sets exactly the wrong precedent for the next person tuning prompts.

Blocking:

  • Finding 1 — add skill_pipeline.rs guidance to guidance_corpus() and fix the contamination it surfaces.

Strongly recommended in the same PR (cheap, and each is a hole in the same mechanism):

  • Finding 2 — make the scenario-2 threshold tie an explicit decision, not an off-by-one.
  • Finding 3 — accept backticks and assert parse coverage so the guard fails closed.

Non-blocking: Findings 4-8. Finding 4's recommendation (capture failure reasons on the honest re-run) is worth carrying into the AC-5 follow-up — without it the new baseline won't be interpretable, which would undercut the point of this whole change.

AC-5's deferral is understood and reasonable; please open a tracking issue rather than relying on the commit message, since it also gates revisiting ADR-056's scenario 5 claim.

@malibio malibio left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recommendation: REQUEST CHANGES (submitted as a comment — GitHub does not permit requesting changes on your own PR). Full review posted above.

The guidance rewrite and the eval rewording are good work and I'd merge them as-is. The blocker is narrow: the guard's coverage. guidance_corpus() omits skill_pipeline.rs, and there is live contamination sitting there right now — skill_pipeline.rs:305 embeds routing-eval.ts:187's prompt verbatim ("an invoice for $500 due next Friday", a 7-word shared run) alongside the exact create_node call and property values it should produce, including the same hardcoded node_type="invoice" literal this PR correctly removed from TOOL_STRATEGY_RULES.

Since the guard is the durable deliverable — the thing that keeps AC-1 true after this PR is forgotten — landing it green while the defect it targets exists elsewhere in the same crate is the part worth fixing before merge.

Also recommended in the same pass (both holes in the same mechanism, both cheap): the scenario-2 threshold tie at exactly MAX_SHARED_RUN, and making eval_prompts() fail closed on template literals.

The guard shipped in the previous commit scanned only agent_guidance.rs
and skill_rules.rs. Review of PR #1763 found live contamination it did
not cover, in the same crate.

Coverage gap (the substantive finding):
- skill_pipeline.rs:307 carried "create an invoice for $500 due next
  Friday" — verbatim scripts/routing-eval.ts:187 — paired with the exact
  create_node call and the same hardcoded node_type="invoice" this PR
  removed elsewhere. Seeded skill markdown_content is injected as skill
  instructions, so it is model-facing guidance and equally contaminable.
- Rewrote that example onto placeholder values that cannot be copied, and
  replaced the TYPE MAPPING literals with the copy-the-type_id rule.
- guidance_corpus() now pulls every seeded skill's markdown_content from
  seed_skill_nodes() rather than naming files. A hand-maintained list is
  precisely how this was missed; enumerating from the seeder covers
  skills added later automatically. Verified by restoring the original
  example: the widened guard flags it (7-word run, correctly attributed).

Guard holes in the same mechanism:
- eval_prompts() failed OPEN — an unparsed literal was silently exempt.
  It now returns a site count and the caller asserts every counted site
  parsed, so a shape the parser does not understand fails loudly. Added
  backtick/template-literal support and escape-aware quote scanning.
- A prompt at or below MAX_SHARED_RUN could be quoted in full and never
  trip a fixed run length. Now also flags a whole-prompt match. This
  caught "What can you do?" in TOOL_STRATEGY_RULES, since fixed.

Eval:
- Scenario 3 now mentions returned status, so scenario 6 tests
  resolve_query routing on an indirect reference rather than whether the
  model guessed a status value the setup never implied.
- Renamed scenario 8 labels to name each step's role, not an entity the
  prompts no longer use.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DkqjrfV1G6kugsEjvyU9wU
@malibio

malibio commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

Pragmatic Code Review — PR #1763 (Re-Review)

Review Type: Re-Review
Scope of this pass: git diff 370782ef..HEAD — commit c4e4e856 "Address review: widen contamination guard to all model-facing guidance" (3 files, +121/-35)


Previous Review Summary

Count
Recommendations in previous review 6 (1 Critical, 3 Important, 2 Nit)
Addressed 5 (Findings 1, 2, 3, 4, 5)
Not addressed (deliberate, agreed) 1 (Finding 6 — longest_shared_run O(n·m))
New issues found this pass 1 (🟢 Suggestion — quoted_whole false-positive class for 1-word prompts)

The blocking finding is resolved. Every fix was independently verified rather than taken on trust — details below.


Verification of Prior Findings

✅ Finding 1 (🔴 Critical) — guard missed skill_pipeline.rs; live contamination present — RESOLVED

📁 packages/agent/src/agent_guidance.rs:187-196, packages/agent/src/skill_pipeline.rs:307-320

The fix is better than what I asked for. I recommended adding skill_pipeline.rs to the corpus; the commit instead enumerates crate::skill_pipeline::seed_skill_nodes() and pulls every non-empty markdown_content. That is the derive-from-the-assembled-artifact form I argued for on principle, and it means a skill added next quarter is covered with no one remembering to update a list. The comment earns its place by saying why ("a hand-maintained file list is exactly how the skill_pipeline.rs contamination went unnoticed") rather than what.

I independently confirmed the coverage claim. Re-implementing the guard's algorithm and running the old EXAMPLE block against both eval files reproduces exactly the failure the main agent reported:

CAUGHT run=7 whole=False 'Add an invoice for $500 due next Friday'
       shared='an invoice for 500 due next friday'

And the widened guard is genuinely green at HEAD — I ran the full cross-product (27 prompts from aichat-matrix.ts + routing-eval.ts × the whole agent_guidance.rs / skill_rules.rs / skill_pipeline.rs text, which is a superset of the real corpus) with zero violations. cargo test -p nodespace-agent agent_guidance → 11 passed. So the guard is not green by omission; it is green because the contamination is gone.

The skill_pipeline.rs TYPE MAPPING rewrite is also the right call: replacing (e.g. "invoice", "customer", "project") with "copied exactly as written — never the user's noun for it, and never a shortened or paraphrased form" makes the skill-level instruction agree with the new NODE_TYPE COMES FROM RELEVANT ENTITY TYPES rule instead of quietly undercutting it with three literals. Two layers of guidance now say the same thing.

✅ Finding 2 (🟡) — zero threshold headroom / short prompts quotable whole — RESOLVED

📁 packages/agent/src/agent_guidance.rs:353-357

quoted_whole = run == prompt_words.len() is option (a) from my recommendation, and the reworded CONVERSATIONAL TURNS line drops the quoted "what can you do?" / "hi" / "hello" literals. The rewording is also better guidance, not just decontamination — "questions about your own capabilities or limits" generalizes past the one phrasing, and the added clause "nothing in the user's graph needs to be read to answer them" states the mechanism rather than leaving the model to induce it from two samples.

The measured effect is real, not cosmetic. Per-prompt longest shared run against the full corpus, before → after:

Prompt words run before run after
"What can you do?" 4 4 (100% quoted) 2
"Hi there" 2 2 1
max across all 27 prompts — 4 3

The threshold is no longer load-bearing at a tie: nothing sits within one word of MAX_SHARED_RUN, and no prompt is fully quoted. The > vs >= operator is now genuinely slack rather than the only thing holding the assertion up.

Nice touch: the violation message branches on which rule fired ("the ENTIRE prompt appears" vs "shares a long phrase"), so a future failure tells you which invariant you broke.

✅ Finding 3 (🟡) — eval_prompts() fails open — RESOLVED

📁 packages/agent/src/agent_guidance.rs:209-250, 334-343

All three sub-parts landed: backtick support, escape-aware closing-quote scanning, and the prompts.len() == sites coverage assertion. I emulated the new parser against both real eval files:

scripts/aichat-matrix.ts   parsed 12  sites 12
scripts/routing-eval.ts    parsed 15  sites 15

Exact coverage, so the assertion is not vacuously satisfied.

On the soundness of sites (you asked me to scrutinize this specifically). The counter is incremented after the quote-character check, so shapes with no literal at all — prompt: SCENARIO_6, prompt: makePrompt("…"), prompt: fixture.prompt, shorthand { prompt } — count 0 sites and parse 0 prompts. That balances, and it is the right call for genuine references. It does mean a deliberately indirected prompt escapes the guard silently, but that is unavoidable without executing the TS, and it is a much narrower hole than "any shape the lexer didn't anticipate."

I probed the shapes that could re-open a fail-open gap:

Shape sites parsed Verdict
prompt: `…${amt}…` (interp) 1 1 ✅ balanced; interpolation token normalizes away harmlessly
prompt: "a" + b (concat) 1 1 ✅ balanced; first fragment still checked
prompt : "…" (space before colon) 0 0 ⚠️ balanced but invisible — see note below
// prompt: "…" (doc comment) 1 1 ✅ fails closed (over-counts, harmless direction)
* The prompt: keep them phrased… (prose) 0 0 ✅ correctly skipped
{ turns: [{ prompt: "ok" }] } (nested) 1 1 ✅

The new preceded-by-identifier skip (is_alphanumeric() \|\| '_' \|\| '.') does not undercount in practice: I checked every non-key prompt occurrence in aichat-matrix.ts (lines 40-51 prose, 230 comment, 321-322 step.prompt) and none is a literal site. The prose case at line 44 is instructive — guidance_is_not_contaminated_by_eval_prompts in a comment is correctly ignored because prompt: there isn't preceded by an identifier and isn't followed by a quote.

prompt : "…" (space before colon) is the one shape that skips silently and balances, so the coverage assertion won't catch it. It is not worth chasing — Prettier normalizes it away on every commit in this repo — but it is the honest limit of the mechanism. Not blocking.

The escape handling is correct. an \"escaped\" quote inside parses in full rather than truncating at She said \. The parser retains the literal backslash in its output (the Rust test asserts "an \\\"escaped\\\" quote inside", which looks alarming but is right), and since normalize() keeps only [a-z0-9]+ runs, the backslash is stripped before comparison — tokenization is ["an","escaped","quote","inside"] either way. Trailing \\ and \t cases also behave. No correctness issue.

✅ Finding 4 (🟡) — scenario 6 may not exercise resolve_query's purpose — RESOLVED

📁 scripts/aichat-matrix.ts:234-238

Adding "and whether it's been returned" to scenario 3's prompt is the minimal, targeted fix: it makes a status field a natural consequence of the stated requirement, so scenario 6's "set it to returned" tests routing rather than whether the model happened to invent a returned enum value. The inline comment explains the coupling at the site where it matters, which is where a future editor rewording scenario 3 will actually see it.

This prompt is now 19 words with a max shared run of 3 — the largest run in the file, but still comfortably inside tolerance and against generic connective words, not distinctive phrasing.

✅ Finding 5 (🟢 Nit) — scenario 8 labels contradict prompts — RESOLVED

📁 scripts/aichat-matrix.ts:265-281

Renamed to 8a. Create type: first / 8b. Create type: second / 8c. Instance: first type / 8d. Instance: second type. The header comment was correspondingly trimmed of the now-unnecessary ("book", "contact") parenthetical. Since AC-5's honest re-run rewrites the baseline anyway, ID churn costs nothing here — this was the right moment to take it.

⏸ Finding 6 (🟢 Nit) — longest_shared_run O(n·m) — NOT ADDRESSED, AND I AGREE

I flagged it as optional and it should stay unaddressed. It is a test-only path; measured inputs are ~27 prompts × ~15 corpus entries of a few thousand words, and the suite runs in 0.03s. Replacing a straightforwardly readable nested loop with a suffix automaton to save unmeasurable time would be a net loss. YAGNI — optimize when there is a measurement, not a Big-O intuition. No disagreement.


Requirements Check

# Acceptance Criterion Status Notes
1 No eval scenario prompt appears verbatim/near-verbatim in guidance ✅ Met Previously ⚠️. The "What can you do?" tie is gone (4/4 → 2/4 words) and skill_pipeline.rs's verbatim routing-eval.ts prompt is removed. Measured max shared run across all 27 prompts × all guidance: 3 words, against generic connectives.
2 TOOL_STRATEGY_RULES teaches node_type from RELEVANT ENTITY TYPES, no hardcoded literal ✅ Met Unchanged this commit, and now reinforced rather than contradicted by skill_pipeline.rs's TYPE MAPPING line.
3 Eval prompts reworded so they don't share distinctive phrasing with guidance ✅ Met Scenario 3 extended, scenario 8 labels fixed. routing-eval.ts remains unreworded but is now clean because the guidance moved, which is the correct direction — guidance should not quote evals, rather than evals contorting around guidance.
4 Automated check fails the build if an eval prompt reappears in guidance ✅ Met Previously ⚠️. Corpus now derives from seed_skill_nodes(); parser fails closed via prompts.len() == sites; the quoted_whole rule closes the short-prompt blind spot. Verified to fire on the historical contamination (run=7) and to be green at HEAD for a non-vacuous reason.
5 Matrix re-run, new score recorded as honest baseline ⏸ Deferred Acknowledged, requires live daemon + model. scripts/results-e4b.json still holds the contaminated 11/12. Please carry Finding 4's recommendation from my prior review into that work — record why each scenario failed, not just pass/fail — otherwise the new baseline cannot separate "decontamination revealed real weakness" from "the reworded prompt is harder," which is the entire point of re-running. Still worth a tracking issue rather than trusting the commit message; it also gates revisiting ADR-056's scenario 5 claim.

Code Review Findings (new this pass)

🟢 Suggestions

1. quoted_whole will false-positive on any single-word prompt

📁 packages/agent/src/agent_guidance.rs:353-357

quoted_whole = run == prompt_words.len() fires whenever the entire prompt is matched — which for a one-word prompt means "guidance contains this one word anywhere." I probed the current corpus:

Hypothetical prompt words run quoted_whole
"Thanks" 1 1 🔴 true
"search" 1 1 🔴 true
"Delete it" 2 1 false
"hello" 1 0 false

So a future abstention scenario with the prompt "Thanks" — an entirely reasonable thing to add to a matrix that already tests greetings and small talk — would hard-fail the build with the ENTIRE prompt appears (1 consecutive words). The message reads as a contamination claim when the real cause is that the prompt is one common word. Two-word prompts are already safe ("Delete it" above), so the exposure is narrow.

My previous review actually suggested the guard: run == prompt_words.len() && run >= 3. The && run >= 3 half was dropped. Adding a floor makes the rule say what it means — a substantial prompt reproduced in full — rather than any prompt short enough to be trivially matched:

// A one- or two-word prompt is matched by coincidence, not contamination.
let quoted_whole = run == prompt_words.len() && run >= 3;

This costs nothing today (nothing is affected either way) and removes a tripwire whose failure message would actively mislead whoever hits it. Non-blocking — it is a latent papercut, not a defect in shipped behavior.

Principle: A check should fail for the reason its message states. A rule that conflates "fully quoted" with "short enough to match by chance" produces false alarms that train people to weaken the guard — the failure mode this whole PR exists to prevent.


Answers to the specific questions raised

Does guidance_corpus() now genuinely cover all model-facing guidance? Substantially yes, with one real-but-empty gap and one out-of-scope category. I scanned every candidate:

  • ✅ agent_guidance.rs constants, skill_rules.rs imperatives, all 8 seeded skills' markdown_content — covered.
  • ⚠️ prompt_assembler.rs:269-320 seed_prompt_nodes() — genuinely uncovered, and it genuinely reaches the model. These five seeds are the base system prompt (the doc comment says so: "All prompt content lives in these graph nodes — there is no hardcoded base prompt"). Three are already transitively covered because they interpolate SCHEMA_CREATION_RULES / TOOL_STRATEGY_RULES / NODE_REFERENCE_FORMAT. Two are not: "Response Formatting Rules" (~10 lines of prose) and "Core Identity" / "Tool Call Formatting". I ran the eval prompts against all of them: zero violations today. So this is a latent gap, not live contamination — but it is the same shape of gap as the one this commit just fixed, and it closes with three lines mirroring the seed_skill_nodes() loop:
    for t in crate::prompt_assembler::PromptAssembler::seed_prompt_nodes() {
        if !t.markdown_content.is_empty() {
            corpus.push(("prompt_assembler::seed_prompt_nodes", t.markdown_content));
        }
    }
    Worth doing, but I am not blocking on it: it is empty today, response-formatting prose is a poor host for a planted CRUD example, and the derive-from-seeds pattern is now established so a follow-up is mechanical. Your call whether to fold it in here or next time you touch this file.
  • ⚪ local_agent/tools.rs tool description fields and parameter_schema — these do reach the model, and I scanned them (zero hits). I would argue they are correctly out of scope: a tool description is a contract about what the tool does, and it must name concepts like "invoice" or "task" to be comprehensible. Sweeping them into the contamination corpus would generate false positives and pressure authors toward vaguer tool descriptions — a real cost for a speculative benefit. Same reasoning for the skill description strings in root_properties (routing text, deliberately keyword-rich).
  • ⚪ acp/context_assembly.rs — scanned, zero hits; it assembles user graph content, not authored guidance. Out of scope.

Is the prompts.len() == sites assertion sound? Yes, with the one narrow caveat above (prompt : with a space, normalized away by Prettier in practice). The counter is placed after the quote check, so references balance at 0/0 rather than inflating sites into a permanent failure — that placement is the detail that makes the assertion usable rather than a nuisance. Doc comments over-count, which fails in the safe direction.

Is the new escape-aware parser correct? Yes — verified against escaped quotes, escaped backslashes, and escaped control chars. The retained backslash is invisible to normalize(). The quoted_whole logic has the narrow false-positive class in Suggestion 1; no false negatives found.

Did the skill_pipeline.rs rewrite preserve behavioral soundness for Gemma E4B? This was my biggest worry going in, and I think the answer is yes — the rewrite is more instructive than what it replaced, for a specific reason. The old example was a trap for a small model precisely because it was concrete: "node_type": "invoice" next to "amount": 500 invites copying, which is the invoice_tracker mismatch that started this issue. The new block front-loads the frame ("the shape of the call, NOT the values"), then closes with an explicit prohibition naming the exact tokens at risk of being copied ("Never reuse widget or these field names — they are placeholders").

Two things make this work better than abstraction usually does on a small model:

  1. It is still fully concrete. widget / Shipment 24 / label, quantity, received_on, condition are real strings in a real JSON body — the model still sees a complete worked example with the same structural cues (nesting, types: string/number/date/enum-ish). Nothing was abstracted into placeholder syntax like <TYPE_ID>, which would have been a comprehension risk.
  2. The domain is deliberately inert. "widget"/"shipment" appears in no eval and maps to no plausible user request, so if the model does copy it, the failure is loud and obvious rather than a silent near-miss like invoice → invoice_tracker.

The residual risk is that a small model copies widget anyway on an ambiguous turn. The mitigation is adequate — a negative instruction placed immediately after the thing it negates, naming the literal token, is about as strong as prompt-level mitigation gets. Watch scenarios 4 / 8c / 8d on the AC-5 re-run; if create_node starts emitting node_type="widget", this block is the first suspect and the fix is a different inert domain, not restoring the invoice example.

Worth noting for completeness: schema_creation_guidance() (skill_pipeline.rs:60-100) still carries fully concrete Invoice and Project schema examples, and graph_editing_guidance() (:128-129) still uses invoice phrasing in its ambiguity examples. Those are fine and should stay — they are covered by the guard now, they contain no eval prompt, and they illustrate schema shape, which is exactly where a concrete example earns its cost. The distinction the PR draws is the right one: concrete examples are good; concrete examples that happen to be the eval's answer key are not.

New issues introduced by this commit? Only Suggestion 1. The parser rewrite, the corpus widening, and the prompt edits all check out; cargo test -p nodespace-agent agent_guidance is green (11 passed) and the guard is green for a verified non-vacuous reason.


Recommendation

APPROVE

The blocking finding is fully resolved, and resolved better than I asked — deriving the corpus from seed_skill_nodes() rather than adding a file to a list turns a hand-maintained invariant into a structural one, which was the actual principle behind Finding 1 rather than the specific remedy I proposed. Findings 2-5 are all genuinely addressed, not papered over: I verified the headroom numerically (max shared run 4 → 3, no prompt fully quoted), verified parser coverage is exact on both eval files (12/12 and 15/15), and verified the widened guard reproduces the historical run=7 catch.

The two carry-forwards are both non-blocking and both belong to AC-5's follow-up rather than this PR:

  • Add prompt_assembler::seed_prompt_nodes() to the corpus (three lines, empty gap today, same shape as the one just closed).
  • Add the && run >= 3 floor to quoted_whole (Suggestion 1).
  • On the honest re-run: record failure reasons, not just pass/fail — without that the new baseline is uninterpretable, and interpretability is the whole reason for decontaminating.

This is a good example of a review response that fixed the underlying class of problem rather than the reported instance. Ship it.

@malibio malibio left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review: APPROVE (posting as comment — GitHub blocks self-approval). Blocking Finding 1 resolved; Findings 2-5 verified addressed. Full review in the PR comment.

@malibio malibio left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review: APPROVE (posting as a comment — GitHub blocks self-approval).

Blocking Finding 1 resolved, and resolved better than requested: guidance_corpus() now derives from seed_skill_nodes() rather than naming files, turning a hand-maintained invariant into a structural one. Findings 2-5 all verified addressed by independent re-implementation of the guard (max shared run 4 → 3, no prompt fully quoted; parser coverage exact at 12/12 and 15/15; historical run=7 contamination still caught). Finding 6 correctly left alone.

Two non-blocking carry-forwards for the AC-5 follow-up: add prompt_assembler::seed_prompt_nodes() to the corpus (empty gap today, same shape as the one just closed), and add the && run >= 3 floor to quoted_whole. Full review in the PR comment.

Both carry-forwards from the PR #1763 re-review, neither blocking:

- guidance_corpus() now also enumerates seed_prompt_nodes(). Those
  templates ARE the base system prompt; three of five interpolate the
  constants already in the corpus, but "Core Identity" and "Response
  Formatting Rules" carry literal text nothing else covered. Clean
  today, so this closes a latent gap rather than a live one.
- quoted_whole now requires >= 3 words. Below that it degenerates into
  "guidance mentions this word anywhere", which would flag ordinary
  vocabulary ("Thanks", "search") and report it as contamination — a
  tripwire whose message would mislead whoever hit it.

The third carry-forward — recording per-scenario failure reasons on the
honest re-run — belongs to the deferred AC-5 measurement, not here.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DkqjrfV1G6kugsEjvyU9wU
@malibio

malibio commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

Re-review carry-forwards addressed in da12629:

  • guidance_corpus() now also enumerates seed_prompt_nodes() — those templates are the base system prompt, and "Core Identity" / "Response Formatting Rules" carry literal text nothing else covered. Clean today, so this closes a latent gap.
  • quoted_whole now requires >= 3 words, so it can't degenerate into "guidance mentions this word anywhere".

The third carry-forward — recording per-scenario failure reasons on the honest re-run — belongs to the deferred AC-5 measurement and is tracked in #1764, along with revisiting ADR-056's Scenario 5 claim and the specific things to watch (scenarios 4/8c/8d for the INSTANCE vs TYPE edit, and whether node_type="widget" ever gets copied from the new placeholder example).

Full gate green on each push.

Completes acceptance criterion 5 of #1760, which the earlier commits
deferred: the matrix had to be re-run against a live daemon and model
before its score meant anything.

Result: 9/12 on gemma-4-e4b-q4km, versus 11/12 recorded under
contamination. The drop is the planted answers coming off.

Two corrections to the issue's premises, found by running it:

- scripts/results-e4b.json did not exist in the repo. The contaminated
  11/12 lived only in a scratch file, so there was nothing to overwrite.
  It is now committed with provenance (model, date, guidance commit,
  host memory), which #1759 wants as eval:agent's default baseline —
  that issue should note the number is 9/12, not the 11/12 it assumes.

- A DB purge is mandatory and was not written down anywhere. Seeding
  skips nodes that already exist, so a reused database keeps the OLD
  prompt/skill nodes and the run silently measures guidance you already
  changed. The daemon binary must be rebuilt for the same reason:
  guidance is compiled-in string constants. My first two runs were
  invalid this way — a stale binary reseeded the pre-fix skill_pipeline
  example, which I caught only by querying the seeded DB for the old
  text. Recorded in the results note.

Scenario 8a/8b reworded again: "Set up somewhere to note down albums"
dropped the tracking vocabulary the DATABASE=SCHEMA rule keys on, and
the model asked for confirmation instead of calling create_schema. A
control run confirmed the ORIGINAL wording still fires create_schema on
decontaminated guidance, so that was a rewording artifact, not a
decontamination finding. Now "Start tracking albums..." — eval-
independent but still carrying the intent.

The three remaining failures are genuine agent defects, not artifacts:
scenario 4 calls create_schema for an instance request and errors, which
starves scenario 6 of a node to update; 8c retries create_node after an
error. These are real behaviors to fix, now visible because nothing is
planted in the prompt.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DkqjrfV1G6kugsEjvyU9wU
@malibio

malibio commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

Pragmatic Code Review — PR #1763 (Third Pass)

Review Type: Re-Review (third pass)
Scope of this pass: c4e4e856..HEAD — commits da126293 (guard gaps) and 805a60e4 (honest baseline). +182/-3 across packages/agent/src/agent_guidance.rs, scripts/aichat-matrix.ts, scripts/results-e4b.json.


Previous Review Summary

Count
Round 1 findings 6 (1 Critical, 3 Important, 2 Nit) — resolved in round 2
Round 2 carry-forwards 3 (all non-blocking)
Addressed in this pass 2 (seed_prompt_nodes() corpus gap, quoted_whole floor)
Partially addressed 1 (per-scenario failure reasons — captured mechanically, not diagnostically)
New issues this pass 3 (1 Important, 2 Suggestions)

Carry-forward verification

✅ CF-1 — guidance_corpus() omits seed_prompt_nodes() — RESOLVED, and correctly.
agent_guidance.rs:199-204 now enumerates it. I verified this is the right call on all four axes round 2 asked about:

  • Not test-only / not private-by-intent: seed_prompt_nodes() is pub fn at prompt_assembler.rs:260 with a production caller at daemon/src/services/assembly.rs:229. It is genuinely the first-run seeding path for the base system prompt, not a test fixture. Including it is consistent with the existing seed_skill_nodes() precedent.
  • Not vacuous: all 5 templates (Core Identity, Workspace Context Template, Tool Strategy Guide, Response Formatting Rules, Tool Call Formatting) carry non-empty markdown_content, so the !is_empty() filter is defensive rather than silently skipping the whole set. The three that interpolate SCHEMA_CREATION_RULES / TOOL_STRATEGY_RULES / NODE_REFERENCE_FORMAT do add duplicate coverage, but duplicates only cost runtime, they cannot mask a violation.
  • Not over-broad: the guard is green at HEAD and I confirmed it is green for a non-vacuous reason — cargo test -p nodespace-agent --lib guidance passes 11/11 including contamination_guard_detects_a_planted_example, which still fires at run=7 on the historical contamination. Adding this corpus did not require loosening any threshold.

The comment ("Some interpolate the constants above, but others carry their own literal text that nothing else covers") accurately describes the actual situation. Good.

✅ CF-2 — quoted_whole degenerates for 1-2 word prompts — RESOLVED.
agent_guidance.rs:364 now reads run == prompt_words.len() && run >= 3, exactly the floor round 2 proposed. The comment explains why the floor exists rather than restating the condition. This is a tripwire-quality fix: the failure it prevents is a misleading error message, and the comment is what stops a future reader from "simplifying" the >= 3 away.

⚠️ CF-3 — record why each scenario failed, not just pass/fail — PARTIALLY ADDRESSED.
results-e4b.json records a failure string per failing scenario, but it is the assertion's mechanical output ("Expected 'create_node' exactly once, got 0 (tools: create_schema)") — it says what the assertion saw, not why the model did it. Round 2's ask was specifically for the distinction between "decontamination revealed real weakness" and "the reworded prompt is harder." That analysis exists in the commit message for 8a/8b, and nowhere for scenario 4. See Finding 1 — this carry-forward turns out to be the load-bearing one.


Requirements Check (issue #1760)

# Acceptance Criterion Status Notes
1 No eval prompt verbatim/near-verbatim in agent_guidance.rs / skill_rules.rs ✅ Met Mechanically: max shared run across all prompts × the now-wider corpus is 3 words. Unchanged this pass.
2 TOOL_STRATEGY_RULES teaches node_type from RELEVANT ENTITY TYPES, no literal ✅ Met Unchanged this pass.
3 Eval prompts reworded, no distinctive shared phrasing ⚠️ Met mechanically, weakened in spirit Passes the guard, but 8a/8b were reworded toward the guidance's own trigger vocabulary. See Finding 1.
4 Automated check fails the build if a prompt reappears in guidance ✅ Met Now strictly stronger: corpus covers the base system prompt too, and the short-prompt blind spot is closed.
5 Matrix re-run, new score recorded as honest baseline ⚠️ Substantially met 9/12 recorded with provenance, on a purged DB and rebuilt binary. Genuinely honest work — the invalid-run catch is the strongest evidence of that. Two gaps: n_ctx missing (Finding 2), and 8a/8b's rewording makes two of the nine passes weaker evidence than the number implies (Finding 1).

On AC-5 more broadly: I want to be explicit that this commit does the hard, unglamorous thing correctly. The author ran the matrix, got a worse number, and committed the worse number. They then caught two invalid runs by querying the seeded DB for pre-fix text — a failure mode that produces a plausible-looking result and that nothing in the harness would have flagged. That is exactly the discipline this PR exists to install, and it is worth more than the score itself. My findings below are about interpretation of the number, not about the integrity of the effort.


Code Review Findings

🟡 Important

Finding 1 — Scenarios 8a/8b were reworded toward the guidance's trigger vocabulary; the control run does not establish what it is cited for

📁 scripts/aichat-matrix.ts:265-272 (prompts) · packages/agent/src/agent_guidance.rs:21 (the rule they now key on)

This is the question the review was asked to weigh most carefully, so I want to show the measurement rather than assert a judgment.

The DATABASE = SCHEMA rule triggers on an enumerated surface-vocabulary list:

"DATABASE" = SCHEMA: A request to start tracking a kind of thing — worded as a database, a tracker, a list, or "track X" — means call create_schema IMMEDIATELY

Content-word overlap between each candidate wording and that rule's trigger clause:

Wording Content words Overlapping the trigger list Fired create_schema?
"Create a database to track books…" (original, contaminated era) create, database, track, books, read database, track ✅ (control)
"Set up somewhere to note down albums…" (rewording #1) set, up, somewhere, note, down, albums, mean, listen none ❌ asked for confirmation
"Start tracking albums I mean to listen to" (rewording #2, HEAD) start, tracking, albums, mean, listen start, tracking ✅
"I also need a tracker for the venues I book" (rewording #2, HEAD) also, need, tracker, venues, book tracker ✅

The pattern is unambiguous: the scenario passes exactly when the prompt contains a word from the rule's enumerated trigger list, and fails when it does not. Rewording #1 was the only variant that removed all trigger vocabulary, and it was the only one that failed. Rewording #2 restored trigger words (tracking, tracker) drawn from the same enumeration as the original's (database, track) — different lexemes, same list, same stem.

Why the control run does not resolve this. The control established: original wording still fires create_schema post-decontamination. That is a real and useful result — it rules out "decontamination broke schema creation outright." But it does not distinguish the two live hypotheses, because the original wording is also trigger-vocabulary-bearing:

  • H1 (the commit's claim): rewording Set up Tauri + Svelte project structure #1 was unnaturally phrased; the model's failure was an artifact of awkward English.
  • H2 (not excluded): the model does not generalize the DATABASE=SCHEMA intent at all — it pattern-matches the enumerated trigger words, and any phrasing lacking them fails.

Both predict the control's outcome identically, so the control has zero discriminating power between them. Under H2, rewording #1 was not a bad prompt — it was the single most informative datapoint in the run, the one place the eval measured generalization rather than keyword lookup, and it returned a genuine negative. Rewording #2 then converted that finding into a pass by supplying the magic word.

To be clear about severity and intent: I do not read this as bad faith. The commit message is transparent about the sequence, ran a control rather than just re-rolling, and the contamination guard passes — the shared run is 2 words, well inside tolerance. This is not verbatim contamination. But #1760's stated purpose is that the eval "cannot distinguish 'the agent generalized' from 'the answer was in the system prompt.'" A prompt tuned until it contains a word the guidance enumerates as a trigger reintroduces that exact ambiguity at the vocabulary level, one layer below where the mechanical guard looks. The guard measures phrase overlap; the coupling here is lexical.

The methodological hazard is the more important half: rewording a prompt because it failed, then reporting the result as a baseline, is the move that launders a weakness into a score. It is defensible only when the artifact hypothesis is independently confirmed, and here it is not.

Recommendation — pick one, all cheap, none blocking on their own:

  1. Preferred, and it costs one run: re-test rewording Set up Tauri + Svelte project structure #1 ("Set up somewhere to note down albums…") as an explicit generalization probe, kept alongside the current 8a rather than replacing it. If it still fails, that is a genuine, recordable finding — "the model keys on trigger vocabulary and does not generalize the intent" — which is a far more valuable eval output than a 10th pass. If it now passes, H1 is confirmed and the artifact conclusion is earned.
  2. Record the control run's inputs and outcome in results-e4b.json, not only in the commit message. A future reader comparing against this baseline sees the file; git archaeology on why 8a is worded as it is will not happen.
  3. Soften the commit-message claim from "was a rewording artifact" to what the evidence supports: "the original wording still fires; the trigger-vocabulary-free wording does not, and which of those is the artifact is untested."

Principle: an eval's prompts must be sampled independently of the system under test. Tuning a prompt in response to a failure couples them, and the coupling is invisible to a guard that only measures phrase overlap. Related: a control must be capable of distinguishing the hypotheses it is cited against — otherwise it provides confidence without evidence.


🟢 Suggestions

Finding 2 — results-e4b.json omits n_ctx, which #1759 calls for and which a known live bug makes load-bearing

📁 scripts/results-e4b.json:1-8

Provenance recorded: label, model, recorded_at, guidance_commit, host_memory_gb, note. #1759's AC asks for "model, date, host memory, and granted n_ctx" — n_ctx is the one missing field.

This matters more than a checklist gap, because #1759 documents an active compute_effective_n_ctx bug flooring n_ctx to 4096 against a ~6,600-token agent system prompt, and documents a prior run where scenarios 1 and 2 reported as passing while every turn was dying on Context window exceeded — both assert "no tools called," and a timed-out turn calls no tools. Scenarios 1 and 2 are green in this very file.

I do not think that happened here, and I checked rather than assuming: latencies are 2.1s–28.5s with a 145s total and a coherent spread (tool-calling scenarios 8c/8d at 26–28s, trivial ones at 2–4s), and eight scenarios show real tool calls. That is a live-inference profile, not a uniform-timeout profile. So this is a provenance gap, not a correctness doubt — but recording n_ctx is what lets the next reader reach that conclusion in one glance instead of reconstructing it from latency spreads.

Recommendation: add n_ctx (and ideally the daemon commit alongside guidance_commit, since guidance is compiled in). If the value isn't recoverable now, "n_ctx": null with a note beats silent omission — it tells the reader the field was considered. #1759 owns the shared runner that should populate this automatically; this is about the one committed artifact that will be used as the default baseline before that lands.

Principle: a cited measurement must carry the conditions that produced it, especially the conditions under which it is known to fail silently.


Finding 3 — Scenario 4's failure is characterized as a genuine defect, but it carries the same rewording-artifact risk as 8a/8b — untested in the opposite direction

📁 scripts/aichat-matrix.ts:269-271 · scripts/results-e4b.json scenario 4

The commit asserts the three failures "are genuine agent defects, not artifacts." I agree for 6 and 8c, and I think 4 is unproven:

  • Scenario 6 ("The 2400 one came back — set it to returned", called no tools): correctly identified as a cascade — scenario 4 never created the node, so there is nothing to find. Sound; not independently informative until 4 is fixed.
  • Scenario 8c ("Put down Kind of Blue, it's by Miles Davis" → create_node twice): unambiguous defect. It called the right tool and retried after an error, which is precisely the SKILL COMPLETION / IDENTICAL TOOL CALLS rules failing. Real.
  • Scenario 4 ("Log a laser cutter checked out on the 12th, replacement cost 2400" → search_skills, create_schema): the model chose schema over instance. This is the exact INSTANCE vs TYPE discrimination — and it is the mirror image of the 8a/8b situation.

Note the asymmetry in how the two rules are written:

  • DATABASE = SCHEMA keys on surface vocabulary — an enumerated word list (a database, a tracker, a list, "track X").
  • INSTANCE vs TYPE keys on semantic structure — "a request that supplies the particulars of ONE record — a name, an amount, a title, a date."

Scenario 4's prompt satisfies the semantic condition richly (an item, a date, an amount). "Log a…" is fair, natural instance-creation English — I do not think it is a bad prompt. But it was reworded once in 370782ef (from "Add an invoice for $500 due next Friday") and, unlike 8a/8b, never re-tested after it failed. So the run applied rewording-then-retry to the scenarios that failed and were easy to fix, and not to the one that failed and was attributed to the model. That asymmetric treatment is what makes the "genuine defect" label unproven rather than wrong — a control here (does "Add an invoice for $500 due next Friday" fire create_node on decontaminated guidance?) is the same one-run experiment that was already run for 8a/8b, and would settle it.

Given a plausible reading where the model keys on surface vocabulary generally (Finding 1's H2), a competing explanation for scenario 4 is available: "Log a laser cutter checked out…" contains no INSTANCE trigger vocabulary either, and the model fell back to create_schema. Whether that is "a genuine defect" or "the same vocabulary-keying limitation seen in 8a" changes what should be fixed — the model's generalization, or the guidance rule's structure.

Recommendation: run the same control for scenario 4 that was run for 8a/8b, and record the outcome in the results note. If the original wording also fails, "genuine defect" is confirmed and strengthened. If it passes, scenario 4 belongs in the same category as 8a/8b, and the honest baseline is arguably 9/12 with two vocabulary-sensitivity findings, which is a more useful result than 9/12 with three defect labels. Either way this is follow-up work for #1764, not a blocker here.

Principle: apply the same evidentiary standard to failures you attribute to the system as to failures you attribute to the harness. Asymmetric scrutiny biases the conclusion toward whichever explanation was convenient.


Finding 4 — results-e4b.json is committed but referenced by nothing

📁 scripts/results-e4b.json

Nothing in scripts/ or packages/ reads this file — I grepped. It is intended as eval:agent's default baseline, but the wiring is #1759's. Fine as-is; flagging only so it does not sit as an orphan artifact that silently goes stale while being cited as authoritative. #1759 should be updated to say the number is 9/12, not the 11/12 it currently assumes — the commit message notes this, but the issue itself is what the next agent will read.

Nit: guidance_commit: "da126293" is recorded, but the run measured 805a60e4's prompts (8a/8b were reworded in that same commit). The guidance constants are indeed unchanged between the two, so the field is technically accurate — but a reader reconstructing the run needs the eval commit too, and those diverge here. Consider eval_commit alongside guidance_commit.


Recommendation

COMMENT — approve-with-reservations in substance; nothing here should block the merge.

The two unreviewed commits are a clear net improvement. da126293 closes both round-2 carry-forwards exactly as proposed, and I verified the seed_prompt_nodes() addition is production guidance, non-vacuous, and did not require weakening any threshold — the guard is now strictly stronger than at round 2. 805a60e4 does the genuinely hard thing: it ran the matrix, got a worse number than the contaminated one, committed the worse number, and caught two invalid runs through a failure mode (stale binary reseeding pre-fix guidance) that produces plausible-looking results and that nothing in the harness would have flagged. Discovering and writing down the mandatory-DB-purge precondition is a durable contribution beyond this PR.

My reservation is confined to interpretation, not integrity: the 9/12 is honest about what it measured, but two of the nine passes are weaker evidence than the number implies. Scenarios 8a/8b were reworded until they contained a word the DATABASE = SCHEMA rule enumerates as a trigger, and the control offered in defense cannot discriminate between "rewording artifact" and "the model keys on trigger vocabulary and does not generalize" — both hypotheses predict the control's outcome identically. Symmetrically, scenario 4's "genuine defect" label rests on the one failure that was not given the rewording-and-retry treatment the others got.

None of this justifies holding the PR. The contamination guard — this PR's actual deliverable — is sound, wider than it was, and mechanically verified. The baseline is directionally right and vastly better than the contaminated 11/12 it replaces. The right home for Findings 1 and 3 is #1764, as two one-run control experiments:

  1. Re-test "Set up somewhere to note down albums…" as an explicit generalization probe kept alongside 8a.
  2. Re-test "Add an invoice for $500 due next Friday" against scenario 4's expectation.

If both fail, the finding is not "9/12 with three defects" but something considerably more actionable: the guidance rules that key on enumerated surface vocabulary do not generalize, and the eval is sensitive to whether prompts happen to contain the magic words. That would be the most valuable thing this decontamination effort could surface, and it is two runs away.

Finding 2 (n_ctx) should be picked up wherever #1759's provenance work lands.

@malibio malibio left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pragmatic Code Review — PR #1763 (Third Pass)

Review Type: Re-Review (third pass)
Scope of this pass: c4e4e856..HEAD — commits da126293 (guard gaps) and 805a60e4 (honest baseline). +182/-3 across packages/agent/src/agent_guidance.rs, scripts/aichat-matrix.ts, scripts/results-e4b.json.


Previous Review Summary

Count
Round 1 findings 6 (1 Critical, 3 Important, 2 Nit) — resolved in round 2
Round 2 carry-forwards 3 (all non-blocking)
Addressed in this pass 2 (seed_prompt_nodes() corpus gap, quoted_whole floor)
Partially addressed 1 (per-scenario failure reasons — captured mechanically, not diagnostically)
New issues this pass 3 (1 Important, 2 Suggestions)

Carry-forward verification

✅ CF-1 — guidance_corpus() omits seed_prompt_nodes() — RESOLVED, and correctly.
agent_guidance.rs:199-204 now enumerates it. I verified this is the right call on all four axes round 2 asked about:

  • Not test-only / not private-by-intent: seed_prompt_nodes() is pub fn at prompt_assembler.rs:260 with a production caller at daemon/src/services/assembly.rs:229. It is genuinely the first-run seeding path for the base system prompt, not a test fixture. Including it is consistent with the existing seed_skill_nodes() precedent.
  • Not vacuous: all 5 templates (Core Identity, Workspace Context Template, Tool Strategy Guide, Response Formatting Rules, Tool Call Formatting) carry non-empty markdown_content, so the !is_empty() filter is defensive rather than silently skipping the whole set. The three that interpolate SCHEMA_CREATION_RULES / TOOL_STRATEGY_RULES / NODE_REFERENCE_FORMAT do add duplicate coverage, but duplicates only cost runtime, they cannot mask a violation.
  • Not over-broad: the guard is green at HEAD and I confirmed it is green for a non-vacuous reason — cargo test -p nodespace-agent --lib guidance passes 11/11 including contamination_guard_detects_a_planted_example, which still fires at run=7 on the historical contamination. Adding this corpus did not require loosening any threshold.

The comment ("Some interpolate the constants above, but others carry their own literal text that nothing else covers") accurately describes the actual situation. Good.

✅ CF-2 — quoted_whole degenerates for 1-2 word prompts — RESOLVED.
agent_guidance.rs:364 now reads run == prompt_words.len() && run >= 3, exactly the floor round 2 proposed. The comment explains why the floor exists rather than restating the condition. This is a tripwire-quality fix: the failure it prevents is a misleading error message, and the comment is what stops a future reader from "simplifying" the >= 3 away.

⚠️ CF-3 — record why each scenario failed, not just pass/fail — PARTIALLY ADDRESSED.
results-e4b.json records a failure string per failing scenario, but it is the assertion's mechanical output ("Expected 'create_node' exactly once, got 0 (tools: create_schema)") — it says what the assertion saw, not why the model did it. Round 2's ask was specifically for the distinction between "decontamination revealed real weakness" and "the reworded prompt is harder." That analysis exists in the commit message for 8a/8b, and nowhere for scenario 4. See Finding 1 — this carry-forward turns out to be the load-bearing one.


Requirements Check (issue #1760)

# Acceptance Criterion Status Notes
1 No eval prompt verbatim/near-verbatim in agent_guidance.rs / skill_rules.rs ✅ Met Mechanically: max shared run across all prompts × the now-wider corpus is 3 words. Unchanged this pass.
2 TOOL_STRATEGY_RULES teaches node_type from RELEVANT ENTITY TYPES, no literal ✅ Met Unchanged this pass.
3 Eval prompts reworded, no distinctive shared phrasing ⚠️ Met mechanically, weakened in spirit Passes the guard, but 8a/8b were reworded toward the guidance's own trigger vocabulary. See Finding 1.
4 Automated check fails the build if a prompt reappears in guidance ✅ Met Now strictly stronger: corpus covers the base system prompt too, and the short-prompt blind spot is closed.
5 Matrix re-run, new score recorded as honest baseline ⚠️ Substantially met 9/12 recorded with provenance, on a purged DB and rebuilt binary. Genuinely honest work — the invalid-run catch is the strongest evidence of that. Two gaps: n_ctx missing (Finding 2), and 8a/8b's rewording makes two of the nine passes weaker evidence than the number implies (Finding 1).

On AC-5 more broadly: I want to be explicit that this commit does the hard, unglamorous thing correctly. The author ran the matrix, got a worse number, and committed the worse number. They then caught two invalid runs by querying the seeded DB for pre-fix text — a failure mode that produces a plausible-looking result and that nothing in the harness would have flagged. That is exactly the discipline this PR exists to install, and it is worth more than the score itself. My findings below are about interpretation of the number, not about the integrity of the effort.


Code Review Findings

🟡 Important

Finding 1 — Scenarios 8a/8b were reworded toward the guidance's trigger vocabulary; the control run does not establish what it is cited for

📁 scripts/aichat-matrix.ts:265-272 (prompts) · packages/agent/src/agent_guidance.rs:21 (the rule they now key on)

This is the question the review was asked to weigh most carefully, so I want to show the measurement rather than assert a judgment.

The DATABASE = SCHEMA rule triggers on an enumerated surface-vocabulary list:

"DATABASE" = SCHEMA: A request to start tracking a kind of thing — worded as a database, a tracker, a list, or "track X" — means call create_schema IMMEDIATELY

Content-word overlap between each candidate wording and that rule's trigger clause:

Wording Content words Overlapping the trigger list Fired create_schema?
"Create a database to track books…" (original, contaminated era) create, database, track, books, read database, track ✅ (control)
"Set up somewhere to note down albums…" (rewording #1) set, up, somewhere, note, down, albums, mean, listen none ❌ asked for confirmation
"Start tracking albums I mean to listen to" (rewording #2, HEAD) start, tracking, albums, mean, listen start, tracking ✅
"I also need a tracker for the venues I book" (rewording #2, HEAD) also, need, tracker, venues, book tracker ✅

The pattern is unambiguous: the scenario passes exactly when the prompt contains a word from the rule's enumerated trigger list, and fails when it does not. Rewording #1 was the only variant that removed all trigger vocabulary, and it was the only one that failed. Rewording #2 restored trigger words (tracking, tracker) drawn from the same enumeration as the original's (database, track) — different lexemes, same list, same stem.

Why the control run does not resolve this. The control established: original wording still fires create_schema post-decontamination. That is a real and useful result — it rules out "decontamination broke schema creation outright." But it does not distinguish the two live hypotheses, because the original wording is also trigger-vocabulary-bearing:

  • H1 (the commit's claim): rewording #1 was unnaturally phrased; the model's failure was an artifact of awkward English.
  • H2 (not excluded): the model does not generalize the DATABASE=SCHEMA intent at all — it pattern-matches the enumerated trigger words, and any phrasing lacking them fails.

Both predict the control's outcome identically, so the control has zero discriminating power between them. Under H2, rewording #1 was not a bad prompt — it was the single most informative datapoint in the run, the one place the eval measured generalization rather than keyword lookup, and it returned a genuine negative. Rewording #2 then converted that finding into a pass by supplying the magic word.

To be clear about severity and intent: I do not read this as bad faith. The commit message is transparent about the sequence, ran a control rather than just re-rolling, and the contamination guard passes — the shared run is 2 words, well inside tolerance. This is not verbatim contamination. But #1760's stated purpose is that the eval "cannot distinguish 'the agent generalized' from 'the answer was in the system prompt.'" A prompt tuned until it contains a word the guidance enumerates as a trigger reintroduces that exact ambiguity at the vocabulary level, one layer below where the mechanical guard looks. The guard measures phrase overlap; the coupling here is lexical.

The methodological hazard is the more important half: rewording a prompt because it failed, then reporting the result as a baseline, is the move that launders a weakness into a score. It is defensible only when the artifact hypothesis is independently confirmed, and here it is not.

Recommendation — pick one, all cheap, none blocking on their own:

  1. Preferred, and it costs one run: re-test rewording #1 ("Set up somewhere to note down albums…") as an explicit generalization probe, kept alongside the current 8a rather than replacing it. If it still fails, that is a genuine, recordable finding — "the model keys on trigger vocabulary and does not generalize the intent" — which is a far more valuable eval output than a 10th pass. If it now passes, H1 is confirmed and the artifact conclusion is earned.
  2. Record the control run's inputs and outcome in results-e4b.json, not only in the commit message. A future reader comparing against this baseline sees the file; git archaeology on why 8a is worded as it is will not happen.
  3. Soften the commit-message claim from "was a rewording artifact" to what the evidence supports: "the original wording still fires; the trigger-vocabulary-free wording does not, and which of those is the artifact is untested."

Principle: an eval's prompts must be sampled independently of the system under test. Tuning a prompt in response to a failure couples them, and the coupling is invisible to a guard that only measures phrase overlap. Related: a control must be capable of distinguishing the hypotheses it is cited against — otherwise it provides confidence without evidence.


🟢 Suggestions

Finding 2 — results-e4b.json omits n_ctx, which #1759 calls for and which a known live bug makes load-bearing

📁 scripts/results-e4b.json:1-8

Provenance recorded: label, model, recorded_at, guidance_commit, host_memory_gb, note. #1759's AC asks for "model, date, host memory, and granted n_ctx" — n_ctx is the one missing field.

This matters more than a checklist gap, because #1759 documents an active compute_effective_n_ctx bug flooring n_ctx to 4096 against a ~6,600-token agent system prompt, and documents a prior run where scenarios 1 and 2 reported as passing while every turn was dying on Context window exceeded — both assert "no tools called," and a timed-out turn calls no tools. Scenarios 1 and 2 are green in this very file.

I do not think that happened here, and I checked rather than assuming: latencies are 2.1s–28.5s with a 145s total and a coherent spread (tool-calling scenarios 8c/8d at 26–28s, trivial ones at 2–4s), and eight scenarios show real tool calls. That is a live-inference profile, not a uniform-timeout profile. So this is a provenance gap, not a correctness doubt — but recording n_ctx is what lets the next reader reach that conclusion in one glance instead of reconstructing it from latency spreads.

Recommendation: add n_ctx (and ideally the daemon commit alongside guidance_commit, since guidance is compiled in). If the value isn't recoverable now, "n_ctx": null with a note beats silent omission — it tells the reader the field was considered. #1759 owns the shared runner that should populate this automatically; this is about the one committed artifact that will be used as the default baseline before that lands.

Principle: a cited measurement must carry the conditions that produced it, especially the conditions under which it is known to fail silently.


Finding 3 — Scenario 4's failure is characterized as a genuine defect, but it carries the same rewording-artifact risk as 8a/8b — untested in the opposite direction

📁 scripts/aichat-matrix.ts:269-271 · scripts/results-e4b.json scenario 4

The commit asserts the three failures "are genuine agent defects, not artifacts." I agree for 6 and 8c, and I think 4 is unproven:

  • Scenario 6 ("The 2400 one came back — set it to returned", called no tools): correctly identified as a cascade — scenario 4 never created the node, so there is nothing to find. Sound; not independently informative until 4 is fixed.
  • Scenario 8c ("Put down Kind of Blue, it's by Miles Davis" → create_node twice): unambiguous defect. It called the right tool and retried after an error, which is precisely the SKILL COMPLETION / IDENTICAL TOOL CALLS rules failing. Real.
  • Scenario 4 ("Log a laser cutter checked out on the 12th, replacement cost 2400" → search_skills, create_schema): the model chose schema over instance. This is the exact INSTANCE vs TYPE discrimination — and it is the mirror image of the 8a/8b situation.

Note the asymmetry in how the two rules are written:

  • DATABASE = SCHEMA keys on surface vocabulary — an enumerated word list (a database, a tracker, a list, "track X").
  • INSTANCE vs TYPE keys on semantic structure — "a request that supplies the particulars of ONE record — a name, an amount, a title, a date."

Scenario 4's prompt satisfies the semantic condition richly (an item, a date, an amount). "Log a…" is fair, natural instance-creation English — I do not think it is a bad prompt. But it was reworded once in 370782ef (from "Add an invoice for $500 due next Friday") and, unlike 8a/8b, never re-tested after it failed. So the run applied rewording-then-retry to the scenarios that failed and were easy to fix, and not to the one that failed and was attributed to the model. That asymmetric treatment is what makes the "genuine defect" label unproven rather than wrong — a control here (does "Add an invoice for $500 due next Friday" fire create_node on decontaminated guidance?) is the same one-run experiment that was already run for 8a/8b, and would settle it.

Given a plausible reading where the model keys on surface vocabulary generally (Finding 1's H2), a competing explanation for scenario 4 is available: "Log a laser cutter checked out…" contains no INSTANCE trigger vocabulary either, and the model fell back to create_schema. Whether that is "a genuine defect" or "the same vocabulary-keying limitation seen in 8a" changes what should be fixed — the model's generalization, or the guidance rule's structure.

Recommendation: run the same control for scenario 4 that was run for 8a/8b, and record the outcome in the results note. If the original wording also fails, "genuine defect" is confirmed and strengthened. If it passes, scenario 4 belongs in the same category as 8a/8b, and the honest baseline is arguably 9/12 with two vocabulary-sensitivity findings, which is a more useful result than 9/12 with three defect labels. Either way this is follow-up work for #1764, not a blocker here.

Principle: apply the same evidentiary standard to failures you attribute to the system as to failures you attribute to the harness. Asymmetric scrutiny biases the conclusion toward whichever explanation was convenient.


Finding 4 — results-e4b.json is committed but referenced by nothing

📁 scripts/results-e4b.json

Nothing in scripts/ or packages/ reads this file — I grepped. It is intended as eval:agent's default baseline, but the wiring is #1759's. Fine as-is; flagging only so it does not sit as an orphan artifact that silently goes stale while being cited as authoritative. #1759 should be updated to say the number is 9/12, not the 11/12 it currently assumes — the commit message notes this, but the issue itself is what the next agent will read.

Nit: guidance_commit: "da126293" is recorded, but the run measured 805a60e4's prompts (8a/8b were reworded in that same commit). The guidance constants are indeed unchanged between the two, so the field is technically accurate — but a reader reconstructing the run needs the eval commit too, and those diverge here. Consider eval_commit alongside guidance_commit.


Recommendation

COMMENT — approve-with-reservations in substance; nothing here should block the merge.

The two unreviewed commits are a clear net improvement. da126293 closes both round-2 carry-forwards exactly as proposed, and I verified the seed_prompt_nodes() addition is production guidance, non-vacuous, and did not require weakening any threshold — the guard is now strictly stronger than at round 2. 805a60e4 does the genuinely hard thing: it ran the matrix, got a worse number than the contaminated one, committed the worse number, and caught two invalid runs through a failure mode (stale binary reseeding pre-fix guidance) that produces plausible-looking results and that nothing in the harness would have flagged. Discovering and writing down the mandatory-DB-purge precondition is a durable contribution beyond this PR.

My reservation is confined to interpretation, not integrity: the 9/12 is honest about what it measured, but two of the nine passes are weaker evidence than the number implies. Scenarios 8a/8b were reworded until they contained a word the DATABASE = SCHEMA rule enumerates as a trigger, and the control offered in defense cannot discriminate between "rewording artifact" and "the model keys on trigger vocabulary and does not generalize" — both hypotheses predict the control's outcome identically. Symmetrically, scenario 4's "genuine defect" label rests on the one failure that was not given the rewording-and-retry treatment the others got.

None of this justifies holding the PR. The contamination guard — this PR's actual deliverable — is sound, wider than it was, and mechanically verified. The baseline is directionally right and vastly better than the contaminated 11/12 it replaces. The right home for Findings 1 and 3 is #1764, as two one-run control experiments:

  1. Re-test "Set up somewhere to note down albums…" as an explicit generalization probe kept alongside 8a.
  2. Re-test "Add an invoice for $500 due next Friday" against scenario 4's expectation.

If both fail, the finding is not "9/12 with three defects" but something considerably more actionable: the guidance rules that key on enumerated surface vocabulary do not generalize, and the eval is sensitive to whether prompts happen to contain the magic words. That would be the most valuable thing this decontamination effort could surface, and it is two runs away.

Finding 2 (n_ctx) should be picked up wherever #1759's provenance work lands.

…e baseline

Third-pass review argued my "rewording artifact" conclusion for scenarios
8a/8b rested on a control that could not discriminate the two competing
hypotheses, since the original wording is also trigger-vocabulary-bearing.
That was correct. Ran the two controls it asked for; both are recorded in
the results file rather than only in a commit message.

(a) The DATABASE=SCHEMA rule DOES generalize. Two trigger-vocabulary-free
    phrasings ("Set up somewhere to note down albums I mean to listen to" --
    the exact wording that failed in the matrix -- and "I want a place to jot
    down albums worth hearing") both fire create_schema correctly. The model
    is not merely keying on database/track/tracker/list, so the alternative
    hypothesis is falsified.

(b) Scenario 4's wording is not the cause, but not for the reason I claimed.
    Both it and an alternative instance phrasing behave identically, because
    both are downstream of a schema that never existed.

That led to the real finding: scenario 3 is a FALSE PASS. Its noExtraTypes
assertion only checks create_schema was called once, never that the schema
persisted. In the baseline run create_schema returned is_error=true --
title_template placeholders with no fields array -- so no equipment type was
registered, and scenario 4 correctly reported the type missing and was scored
a failure for it. Scenarios 4 and 6 are consequences of scenario 3, not
independent defects; 8c remains independent and real.

Confirmed pre-existing: this PR never touched the TITLE TEMPLATE or FIELD
RULES guidance. Root cause and revised scope moved to #1775.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DkqjrfV1G6kugsEjvyU9wU
@malibio

malibio commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

Pragmatic Code Review — PR #1763 (Fourth Pass)

Review Type: Re-Review (fourth pass)
Scope: 805a60e4..HEAD — one commit, 46bfed40. +5/-0, scripts/results-e4b.json only. No code changes.

The diff is JSON, but it asserts conclusions that #1775 and #1759's baseline work will consume. I reviewed the reasoning, not the syntax, and independently verified the claims I could check read-only against /tmp/nodespaced-e4b-honest/.


Previous Review Summary

Round 3 item Status in 46bfed40
Finding 1 — control for 8a/8b had zero discriminating power vs H2 ("model keys on enumerated trigger vocabulary") ⚠️ Addressed with new evidence; recorded in the artifact as round 3 asked (rec #2). But the evidence is weaker than the caveat's wording claims. See Finding 1 below.
Finding 2 — n_ctx missing from provenance ❌ Not addressed. Assigned to #1759 per instructions; still absent from this file, noted not re-argued.
Finding 3 — scenario 4's "genuine defect" label unproven, never got rewording-and-retry ✅ Addressed, and the answer is better than the one requested — scenario 4 turns out not to be a model defect at all. Superseded by the scenario-3 false-pass discovery.
Finding 4 — artifact referenced by nothing; eval_commit vs guidance_commit ❌ Not addressed (#1759 owns wiring). Nit, unchanged.
CF-3 (round 2) — record why each scenario failed ✅ Now genuinely addressed. caveats[0..1] supply the causal story the mechanical failure strings could not.

New issues this pass: 3 (1 Important, 2 Suggestions). No blockers.


What I verified independently

Round 3's most useful lesson was that a claim offered in defense of a result deserves the same scrutiny as the result. So I checked Claim B rather than accepting it, and it holds — completely.

✅ Claim B (scenario 3 is a false pass) — CONFIRMED, on four independent lines of evidence.

  1. Schema table. select id from node where node_type='schema' on the honest DB returns exactly the 18 built-ins plus album_to_listen and venue_booking. No equipment/checkout schema exists. Scenario 3 persisted nothing.
  2. Daemon log, daemon-e4b-decontaminated.log:1605:
    tool=create_schema is_error=true duration_ms=0
    args_preview={"name":"Equipment Check-out Tracker","title_template":"{equipment} ({status})"}
    result_preview={"error":"Internal(\"Failed to create schema node for 'equipment_check_out_tracker':
      Node validation failed: Properties validation failed:
      title_template references undefined field 'equipment' — add it to schema.fields\")"}
    
    Exactly as described. duration_ms=0 — rejected at validation, never reached storage.
  3. The assertion cannot detect it. scripts/aichat-matrix.ts:207-216 — the noExtraTypes branch is actions.filter((t) => t === "create_schema").length !== 1. It reads only the tool-name array. is_error is not in TurnResult at all (aichat-matrix.ts:89-99 — toolsCalled: string[]), so no assertion in this harness can see a tool error. The claim that it "does not check the schema persisted" understates it: the harness has no channel through which persistence success could be checked. That is the fix Agent emits malformed tool calls: title_template referencing undefined fields, and calls with empty arguments #1775 needs, and it is bigger than one assertion.
  4. The model's own reply is dispositive — scenario 3's recorded reply is "I encountered a validation error while trying to create the schema… Would you like me to proceed by adding Equipment as a text field?" The scenario scored green on a turn where the agent explicitly told the user it had failed and asked a clarifying question. That is the sharpest possible statement of the false pass, and it is sitting in the artifact this PR ships.

✅ Pre-existing, not introduced by this PR — CONFIRMED. git diff 370782ef~1 HEAD -- packages/agent/src/agent_guidance.rs filtered for TITLE TEMPLATE / title_template / FIELD RULES returns nothing. This PR did not touch the rules that produced the malformed call. The defect is pre-existing and correctly scoped to #1775.

✅ Scenario 4 as downstream — CONFIRMED, and stronger than claimed. Scenario 4's reply is "⚠️ schema creation encountered an error. Please try again or check the details and retry." The model was not confusing instance-with-type; it was still stuck on turn 3's failure and propagating it. Round 3's Finding 3 asked whether "genuine defect" was proven. The answer is now clear: it was wrong, and this commit corrects it. That is the right outcome from round 3's challenge, reached by better means than the control run I proposed.

⚠️ Claim C (8c independent and real) — CONFIRMED as a defect, but the caveat's one-line characterization is imprecise. See Finding 2.


Requirements Check (issue #1760)

# Acceptance Criterion Status Delta this pass
1 No eval prompt verbatim/near-verbatim in guidance ✅ Met Unchanged (guard green, max shared run 3).
2 TOOL_STRATEGY_RULES teaches node_type from RELEVANT ENTITY TYPES ✅ Met Unchanged.
3 Eval prompts reworded, no distinctive shared phrasing ✅ Upgraded from ⚠️ to ✅ Round 3 marked this "met mechanically, weakened in spirit" because 8a/8b were reworded toward trigger vocabulary. The new probes are the first evidence offered on the spirit question, and they point the right way. Residual doubt is Finding 1 — a hedging issue, not an AC failure.
4 Automated check fails the build if a prompt reappears ✅ Met Unchanged.
5 Matrix re-run, new score recorded as honest baseline ✅ Upgraded from ⚠️ to ✅ The baseline is now recorded with its own known defects attached, including one that flatters the score. Recording "one of my nine passes is fake" against your own headline number is the strongest AC-5 evidence in this PR.

All five ACs met. AC-5's residual concern is presentation (Finding 3), not substance.


Code Review Findings

🟡 Important

Finding 1 — caveats[2] overstates what two probes support; three specific sentences need hedging

📁 scripts/results-e4b.json:10 (the caveats[2] string)

Round 3's rec #2 was "record the control's inputs and outcome in results-e4b.json, not only in the commit message." This does that, and the framing sentence ("recorded here so the 8a/8b wording is not re-litigated from git archaeology") names exactly the right reason. Good.

But the recorded conclusion is stated more strongly than two probes license. Taking the sentences one at a time:

Sentence A — "so the DATABASE=SCHEMA rule generalizes and does not merely key on the words database/track/tracker/list." This is the load-bearing overstatement, on three counts:

  • The probe phrasings are not trigger-vocabulary-free in the sense that matters. "Set up somewhere to note down albums…" and "I want a place to jot down albums…" avoid the four enumerated lexemes, but both are still canonical record-keeping English. The rule's semantic content is "a request to start tracking a kind of thing"; "somewhere to note down X" and "a place to jot down X" state that intent about as directly as it can be stated without the listed words. What the probes establish is the narrow, useful fact that the rule is not a literal four-word lookup table. What they do not establish is generalization to phrasings that are semantically oblique rather than merely lexically disjoint. The stated conclusion is the broad one; the evidence is the narrow one.
  • n=2, on a stochastic sampler, both in the same direction. Two successes cannot separate "reliably generalizes" from "generalizes ~70% of the time and I drew two heads" — and the matrix run itself is the existence proof that this phrasing has failed at least once. Which brings up the sharpest problem:
  • The first probe is the exact wording that failed in the matrix, and the caveat does not say so. "Set up somewhere to note down albums I mean to listen to" now fires create_schema; in the matrix it did not. The caveat reports only the success. The same prompt producing both outcomes is not evidence that the rule generalizes — it is evidence that this scenario is non-deterministic, which is a materially different and arguably more important finding for anyone using this as a baseline. A reader of caveats[2] alone cannot learn that a documented failure of that string exists.

Sentence B — the implicit environment claim. Your brief flags this and I agree it is not established. The probes ran single-turn on a fresh chat; in the matrix, 8a is the first turn of a multi-turn group carrying prior context. Those differ in system-prompt length, KV-cache state, and conversation history. The probe therefore does not reproduce the failing condition — so it cannot attribute the difference to wording, environment, or sampling noise. "Environment-dependent rather than vocabulary-dependent" would be an unsupported substitution of one untested explanation for another; to the commit's credit, that phrase does not appear in the JSON. Keep it out.

Sentence C — "(b) scenario 4's wording is not the cause — an alternative instance phrasing behaves identically, because both are downstream of the missing schema." This one I'd rate sound, and it is the strongest sentence in the caveat — because unlike (a) it does not rest on the probe alone. The DB shows no equipment schema and the model's own scenario-4 reply is a propagated error message. Any instance phrasing would fail with no type to instantiate. The probe is confirmatory, not load-bearing.

Recommendation — rewrite caveats[2](a) to claim only what was measured. Concretely:

"(a) Two trigger-vocabulary-free schema requests were probed single-turn on fresh chats after this baseline: 'Set up somewhere to note down albums I mean to listen to' and 'I want a place to jot down albums worth hearing'. Both fired create_schema. The DATABASE=SCHEMA rule is therefore not a literal lookup on the words database/track/tracker/list. Note the first phrasing is the one that FAILED in the matrix run, where it ran as the first turn of a multi-turn group rather than single-turn — so this scenario is non-deterministic across environments, and n=2 successes do not establish how far the rule generalizes to semantically oblique phrasings."

That is longer, but it is the version a future reader can act on rather than one they must re-derive. Non-blocking — a wording change to a caveat, not a defect.

Principle: a recorded conclusion inherits the confidence interval of its evidence, not the confidence of its author. Reporting only the confirming instance of a prompt that has produced both outcomes is selective reporting even when unintended — and it is precisely the failure mode this PR exists to prevent, applied one level up from the guidance to the analysis.


🟢 Suggestions

Finding 2 — caveats[1]'s one-line description of 8c is imprecise; the log shows a different, better-scoped defect

📁 scripts/results-e4b.json:9 · verified against daemon-e4b-decontaminated.log:1681-1684

The caveat says: "8c (create_node retried after an error) is independent and real." The log shows the actual sequence:

1681  tool=create_node is_error=true  args_preview={}
      error: "invalid arguments for tool create_node: missing field `node_type`"
1684  tool=create_node is_error=false args_preview={"content":"Kind of Blue by Miles Davis","node_type":"album_to_listen"}
      result: {"id":"nodespace://3a84ac27-..."}

The first call was malformed — empty arguments, no node_type — and never created anything. The second was correct and created exactly one node. So:

  • The model's behavior was arguably right. A tool call failed on a schema error; it corrected the arguments and retried once, successfully. That is desirable error recovery, not the IDENTICAL TOOL CALLS violation the phrase "retried after an error" implies. The two calls were not identical — the failing one had {}.
  • The real defect is elsewhere, and it rhymes with scenario 3. The model emitted a create_node call with empty arguments — the same class of malformed-tool-call generation that produced scenario 3's title_template-without-fields. Both are argument-construction failures on E4B, and Agent emits malformed tool calls: title_template referencing undefined fields, and calls with empty arguments #1775 already owns the schema half.
  • The assertion is also questionable. toolOnce counts tool names and cannot distinguish "called it twice, both succeeded, two nodes exist" (a real duplicate-write bug) from "one malformed call, one correct call, one node exists" (successful recovery). Same root limitation as Finding B/3 above: is_error never reaches the assertion layer. I checked the DB — the duplicate album_to_listen node visible at log:1706/1719 (count:2) comes from 8d re-creating the same album, not from 8c.

So "independent" is right; "real" is right in the sense that something is wrong; but the characterization will send whoever picks up #1775 hunting for a retry-loop bug that isn't there.

Recommendation: amend to something like "8c is independent: the model emitted one malformed create_node (empty args, missing node_type), then a correct one. Only one node was created — the retry was successful recovery, not a duplicate write. The defect is malformed tool-argument generation (same class as scenario 3's title_template), and toolOnce counting tool names cannot tell recovery from duplication." Worth a line in #1775 since it widens that issue's root cause from "schema validation" to "tool-argument construction," which your brief says is already the revised scope.

Principle: a defect note's value is the debugging time it saves the next reader; a plausible-but-wrong characterization has negative value, because it costs more than no note at all.


Finding 3 — passed: 9/12 needs the asterisk at the headline, not only in caveats[0]

📁 scripts/results-e4b.json:1-6 (label, note)

Your brief asks this directly, and the answer is yes. The file's top-level fields are label, model, recorded_at, guidance_commit, host_memory_gb, note — and caveats sits after note and before the 12-element results array. A reader computing 9/12 from results, or a future runner printing "baseline: 9/12", reaches the number without necessarily reading caveats[0]. The known-false pass is exactly the kind of qualifier that must travel with the number it qualifies.

This matters more than usual here because the file is explicitly a comparison baseline. If a later run legitimately fixes #1775, scenario 3 will pass for real and the score may stay 9/12 or move oddly — and a reader comparing headline-to-headline will draw the wrong conclusion about whether anything improved. The honest comparison target is 8/12 real passes with a known harness blind spot, not 9.

Recommendation: append one sentence to note — "Effective score is 8/12: scenario 3 passes only because the assertion cannot see that its create_schema call failed (see caveats[0])." Cheapest possible fix, and it puts the asterisk where the number is. Optionally add "effective_passed": 8 as a sibling field so the qualifier survives programmatic consumption once #1759 wires this file up.

Principle: a qualifier that a reader can skip is a qualifier that will be skipped. Attach caveats to the number, not adjacent to it.


Nit — does interpretation belong in the results artifact at all?

📁 scripts/results-e4b.json:6-11

Your brief raises the staleness risk: caveats is analysis pinned next to data, and when the numbers are re-recorded the caveats can silently outlive the run they describe. Real risk, but I think including them is the right call, for two reasons: round 3's Finding 1 rec #2 asked for exactly this (the alternative is git archaeology, which does not happen), and caveats[0] is a property of this run, not a general truth — it belongs with this run's data.

The mitigation is structural, not editorial: caveats must be regenerated or dropped whenever results is, never carried forward by hand. Since #1759 owns the runner that will rewrite this file, that is the natural place to enforce it — either have the runner emit caveats: [] on every fresh run (forcing a deliberate re-add), or make the caveat entries carry the recorded_at they were written against. Worth one line in #1759. Not this PR's job.


Recommendation

COMMENT — and to state it explicitly as asked: yes, this PR is merge-ready as it stands.

Round 3 closed with two one-run control experiments as the recommended follow-up. This commit ran both, and the result is better than a tidier number would have been. The scenario-3 investigation in particular is the kind of work that is easy to skip: the author had a green scenario, went looking anyway, found the pass was fake, and wrote that down against their own headline score. Round 3's Finding 3 asked whether scenario 4's "genuine defect" label was proven; the answer came back "no — and the reason is a false pass one scenario upstream, in a scenario I had recorded as passing." Overturning your own favorable result on a challenge is the behavior the whole decontamination effort is trying to institutionalize, and it is worth more than the 9/12.

I verified Claim B end-to-end rather than taking it on report — schema table, daemon log, assertion source, and the model's own reply all agree, and git diff 370782ef~1 HEAD confirms the underlying defect is pre-existing and correctly scoped to #1775. The harness limitation is in fact broader than the caveat states (is_error never reaches any assertion, so no assertion in this file can detect a failed tool call) — useful input for #1775's revised scope.

My reservations are all about wording of the recorded conclusions, none about integrity or code:

  1. Finding 1 (Important) — caveats[2](a) claims "the rule generalizes" on n=2 single-turn probes, one of which is the exact string that failed in the multi-turn matrix. The probes support "not a literal four-word lookup"; they do not support generalization, and omitting that the first phrasing has produced both outcomes hides a non-determinism finding. Hedge the sentence and record the discrepancy.
  2. Finding 2 (Suggestion) — 8c is mischaracterized as "retried after an error." The log shows one malformed call (empty args) then one correct call, one node created: successful recovery, and the real defect is tool-argument construction — the same class as scenario 3.
  3. Finding 3 (Suggestion) — put the false-pass asterisk in note, since passed: 9/12 is readable without ever reaching caveats[0], and this file's whole purpose is headline-to-headline comparison.

All three are single-string edits to a JSON artifact. Any of them can land in this PR in under a minute or ride along with #1775; none justifies holding the merge. n_ctx (round 3 Finding 2) remains absent and remains #1759's — noting for continuity only, not re-arguing.

Merge readiness: unchanged from round 3 — merge-ready. The contamination guard, this PR's actual deliverable, is sound and mechanically verified. The baseline is now more honest than it was two commits ago, because it documents a defect that inflates its own score.

@malibio malibio left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review (fourth pass): COMMENT — merge-ready as it stands (posting as a comment; GitHub blocks self-approval). Full review in the PR comment above.

Round 3 closed asking for two control experiments. 46bfed40 ran both, and one of them overturned a result in the author's own favor — scenario 3 was recorded as passing, was investigated anyway, and turned out to be a FALSE PASS that was then written down against the headline 9/12. Round 3's Finding 3 ("is scenario 4's 'genuine defect' label proven?") is answered: no, and the reason is one scenario upstream. That is the right outcome, reached by better means than the control I proposed.

I verified Claim B independently rather than on report — all four lines agree: the honest DB has album_to_listen and venue_booking but no equipment schema; daemon-e4b-decontaminated.log:1605 shows create_schema is_error=true on a title_template validation error at duration_ms=0; aichat-matrix.ts:207-216 counts tool names only; and scenario 3's own recorded reply is the model telling the user it failed and asking a clarifying question. git diff 370782ef~1 HEAD -- agent_guidance.rs confirms the defect is pre-existing and correctly scoped to #1775. Worth passing to #1775: the harness limitation is broader than the caveat states — is_error never reaches TurnResult, so no assertion in this file can detect a failed tool call.

Three non-blocking findings, all single-string edits to the JSON:

  1. Important — caveats[2](a) claims the DATABASE=SCHEMA rule "generalizes" on n=2 single-turn probes. The probes support the narrower "not a literal four-word lookup." More importantly, the first probe string is the exact wording that FAILED in the matrix (as turn 1 of a multi-turn group), and the caveat records only the success — which hides a non-determinism finding rather than establishing generalization.
  2. Suggestion — 8c is mischaracterized. The log shows one malformed create_node (empty args, missing node_type) then one correct call, one node created. That is successful recovery, not a retry-loop; the real defect is malformed tool-argument generation, same class as scenario 3.
  3. Suggestion — put the false-pass asterisk in note. passed: 9/12 is reachable without ever reading caveats[0], and this file exists to be compared headline-to-headline.

None of these justifies holding the merge. n_ctx remains absent and remains #1759's — noted for continuity only.

malibio and others added 2 commits July 28, 2026 16:30
Sweeping the open non-blocking items from all three review rounds.

Round 3 Finding 2 - n_ctx omitted. Recovered from the run's daemon log
("Creating chat LlamaContext (n_ctx=32768...)") and recorded, with a note
on why it matters: 32768 is far above the ~6600-token agent system prompt,
so this baseline was NOT context-starved. That rules out the failure mode
#1759 documents, where every turn dies on "Context window exceeded" and the
two no-tool assertions still score as passes. Previously a reader had to
infer this from the latency spread.

Round 3 Finding 4 nit - guidance_commit alone is ambiguous. The run measured
805a60e's prompts (8a/8b were reworded there) against da12629's guidance
constants, so eval_commit is now recorded alongside guidance_commit.

Round 3 Finding 4 - a reader skimming "passed: 9/12" may never reach
caveats[0] and learn that one of the nine is a confirmed false pass. Added a
top-level `score` field carrying the asterisk inline.

Round 1 Finding 6 - longest_shared_run O(n.m): re-checked rather than
inherited. The corpus is much larger now (seeded skill nodes and the base
system prompt joined it since that call), so the YAGNI verdict was worth
retesting. Guard still runs in 0.03s. Leaving the readable algorithm.

Rounds 1 and 2 have no remaining open items.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DkqjrfV1G6kugsEjvyU9wU
Fourth-pass review findings, all in the baseline artifact.

8c was mischaracterized. I wrote it up from the tool-NAME sequence
(create_node, create_node) as a blind retry violating IDENTICAL TOOL CALLS.
The log shows otherwise: the first call had EMPTY arguments and failed
("missing field node_type"), the second supplied correct arguments and
succeeded. The calls were not identical and the recovery was reasonable.
The real defect is emitting an argument-less tool call - the same
argument-construction failure class as scenario 3's title_template without
a fields array, and a third instance (create_schema with args_preview={}).
Left as-is this would have sent whoever picks up #1775 hunting a retry loop
that does not exist; that issue is corrected too.

caveats[2] overstated the control probes. "The rule generalizes" is broader
than n=2 supports: both probes are still canonical record-keeping English,
neither reaches oblique phrasing, and the sampler is stochastic. More
importantly the first probe string is the exact wording that FAILED as
scenario 8a in this same run - same prompt, both outcomes - so the honest
finding is that 8a is non-deterministic, and reporting only the passing
instance was selective even though unintended. Also noted the probes ran
single-turn while 8a runs first-in-group, so they do not reproduce the
failing condition.

Headline asterisk: effective_passed=8 added, and the note now says the raw
9/12 may not move when #1775 fixes scenario 3 for real, so comparisons
should use effective_passed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DkqjrfV1G6kugsEjvyU9wU
@malibio
malibio merged commit ebf8350 into main Jul 28, 2026
2 checks passed
@malibio
malibio deleted the worktree-issue-1760-decontaminate-guidance branch July 28, 2026 14:47
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.

Agent guidance contains verbatim aichat-matrix prompts — the eval is measuring recall of planted examples, not generalization

1 participant