Repository navigation
GIT-74 token audit + GIT-76 field-drop fix (draft, stacked on #22) - #23
Merged
Merged
Conversation
…/ GIT-50) Measures what gitmem injects into agent context, per surface, against the real corpus. Built as a re-runnable script rather than hand-collected numbers because the acceptance criterion is a before/after table — the "after" column has to be produced the same way as the "before". Method, stated because it qualifies every number: - o200k_base (GPT-4o BPE) via gpt-tokenizer, installed OUTSIDE the repo so no dependency lands in package.json days before a version bump. Not Claude's tokenizer; absolute counts carry modest error, deltas are sound. - Display strings only — structured fields that never render are not injection cost. - Read-only against live data. Write-path shapes are measured under the outage env, where nothing durable is produced. - Credentials read from the MCP config programmatically, never printed. Two false starts are baked in as guards, because both produced numbers that looked like data: 1. The first run reported recall at 133 tokens, identical for match_count 8 and 3. That was not a measurement — it was an RPC error string. The audit had called recall before background cache init finished and hit a cold-start fallback. Now gated on cache-status, and it says loudly when the cache never warms rather than publishing the fallback path. 2. An isolated GITMEM_DIR discards the warm scar cache; a cold load returns learnings with no embeddings, degrading recall to weak keyword matching. The live section now uses the real gitmem dir. Read-only tools only — no session is registered, no record written. DUMP=1 prints every measured string. A measurement you cannot read is a number you cannot defend. Recall is measured at two deliberately different yields. Cost is driven by how many scars clear the similarity threshold, not by match_count — with a narrow query only one scar clears and match_count never binds, so a single query would report one point and imply it was the cost. Refs GIT-74, GIT-50, GIT-49 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…R13a⚠️ THIS COMMIT IS HONESTLY RED. 9 tests fail in tests/unit/tools/provenance-citation.test.ts. They are named below and are the first task of the next session per R13a. Committed rather than left in a working tree overnight — the sprint preserved a stranger's 519 uncommitted lines on that principle. TIER RENDERING (GIT-74 / R11, R12b) Banded by outcome rate rather than a flat budget: stub <0.55 id + title + score compact 0.55–0.75 + why_this_matters + applies_when extended >=0.75 or top + first counter-argument full explicit fetch by id R12b: renders honest fields or nothing. Description-first-sentence is RETIRED — against the real corpus it produced provenance metadata ("Liberation Blueprint nugget, session 096e940d…") or the title echoed back by variant-enforcement text about as often as a lesson. A fragment that looks like a rendered judgment is worse than no line; it teaches the reader to skim the field. When why_this_matters and applies_when are both absent, the compact tier is the header alone. The per-scar "Full detail" affordance became one footer line — it was spending ~12 tokens × N to say one thing. MEASURED (o200k_base, net of a measured 35-token no-session banner that does not fire in normal use): high-yield recall 2565 → 487 (81%) low-yield recall 246 → 115 (53%) Both inside R12c's amended acceptance (≤750 / ≤115). Chrome cuts stopped at the citation line, the acknowledge line that drives confirm_scars, and one footer — trimming those buys a round number with honesty. APPLIES_WHEN (GIT-76) Assignment moved out of the `win` branch. It sat there since the monorepo extraction (d0530f7, 2026-02-03), so every scar and pattern this package ever wrote validated the field, reported success, and discarded it. Found because the compact tier had nothing to render — the audit failing honestly. NOT fixed here, absorbed into GIT-76 per R13b: problem_context and solution_approach have the identical branch-scoped defect at the identical site, and anti_pattern has no branch at all (so no severity default). They land together behind one parameterized test asserting every schema-accepted field persists for every learning_type — the guard that makes a fifth sibling structurally impossible. CITATION UNIFICATION (in scope, tests pending) One CITATION_LINE constant in display-protocol.ts, imported by recall, search and prepare_context. The consistency test that caught this was passing on a shared prefix over already-divergent wording. RED, pending R13a rewrite to invariants (not prose): - all paths use the same citation rule text - citation protocol uses consistent separator (separator removed by design) - citation protocol appears before results - display includes CITATION RULE ×3 (recall remote, recall free, prepare_context) - includes example with record ID format ([id:48ebca14] example removed) - renders the full body for >=0.55 scars ← encodes the pre-R11 contract - search citation text assertions Pre-authorized fallback if coupling exceeds these nine: revert recall's citation wording, ship low-yield ~133, unification becomes a follow-up. The 487 high-yield figure is unaffected either way. Refs GIT-74, GIT-76, GIT-50, GIT-49 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Branch bff6cc8 was honestly red: 9 tests asserted the prose of the pre-audit citation chrome, which R12/R12b compacted and retired. The tests were pinning wording, not guarantees. Per R13a they now assert what has to hold: (a) all three retrieval surfaces emit the identical CITATION_LINE constant, exactly once, with no drifted second copy. The old "consistency" test compared a shared PREFIX and passed while the three literals had already diverged — that hole is closed. (b) the id-citation format is present: each surfaced row carries the 8-char id: form an agent is being told to cite, replacing the assertion on the retired worked example "[id:48ebca14]". (c) R11 tiering replaces the obsolete "full body at >=0.55". The four bands are asserted off LOW_CONFIDENCE_THRESHOLD/EXTENDED_THRESHOLD rather than restated as literals. The retired separator test is replaced rather than deleted: the box-drawing rule is gone from the retrieval surfaces (it survives only in server.ts/session-start.ts), so it is no longer a provenance guarantee, while single-constant identity is. Coverage: 31 -> 34 tests. Net new coverage is the compact/extended band split and the top-hit escalation rule — the top hit renders extended regardless of score, so a single-scar fixture can never exercise the compact band. Every previous tiering test used a single scar and was therefore testing extended while claiming to test the 0.55 boundary. Invariants shown to bind, not merely pass: - reword CITATION_LINE entirely -> 34/34 still green (prose-independent) - EXTENDED_THRESHOLD 0.75 -> 0.55 -> compact-band test fails - drop id: from recall's header -> id-citation test fails Source restored byte-identical after each mutation; git diff touches only the test file. Full suite: 65 files, 1096 tests, 0 failures. tsc --noEmit clean. Coupling did not extend beyond the 9, so the R13a fallback (revert recall's citation wording, ship low-yield ~133) was not needed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…IT-74/R14)
prepare_context (full) and formatCompact each emitted a citation
instruction while rendering no record ids at all — they commanded the
impossible, and the predictable failure mode is fabricated or absent
citations from sub-agents. prepare_context is the sub-agent injection
path, where provenance matters most downstream.
Per R14, the fix is "add ids", not "drop the instruction":
- prepare-context.ts renders id:<8-char> on each scar header
- format-utils.ts renders id:<8-char> on each compact line
- format-utils.ts:102's fourth un-unified literal now imports
CITATION_LINE. R12 unified recall/search/prepare_context and left
this one behind, so the drift R12 fixed could silently reopen here.
src/ diff is the two renderers plus the import, per the ruling's
constraint. Coupling did not extend further: the only test that broke
was formatCompact's assertion on the retired literal's prose
("factual claims"), which was the prose-coupling R13a exists to remove.
Invariant (b) now binds all four surfaces, including one class-killing
test — "every surface that instructs citation also renders citable ids"
— so a fifth id-less-but-instructing surface is structurally impossible
rather than merely absent.
Mutation-checked to bind, by name; source restored byte-identical:
- drop id from prepare_context -> its id test + the class-killer fail
- drop id from formatCompact -> its id test + the class-killer fail
- restore the drifted literal -> 3 tests fail, incl. the class-killer
MEASURED COST (o200k_base, same ruler as the audit), correcting the
ruling's ~3-token estimate: 6 tok/record for prepare_context, 7 for
formatCompact, and the literal unification COSTS +3 once per payload
rather than saving — the shared constant is longer than the fourth
literal it replaced. At 5 scars: +30 and +38 tokens respectively.
Still affordable honesty, and the recall path is untouched, so the
487/115 high/low-yield figures stand.
Suite: 65 files, 1099 tests, 0 failures. tsc --noEmit clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…T-76/R13b) applies_when was fixed on bff6cc8. Its two siblings were not: problem_context and solution_approach had the identical defect at the identical site — assigned only inside the `win` branch, so a scar, pattern or anti_pattern that submitted them had them validated, acknowledged and discarded. anti_pattern had no branch at all and so reached the row with no severity: the same omission one level up. Per R13b all four are normalized together rather than fixing one field and shipping the same bug three more times beside its own fix: - applies_when / problem_context / solution_approach move to a universal block applied for every learning_type - anti_pattern gets a severity default ("medium", matching win's neutral default — a cheap correction if it should sit higher) - win keeps its empty-string floor for the two prose fields, so its stored shape is unchanged as the acceptance criterion requires The scope guard is R13b's single class-killing test, not four targeted ones: every schema-accepted field must persist to the stored row for every learning_type, asserted against the row handed to directUpsert rather than the tool's success message. A success message describing submitted content instead of stored content is the defect class 1.7.0 exists to close. counter_arguments is asserted scar-only, confirming the per-field intent R12a asked for rather than assuming it. Shown to FAIL against the pre-fix code, by name — the tests were run against stashed HEAD and 7 failed, exactly the siblings: scar/pattern/anti_pattern x problem_context, solution_approach (6) anti_pattern severity (1) applies_when passed pre-fix, correctly, because bff6cc8 already fixed it. +40 tests (1 new file). Suite: 66 files, 1139 tests, 0 failures. tsc --noEmit clean. Still open on GIT-76: the eleven-learning recovery (R13c). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…only)
Consumer verification for GIT-69's 2026-08-05 defect: session_start's panel
showed a foreign-project thread while list_threads correctly excluded it.
Runs both surfaces against the LOCAL BUILD over stdio — the session's own MCP
tooling serves published 1.6.6 and would green-light unfixed code — then
resolves every panel entry's true project by reading orchestra_threads
directly, because a tool confirming itself is not confirmation.
READ-ONLY against orchestra_threads per the acceptance-venue ruling
(GIT-70 comment 10b75c26): production is evidence, never fixture. session_start
does insert its own session row in orchestra_sessions — an ordinary operational
write, not a seeded fixture — and the header says so rather than hiding it.
Two instrument defects were found and fixed while building this, both of which
produced FALSE FAILURES and either could have produced a false pass on a
differently-shaped defect:
- the panel renders truncated thread TEXT and no ids, so an id regex over it
returns zero matches — which reads as "no leak" but means "not measured"
- thread rows carry lifecycle status "emerging", not a literal "open", so
status=eq.open excluded the very rows being matched against
Both are the measurement-validity class (scar 7e8bda56). The script therefore
carries an instrument self-test: it feeds the detector a thread that really
does belong to another project and asserts a LEAK is reported. A repro that
cannot fail is not a repro.
Result on the branch build: panel claims 2 / prints 2, list_threads 2, parity
exact, both gitmem, no weekend_warrior threads. Self-test detects a real
orchestra_dev thread.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ain it
The first run of this harness registered its session in the REPO's
.gitmem/active-sessions.json even though GITMEM_HOME pointed at a temp
directory. The server resolves its state directory by walking UP from the
working directory looking for .gitmem, and that walk-up beats the env var.
Consequence, observed: five enforcement unit tests flipped to failing
("warns when recall called without session" and four siblings). They were
not broken by any code change — they resolve the same real .gitmem, adopt
whatever session they find there, and so stop seeing a no-session state.
Clearing the orphaned entry returned the suite to 1139/1139 green with no
source change.
Two findings, both worth their own tickets:
1. GITMEM_HOME is not sufficient to relocate server state; the cwd walk-up
wins. Any tool or test that sets it and assumes isolation does not have
isolation.
2. tests/unit/services/enforcement.test.ts is not hermetic. Its result
depends on whether a session happens to be registered on the developer's
machine, so it can go red locally with clean code, and could go green
against a real no-session defect if a stray session exists.
Fixed here by pinning the child's cwd to the sandbox as well as GITMEM_HOME.
Re-verified: repro passes, repo registry stays empty, suite 1139/1139.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Gate item (c) includes smoke-pro. The suite could not execute against a live
store, and had been auto-skipping all sprint on describe.skipIf(!HAS_SUPABASE)
because no credentials existed — so it was reporting green by never running.
mcp-client.ts's own comment predicted exactly this: "a suite that silently
skips its only real assertion is the same lie as a green message over an
unwritten row."
Bounds honoured: tests/ diff only, zero src/ changes.
- parseToolResult: tools with a `display` field return rendered text and no
machine-data blob (server.ts:417-430, deliberate). Calling it on those
surfaced as a bare SyntaxError; it now names the contract instead.
- querySessionRow: sibling of queryThreadRow. Since the response no longer
carries a performance object, "did session_start reach Supabase" is
answered by the row it wrote — a stronger assertion than the one it
replaces, which could pass on a latency number with nothing persisted.
- recall: asserts the render, and no longer assumes a populated corpus. The
venue is a deliberately blank project; "returns semantic matches" was a
fixture assumption. Now: a cited hit or an explicit miss, never neither.
- smoke server runs with cwd + GITMEM_HOME in a sandbox. Without it the
suite resolved the developer's real .gitmem (GIT-80) and session_close
tried to sync local threads into the smoke target, producing foreign-key
violations that were local state leaking into a test, not product defects.
Result against gitmem-e2e: 4/5, up from 2/5, verified by reading the parsed
output rather than the green count.
The 5th failure is NOT instrument staleness and is NOT fixed here — it is a
real product/schema defect the repaired instrument caught within minutes:
session_close spreads the local session record into the Supabase upsert, and
that record carries a `display` field no store defines. Confirmed absent from
schema/setup.sql AND from production orchestra_sessions. It fails HONESTLY
("close · FAILED", "Failed to persist session"), which is 1.7.0's contract
working as designed — on 1.6.6 this reported success. Fixing it needs src/ or
a schema change, both outside R16a's bounds. Posted for ruling.
Also observed, not fixed: analytics queries `public.scar_usage` unprefixed,
bypassing GITMEM_TABLE_PREFIX — same class as GIT-73's hardcoded
`gitmem_query_metrics`. Non-fatal.
Unit suite 1139/1139, tsc clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
session_close builds its Supabase payload by spreading the existing session
record. On the Supabase-miss path that record is the LOCAL file record — a
different shape, carrying rendering fields like `display`. Postgres rejects the
whole upsert on the first unknown key (PGRST204), so one local-only field
failed the entire close:
((●)) gitmem ── close · FAILED
Failed to persist session: Could not find the 'display' column
of 'gitmem_sessions' in the schema cache
A fresh Pro install provisioned from schema/setup.sql could not close its first
session. Not a regression — 1.7.0 made a pre-existing silent failure loud.
Fixes the CLASS, not the instance: filter to the known column set, so any
future local-only field is inert rather than fatal. Adding a `display` column
was rejected — a rendering field does not become persisted state to appease an
upsert.
THE COLUMN SET IS A UNION, DELIBERATELY. schema/setup.sql is the canonical
source of truth, but it is STALE: the live production table carries ELEVEN
columns it does not define (children, claude_code_session_id, compacted*,
insights, metrics, task_observations, blocked_by, handover_linear_slug,
pre_compaction_summary) and lacks updated_at, which setup.sql has. Filtering to
setup.sql alone would have silently dropped real production data on every
close — the same disease pointed the other way, and worse, because data loss
reports nothing. Both origins are enumerated and commented in
src/services/session-columns.ts. The setup.sql/production drift is itself a
defect, tracked separately; do not trim this list until they agree.
Shown to FAIL against pre-fix behaviour per scar d72bd8c0: with the filter
mutated to a pass-through (which is what the code did before), 3 tests fail by
name — the `display` drop, the any-unknown-key drop, and the realistic close
payload. The 5 inverse-guard tests correctly still pass, since pass-through
never dropped a real column.
Verified at the persisted row, not the banner: session 1e5908a8 in gitmem-e2e
carries close_compliance.close_type="quick". Every pre-fix run in that project
(02:01–02:10) shows only "superseded" or none — no quick close had ever landed
before this commit.
smoke-pro against gitmem-e2e: 5/5 (was 2/5, then 4/5 after the instrument fix)
full suite: 67 files, 1147 tests, 0 failures
tsc --noEmit clean
Not touched, files as 1.7.1: analytics queries public.scar_usage unprefixed,
bypassing GITMEM_TABLE_PREFIX (GIT-73's family).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #22 (
feature/GIT-70-sync-completeness). Draft — merge, bump, gate and tag remain gated on Chris.Commits
dc6021dbff6cc8applies_whenfix (was honestly red in 9 tests)0f420360908352cd9cb98learning_typeBranch is green: 66 files, 1139 tests, 0 failures.
tsc --noEmitclean.What changed
Tiered scar rendering (R11). stub
<0.55/ compact0.55–0.75/ extended>=0.75or top hit / full on blocking verification. Measured cut: high-yield recall 2565 → 487 tokens (81%), low-yield 246 → 115 (53%), both inside R12c's amended acceptance.One citation constant, four surfaces (R12/R14).
recall,search,prepare_contextandformatCompactall emitCITATION_LINE.prepare_contextandformatCompactpreviously instructed agents to cite record IDs while rendering none — they now render theid:form, because an instruction ships only on surfaces that render the capability to obey it.Field persistence (GIT-76/R13b).
applies_when,problem_contextandsolution_approachwere assigned inside thewinbranch only, so every otherlearning_typehad them validated, acknowledged and discarded.anti_patternhad no branch at all and reached the row with no severity. All four normalized;win's stored shape unchanged.Tests
+40 on
cd9cb98alone. Two class-killing invariants rather than targeted assertions:learning_typeBoth assert against the stored row or the rendered surface — never a tool's own success message.
Every new invariant was mutation-checked to bind by name, with source restored byte-identical after each: reword
CITATION_LINE(stays green — proves prose-independence), breakEXTENDED_THRESHOLD, drop an id render, restore the drifted literal, and stash the GIT-76 fix (7 fail, exactly the siblings).CI note
Per the stack's known trap,
ci.ymlfires onpull_request: branches: [main]only, so this PR will report no checks. To verify: retarget the tip tomain, close and reopen (a base change firespull_request: edited, which is not in the default types), then retarget back. The durable fix is filed as a GIT-45 line item and deliberately not executed —ci.ymlis the release-path file.Refs GIT-74, GIT-76, GIT-50.