feat(optimizer): context-compression + memory optimizer for Claude Code - #466
initializ-mk wants to merge 18 commits into
Conversation
Adds `forge optimizer` — a local server Claude Code (or any Anthropic Messages API client) routes through via ANTHROPIC_BASE_URL. It meters real token usage and, with compression on, shrinks bulky conversation content before it reaches the model, serving context_expand so the model can retrieve anything dropped. Engine (forge-core/optimizer): - streaming pass-through proxy with real billed-usage metering + gateway chaining - reversible ctxzip compression on the outbound live zone (cache-stable: works on raw JSON, deterministic so provider prompt caches keep hitting) - context_expand retrieval, either over MCP-HTTP or resolved in-band - ephemeral per-process compression store (retrieval cache; no cross-session value) - local episodic memory distilled off the wire at task boundaries, per-session frozen recall injection (cache-safe), background procedural consolidation, and confidence feedback (explicit + ambient outcome/corroboration, staleness decay) - memory scoped per session to the working-directory repo, not the launch dir CLI (forge-cli/cmd): - `forge optimizer claude` wraps Claude Code (auto-attach to a running proxy; auto-registers context_expand over MCP) - `forge optimizer start|stop|status` — detached always-on daemon that wires ~/.claude/settings.json so every `claude` routes through it (reverted on stop) - `forge optimizer savings|bench|stats|memory` — local views + offline benchmark Deps: bump ctxzip v0.3.0 -> v0.4.1 (tighter error-keep + adaptive keep-budget crushers; also benefits forge-core/compress and the markdown converter). The forge-ui dashboard page for the optimizer follows in a separate PR (its app.js integration is entangled with other in-flight UI work).
Surfaces the optimizer in the forge dashboard:
- forge-ui/handlers_optimizer.go: /api/optimizer/{stats,savings,memory,daemon}
read views + memory delete/feedback + daemon start/stop control (shells the
forge binary, so the UI performs the exact same settings-merge/revert).
- server.go: register the routes.
- app.js: an "Optimizer" nav item + page with a Savings tab (live stats +
windowed $), a Memory tab (episodes/procedures table, confidence bar with
👍/👎, detail drawer, delete), and a daemon status banner with Start/Stop.
The savings ratio was saved/compressed-blocks-before, which conflates "how well the compressed blocks shrank" with effectiveness on the addressable surface. Add two honest denominators: - EligibleTokens: pre-compression size of EVERY block the compressor examined as a candidate (compressible role, past the size gate), whether or not it shrank. saved/eligible = what compression achieves on what it can actually touch. - TotalTokens: estimate of the whole outbound request (system + tools + all messages). saved/total = the diluted "of everything sent" (includes the frozen prompt + incompressible content compression can't act on). CLI savings + the forge-ui savings rows now show "X% of compressible · Y% of all sent". Legacy usage-log records (no eligible/total) fall back to the prior compressed-before denominator.
…ecords The eligible/total ratios summed SavedTokens across ALL records but the new denominators only for records written by the newer build, so numerators included savings whose denominators were missing — yielding >100%. Now the numerator and denominator come from the same record set: - EligibleTokens gets a per-record legacy fallback (compressed-before) so it covers every record and SavedTokens is a valid numerator. - TotalSaved accumulates saved only for records that carry TotalTokens, so the "% of all sent" ratio never mixes sets. Displays clamp to 100% defensively.
- `stop` now kills the proxy by its listening port (verified as a forge optimizer via /healthz) when no pid was recorded — fixes the "did not start the running proxy myself — leaving it up" case for adopted proxies. - `start --ui` records the dashboard pid; `stop` stops it too. `start` also records an adopted proxy's pid so a later `stop` can end it. - No-state `stop` still does a best-effort full teardown (settings + MCP + proxy).
Replace the eligible/total dual-ratio savings metric with a single honest
ratio: saved / (cache_read + cache_creation) — the billed cache token volume
that compression actually shrinks. The prior "of compressible / of sent"
denominators mixed old and new usage-log records (producing >100% skew) and
diluted the number with the frozen prompt, cached reads, and output.
Dollars now credit the cache-WRITE rate (~1.25× input) the dropped content
would have incurred, via Pricing.CacheWriteCostAvoided, since the saved bytes
are conversation history Claude Code caches.
- pricing.go: add CacheWriteCostAvoided
- usagelog.go: WindowTotals = {saved_tokens, cache_tokens, cost_avoided_usd};
aggregate cache = cache_read + cache_creation; $ = cache-write rate
- compress.go: drop eligible/total stat plumbing
- optimizer_savings.go + app.js: single "N% saved X / Y (cache read+write)" row
Cache reads re-count the entire cached prefix on every turn, so a read+write denominator diluted the ratio ~30× (0.4% instead of a meaningful figure). Switch the denominator to cache-CREATION (write) tokens only — the freshly-cached bytes each turn adds, which is the traffic compression directly shrinks. WindowTotals.CacheTokens → CacheWriteTokens (json cache_write_tokens); UI/CLI label "cache write". Dollars unchanged (cache-write-priced saved tokens).
The dashboard header valued saved tokens at 1× input list price while the Savings tab uses the 1.25× cache-write rate — same saved counts, different dollars. Align the header to cache-write (optimizerCacheWritePrice = input × 1.25) so both views agree; relabel the tile 'cache-write rate'.
… cache-read)
Value saved tokens across all three billing tiers they would have incurred
without compression, per the corrected model:
- uncached INPUT (1×) and CACHE-WRITE (1.25×) on the turn a chunk is first
removed, split by that request's input:cache_creation ratio;
- a compounding CACHE-READ (0.1×) for every LATER turn in the session that no
longer re-reads the removed chunk — tracked via a per-session running
cumSaved as records stream in chronological order.
The compounding read term dominates (a chunk removed early avoids a re-read on
every subsequent turn), which is the real economic value of keeping the prefix
small across a long session.
- pricing.go: CostAvoidedBreakdown(input, cacheWrite, cacheRead); drop the
single-tier CacheWriteCostAvoided
- usagelog.go: WindowTotals gains avoided_{input,cache_write,cache_read}_tokens;
new all_time window; per-session cumSaved drives the read term
- optimizer_savings.go: print the avoided-token breakdown line
- app.js: savings rows show the breakdown; header Cost-avoided tile sources the
durable all-time three-tier figure (falls back to client cache-write estimate)
…able Add a Cost-avoided column to the Live sessions table, sourced from the durable usage-log report's per-session three-tier figure (indexed by session id). It climbs as the current session's turns accumulate — the log is appended each turn and the dashboard re-fetches every 5s. Token counters stay live/in-memory; only the dollar figure comes from the durable compounding calc.
Rework the dashboard for credibility (addresses the live-vs-durable scope mismatch that made per-token economics look inflated): - Single scope: header tiles and the session table now source the DURABLE usage-log report, so token counts and dollars share one all-time scope. New ReportTotals carries all-time requests/tokens/spend/avoided. - Ground it: track actual SpendUSD per record/window/session (Pricing.SpendUSD) and show "avoided vs spent", leverage (×), and Effective savings % (avoided / (avoided+spent)). The progress bars now plot effective savings, so bar and dollar tell one story. - Explain the multiplier: a callout states the saved tokens vs the compounding cache-reads avoided, so the large read term reads as mechanism, not padding. - Show composition: per-tier avoided dollars (input/write/read) surface as a stacked bar + line (Pricing.AvoidedTiers). - Merge the Live + Previous session tables into one durable table with a live dot for currently-active sessions.
…drop <1× leverage)
…(htm trims whitespace at element boundaries)
- errcheck: wrap unchecked Close() in tests and fmt.Fprint* in the CLI (optimizer.go, optimizer_bench.go) - govet: check the http.Get error before using fresp in session_test.go (was "using fresp before checking for errors") - staticcheck S1021: merge the non-recursive find closure decl+assign - staticcheck QF1008: drop the embedded Totals selector (ps.add) Verified: golangci-lint run ./... = 0 issues across all three modules; go vet + go test ./... green.
…iters) TestRecall_WrapsStringSystem intermittently failed in CI with "directory not empty": the former's fire-and-forget store writers (recall recording, consolidation, distillation) could write into the test's t.TempDir() while RemoveAll was running. Track those goroutines on a MemoryFormer.bg WaitGroup and add a test-only newFormer(t, cfg) helper that registers former.waitAsync via t.Cleanup — LIFO ensures it drains before the TempDir is removed. Routed all test former constructors through the helper. Verified: go test -race -count=5 ./optimizer/ green; golangci-lint = 0 issues.
The Build matrix (which only ran once Lint/Test passed) failed on windows/*:
optimizer_daemon.go used POSIX-only syscall.Kill and SysProcAttr{Setsid},
which don't exist on Windows.
Extract the three OS-specific operations behind build tags:
- processAlive(pid), terminatePID(pid), detachSysProcAttr()
- optimizer_daemon_unix.go (!windows): signal 0 probe, SIGTERM, Setsid
- optimizer_daemon_windows.go (windows): os.FindProcess handle check,
Process.Kill (TerminateProcess), DETACHED_PROCESS
Verified: builds for linux/darwin/windows × amd64/arm64; golangci-lint = 0
issues (native and GOOS=windows); go test ./cmd green.
initializ-mk
left a comment
initializ-mk
left a comment
There was a problem hiding this comment.
Review — context-compression + memory optimizer for Claude Code
A large, well-architected subsystem. I reviewed the security core (proxy, daemon, MCP expand, compression-store lifecycle) plus the memory subsystem and the CLI/pricing/UI surface. No HIGH-severity findings — the credential posture is genuinely careful (caller auth forwarded untouched, none at rest; distillation auth headers used only for the side call and never persisted — Episode has no header field; stores at 0600/dir 0700; telemetry off by default, no hidden endpoint). All 10 CI checks green. Issues are two security-hardening MEDIUMs, a savings-accounting MEDIUM, a memory-correctness cluster, and LOWs.
🟠 MEDIUM #1 — forge-ui optimizer endpoints are unauthenticated with Access-Control-Allow-Origin: *
corsMiddleware sends ACAO: * and there's no auth/CSRF, so while the dashboard runs, any website the user visits can fetch('http://127.0.0.1:<port>/api/optimizer/memory') and read it cross-origin — returning distilled episodic memory (Summary/Lesson/Files/TaskSignature, i.e. summaries of the user's private code work) — and POST /api/optimizer/daemon/start|stop, which rewrites ~/.claude/settings.json. Inherits forge-ui's pre-existing no-auth-localhost posture but materially widens it with sensitive read + state-changing endpoints (live now, even though the dashboard UI is deferred). Fix: drop ACAO: * for these routes (or gate with same-origin/CSRF), treat daemon start/stop as state-changing.
🟠 MEDIUM #2 — "avoided cache-read" savings are an unbounded optimistic estimate shown as concrete dollars
usagelog.go credits every previously-compressed token in the session as an avoided cache-read on every subsequent turn, so the headline $ avoided grows ~S·N(N-1)/2 (100 turns × 10k saved ≈ 49.5M "avoided read tokens", 10–50× the real per-turn value). Comments call it directional, but forge optimizer savings + the dashboard render it as a hard figure. Fix: label it an upper-bound estimate or cap the compounding horizon.
🟠 MEDIUM #3 — memory staleness decays against the wrong repo's commit
memory_recall.go gates the ×0.85 staleness penalty on f.commit — the launch-dir commit — not the session repo's resolved commit (Inject discards the resolved commit; computeRecall never receives it). For a proxy serving multiple repos, episodes from repo B are checked against repo A's HEAD → false decay or none. Breaks per-session scoping on the staleness axis, and it's untested. Fix: thread the resolved commit into computeRecall.
🟠 MEDIUM #4 — frozenBlock compute is non-atomic → concurrent first-turns can break the cache-safety guarantee
frozenBlock releases the mutex, computes recall unlocked, then re-locks to cache. Two overlapping first-turns of one session both miss, both compute, and each returns its own block (last writer wins the cache); if the store changed between them, the two in-flight turns inject non-identical blocks — the exact cache-bust the design forbids — plus double-recorded recall. Sequential turns are fine (tests pass), but it isn't atomic. Fix: hold the lock across compute, or single-flight per session.
🟠 MEDIUM #5 — memory scope basename-collision merges distinct repos
memory_scope.go default key is filepath.Base(cwd), so ~/a/client and ~/work/client collapse to "client" and cross-inject. Fix: include a path hash, or require the git-toplevel resolver.
🟠 MEDIUM #6 — failed distillation is silently never retried (episode lost)
Observe advances distilled[sessionID] to completed before dispatch; on failure dispatch deletes the span from seenSpans "to allow retry," but a later Observe returns early (completed <= already), so it's never revisited — a transient 401/timeout permanently drops that task's memory. Fix: roll back distilled on failure (or drop the misleading delete+comment).
🟠 MEDIUM #7 — test gaps on load-bearing invariants
No pricing.go tests (the dollar math); no test for feedback bounds under extreme input, the staleness branch (would've caught #3), or concurrent first-turns (would've caught #4); no regression guard that the usage log/reports never contain prompt content or keys.
🟡 LOW cluster
Listennot guarded to loopback (server.go) — non-loopback bind → open forward-proxy +/statsexposure; warn/refuse.~/.claude/settings.jsonwritten 0644 (should be 0600) and non-atomically (temp+rename;claudereads it concurrently).- Distilled
errors/summarycan persist a secret if the distiller echoes one (durable at 0600, no new wire exposure) — consider a scrub pass. - cwd is client-supplied → memory scope spoofable (fine under local single-user trust; note it).
- Ephemeral store not removed on SIGKILL/crash (temp bbolt with prompt content lingers).
- Upstream URL with embedded creds echoed to banner/0600 log;
[1m]long-context models priced at base rate (under-report); UI returns raw FS error strings; unbounded per-request telemetry goroutines (2s-capped); UTF-8 rune-splitting in transcript truncation; orphaned recall/feedback on delete (content-hash IDs resurrect stale aggregates); no-git default → staleness inert.
✅ Verified correct
- No key leakage: proxy forwards caller auth untouched with none at rest; distillation auth used only for the side call, never persisted; usage log + telemetry token-counts-only; telemetry off by default, no hidden endpoint.
context_expandMCP tool is a content-addressed hash lookup bounded to this process's store — no file/path access, no traversal, no cross-session leak, miss non-fatal, logs no content.- Stores 0600/dir 0700; ephemeral store
MkdirTemp+RemoveAllon shutdown; daemon preserves other settings.json keys and cleanly reverts. - Pricing rates + cache multipliers match the current reference exactly.
- Feedback math bounded (∈ (0,1], no NaN/negative/overflow); frozen recall byte-stable across sequential turns (real invariant test); scoping correct on the normal path; consolidation single-flighted, goroutines bounded (no leak).
Note — ctxzip v0.4.1 + reversibility
The bump is a library I've reviewed before; the reversibility mechanism here is sound (store holds the Original, marker→context_expand returns it byte-exact). Caveat worth documenting: reversibility is bounded by the ephemeral store's 30-min TTL and process lifetime — a marker expanded after eviction/restart misses (non-fatal, but that turn's dropped content is unrecoverable). Deterministic re-compression refreshes entries for content still in the live window, mitigating within an active conversation.
Verdict: changes requested — no HIGH, but MEDIUM #1 (cross-origin memory disclosure + daemon control) and the memory-correctness pair (#3 wrong-commit staleness, #4 frozen-block race) are the ones to prioritize; the rest is hardening/accounting/test coverage.
| "store_path": path, | ||
| "count": len(eps), | ||
| "recalled_count": recalled, | ||
| "episodes": eps, |
There was a problem hiding this comment.
🟠 MEDIUM #1 — this returns private episodic memory to any cross-origin caller. The route is wrapped only in corsMiddleware, which sends Access-Control-Allow-Origin: * with no auth/CSRF. So any page the user visits while the dashboard runs can fetch('http://127.0.0.1:<port>/api/optimizer/memory') and read these episodes (Summary/Lesson/Files/TaskSignature = summaries of the user's private code work) cross-origin, and POST .../daemon/start|stop (which rewrites ~/.claude/settings.json). Drop ACAO:* for these routes or gate with a same-origin/CSRF check; treat daemon start/stop as state-changing.
| } | ||
| conf := EffectiveConfidence(e.Confidence, fb[e.ID]) | ||
| // Staleness: down-weight memory formed against a now-moved commit. | ||
| if f.commit != "" && e.CodeState.Commit != "" && e.CodeState.Commit != f.commit { |
There was a problem hiding this comment.
🟠 MEDIUM #3 — staleness decays against the wrong repo's commit. f.commit is the launch-dir commit, but the episode was scoped to the session repo (Inject resolves the repo then discards the commit; computeRecall never receives it). For a proxy serving multiple repos, an episode from repo B is staleness-checked against repo A's HEAD → false decay when they differ, no decay when B has moved. Thread the resolved commit into computeRecall and compare against that. (Also untested — a multi-repo recall test would catch it.)
|
|
||
| // frozenBlock returns the session-frozen, task-agnostic recall block (block, | ||
| // count, ids), computing it once on first use. | ||
| func (f *MemoryFormer) frozenBlock(sessionID, repo string) (string, int, []string) { |
There was a problem hiding this comment.
🟠 MEDIUM #4 — non-atomic frozen-block compute can break the byte-stable-recall guarantee under concurrency. This releases f.mu, runs computeRecall unlocked, then re-locks to store the cache. Two overlapping first-turns of the same new session both miss, both compute (each also firing RecordRecall + a log line), and each returns its own block; last writer wins the cache. If the store changed between the two computes, the two in-flight turns inject non-identical blocks — the exact cache-bust the design forbids — plus duplicated recall attribution. Sequential turns are safe (so tests pass), but the guarantee isn't atomic: hold the lock across compute, or single-flight per session.
…labeling, tests MEDIUM #1 (cross-origin memory disclosure + daemon control): the optimizer endpoints return private distilled memory and can rewrite ~/.claude/settings.json. corsMiddleware no longer sends ACAO:* for /api/optimizer/* and refuses cross-site requests via the Fetch Metadata Sec-Fetch-Site header (blocks cross-origin reads and CSRF); same-origin dashboard and non-browser clients still work. MEDIUM #3 (wrong-commit staleness): thread the SESSION repo's resolved commit through Inject → frozenBlock → computeRecall and judge staleness against it, not the launch-dir f.commit. MEDIUM #4 (frozen-block race): hold f.mu across computeRecall so overlapping first-turns of a session can't each compute and return non-identical (cache- busting) blocks. computeRecall takes no lock, so no re-entrancy. MEDIUM #5 (basename scope collision): the no-git fallback appends a short FNV path hash so ~/a/client and ~/work/client don't collapse to one scope. MEDIUM #6 (lost distillation): bounded retry — roll back distilled[session] and seenSpans on transient failure so a later turn revisits the span; give up after maxDistillAttempts to avoid a permanent-failure storm. MEDIUM #2 (optimistic savings shown as hard $): label cost-avoided an upper-bound estimate (≈, "up to", est.) in the UI tiles, callout, and CLI; the compounding cache-read note now states the cache-warm assumption. LOW (settings.json): write 0600 and atomically (temp+rename); mkdir 0700. MEDIUM #7 (test gaps): add pricing_test.go (rates, prefix match, unknown fallback, overrides, AvoidedTiers, SpendUSD) and tests for the staleness session-commit path (#3), concurrent frozen-block byte-stability (#4, -race), basename-collision disambiguation (#5), and transient-distill retry (#6). Verified: go test -race -count=3 ./optimizer green; golangci-lint = 0 issues (native + GOOS=windows); all six cross-compiles build.
Review fixes — pushed in
|
1 participant
Adds
forge optimizer— a local server Claude Code (or any Anthropic Messages API client) routes through viaANTHROPIC_BASE_URL. It meters real billed token usage and, with compression on, shrinks bulky conversation content before it reaches the model, serving acontext_expandtool so the model can retrieve anything dropped.Engine (
forge-core/optimizer)context_expandretrieval, over MCP-HTTP or resolved in-band.CLI (
forge-cli/cmd)forge optimizer claude— wraps Claude Code (auto-attaches to a running proxy; auto-registerscontext_expandover MCP).forge optimizer start|stop|status— detached always-on daemon that wires~/.claude/settings.jsonso everyclauderoutes through it, and cleanly reverts on stop.forge optimizer savings|bench|stats|memory— local dollar/token views + an offline compression benchmark.Dependency
Bumps ctxzip v0.3.0 → v0.4.1 (tighter error-keep + adaptive keep-budget crushers). Also benefits the existing
forge-core/compressand markdown converter. Supersedes #465 (the standalone bump).Tests / scope
go build ./...and the optimizer + compress test suites pass on all three modules. This PR is the engine + CLI; the forge-ui dashboard page is a follow-up (itsapp.jsintegration is entangled with other in-flight UI work and will be grafted separately).