feat(store,server,claude-code): bound the SessionStart context injection - #685
feat(store,server,claude-code): bound the SessionStart context injection#685neuralabmarketing wants to merge 4 commits into
Conversation
FormatContext renders four fixed-size sections into the SessionStart blob: sessions (5), prompts (10, hardcoded), unpinned observations (cfg.MaxContextResults) and pinned observations, which have had no LIMIT at all since Gentleman-Programming#484. None of it is reachable from outside the store. Adds ContextOptions{Observations, Prompts, Sessions, Pinned, Compact} and FormatContextWithOptions. Each int field follows one convention: 0 keeps that section's legacy default, >0 caps it, <0 omits the section and its header. Compact drops the 300-char body preview from observation-shaped bullets. FormatContext keeps its signature and becomes a thin wrapper over a zero-value ContextOptions, so its four callers (cmd, server, mcp) are untouched and the output is byte-identical -- asserted by a dedicated test. PinnedObservations also keeps its exported signature and delegates to an unexported helper that adds SQL LIMIT only when a positive cap is requested. Refs Gentleman-Programming#163 Co-authored-by: todie <7841714+todie@users.noreply.github.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VtshktvJgt8fVkBr5ZjT1J
handleContext now builds a store.ContextOptions from observations, prompts, sessions, pinned and compact, reusing the existing queryInt/queryBool helpers: an absent, empty or unparseable param falls back to the zero value, so garbage input keeps returning 200 with the legacy blob instead of a 4xx. Negative values are passed through deliberately -- they are how a caller omits a section entirely -- unlike the 'err == nil && n > 0' guard in Gentleman-Programming#162, which would have silently dropped them. Refs Gentleman-Programming#163 Co-authored-by: todie <7841714+todie@users.noreply.github.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VtshktvJgt8fVkBr5ZjT1J
Both session-start.sh and post-compaction.sh now fetch the context with compact=1&pinned=20. compact drops the 300-char body preview from observation bullets while keeping every row and title -- the body is one mem_get_observation away -- and pinned=20 puts a ceiling on the only section that never had one. Measured end to end by running the hooks against a 2,976-observation project: 9,404 B -> 3,179 B injected per session start (-66%), with no row dropped. This is the only commit in the series that changes observable behavior; the store and server commits are mechanism with byte-identical defaults. Drop this one if you would rather choose the numbers yourself. Refs Gentleman-Programming#163 Co-authored-by: todie <7841714+todie@users.noreply.github.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VtshktvJgt8fVkBr5ZjT1J
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe ChangesContext options and retrieval
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant ClaudeHook
participant ContextEndpoint
participant Store
ClaudeHook->>ContextEndpoint: GET /context?compact=1&pinned=20
ContextEndpoint->>Store: FormatContextWithOptions(ContextOptions)
Store-->>ContextEndpoint: Compact bounded context
ContextEndpoint-->>ClaudeHook: .context response
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/server/server.go`:
- Around line 784-790: Update the API specification for the GET /context
endpoint to document the observations, prompts, sessions, pinned, and compact
query parameters, matching the names and default behavior used by the
ContextOptions initialization. Keep the handler and tests unchanged.
In `@internal/store/store_test.go`:
- Around line 8846-9089: Add error-path subtests for FormatContextWithOptions
that force each section fetch—Sessions, Prompts, Observations, and Pinned—to
fail, such as by closing or otherwise invalidating the store database before
invocation. Verify the method returns a non-nil error for every failure path,
while keeping the existing happy-path and option-isolation tests unchanged.
In `@internal/store/store.go`:
- Around line 3436-3460: Extract the duplicated observation bullet formatting
from the pinned and recent-observation loops into a shared helper or local
rendering function, preserving the existing Compact and full output—including
truncate(obs.Content, 300). Update both loops to reuse that single rendering
path while keeping their section-specific slices and headings unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: eb1ef138-05e2-4141-906b-e938013dfbe8
📒 Files selected for processing (6)
internal/server/server.gointernal/server/server_e2e_test.gointernal/store/store.gointernal/store/store_test.goplugin/claude-code/scripts/post-compaction.shplugin/claude-code/scripts/session-start.sh
| opts := store.ContextOptions{ | ||
| Observations: queryInt(r, "observations", 0), | ||
| Prompts: queryInt(r, "prompts", 0), | ||
| Sessions: queryInt(r, "sessions", 0), | ||
| Pinned: queryInt(r, "pinned", 0), | ||
| Compact: queryBool(r, "compact", false), | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: check whether docs mention the new /context query params.
rg -n -i 'context.*(observations|pinned|compact)=' --glob '*.md'
fd -i 'api|http' --extension mdRepository: Gentleman-Programming/engram
Length of output: 166
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate files =="
git ls-files | sed -n '1,200p' | rg -i '(^README|api|http|server|route|handler|test)' || true
echo
echo "== internal/server/server.go relevant lines =="
sed -n '740,830p' internal/server/server.go | nl -ba -v740
echo
echo "== route registration mentioning context =="
rg -n 'context|Context' internal/server -g '*.go'
echo
echo "== markdown files =="
git ls-files '*.md'
echo
echo "== broad /context mentions =="
rg -n -i '/context|\/context|context\s+queries|context.*query|query.*context|ContextOptions|observations|prompts|sessions|pinned|compact' -g '*.md' -g '*.go' .Repository: Gentleman-Programming/engram
Length of output: 2551
🏁 Script executed:
#!/bin/bash
set -u
echo "== context query handling and route registration =="
rg -n 'observations|prompts|sessions|pinned|compact|ContextOptions|/context|context' internal/server internal -g '*.go' | head -200
echo
echo "== markdown files in repo =="
find . -maxdepth 4 -type f -iname '*.md' -print | sed 's#^\./##' | sort
echo
echo "== docs/search for context query parameters in markdown files only =="
find . -maxdepth 4 -type f -iname '*.md' -print | xargs -r rg -n -i 'observations|prompts|sessions|pinned|compact|context' || trueRepository: Gentleman-Programming/engram
Length of output: 50385
Document the new /context query parameters.
GET /context now accepts observations, prompts, sessions, pinned, and compact; the API spec needs the same update as the handler/test changes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/server/server.go` around lines 784 - 790, Update the API
specification for the GET /context endpoint to document the observations,
prompts, sessions, pinned, and compact query parameters, matching the names and
default behavior used by the ContextOptions initialization. Keep the handler and
tests unchanged.
Source: Path instructions
… paths, document params - Extract the Compact/full bullet rendering shared by the Pinned and Recent Observations loops into writeObservationBullet, so a future formatting change (truncate length, redaction) lands in one place instead of two copies that can drift apart. - Add TestFormatContextWithOptionsErrorBranches: four subtests that close the store before invoking, each using the option convention to skip the earlier sections so every 'return "", err' branch is reached (sessions, pinned, observations, prompts). - Document the five query params on GET /context in DOCS.md, including the 0 / >0 / <0 convention and that invalid values fall back to the default rather than returning 400. Output is unchanged: the byte-identity test still passes untouched. Refs Gentleman-Programming#163 Co-authored-by: todie <7841714+todie@users.noreply.github.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VtshktvJgt8fVkBr5ZjT1J
|
Addressed all three review comments in
Output is unchanged — the byte-identity test still passes without being touched. On the inconclusive Linked Issues check: it's right that this PR doesn't implement the env-tunable limits described in #163's original writeup. That's deliberate — @Alan-TheGentleman's approval narrowed the scope to "a bounded-size injection. The scope is the size cap", and I offered in that thread to build a hard server-side ceiling instead of (or on top of) per-caller params if that's the shape he prefers. I'd rather wait for that call than widen the diff on a guess. |
🔗 Linked Issue
Closes #163
🏷️ PR Type
type:feature— New feature📝 Summary
ContextOptions{Observations, Prompts, Sessions, Pinned, Compact}, one convention for all four ints — 0 keeps the legacy default · >0 caps · <0 omits the section and its header.LIMITat all since fix(memory): pin critical observations in context #484 and was the one part of the blob that could grow without any ceiling.FormatContextkeeps its signature and output; a zero-valueContextOptionsreproduces today's bytes exactly, and a test asserts precisely that.📂 Changes
internal/store/store.goContextOptions+FormatContextWithOptions;FormatContextbecomes a thin wrapper; unexported pinned query withLIMIT(exportedPinnedObservationssignature untouched)internal/store/store_test.goTestFormatContextWithOptions— 11 subtests: byte-identity for zero value, cap/omit per section with cross-checks that siblings are untouched, compact, pinned capinternal/server/server.goGET /contextacceptsobservations,prompts,sessions,pinned,compact, reusing the existingqueryInt/queryBoolhelpersinternal/server/server_e2e_test.goTestContextQueryParamsE2E— no params ≡ legacy, caps shrink, negative omits, garbage input still 200plugin/claude-code/scripts/session-start.sh,post-compaction.shcompact=1&pinned=20📊 Measured on a 2,976-observation project
Real install,
projectscope, ~4 months of daily use — measured against a snapshot of that database, not synthetic data:prompts=-1observations=10compact=1compact=1&prompts=-1compact=1&prompts=-1&observations=10End to end, running the hook scripts themselves against that database: 9,404 B → 3,179 B per session start.
That last combo lands at −89%, which reproduces @todie's −88% from #161/#162 on an install roughly 15× larger — the win holds as history grows, because every section is a fixed row count regardless of database size.
🤔 The default I chose, and why it's conservative
The hook commit asks only for
compact=1&pinned=20:compact=1drops the 300-char body preview from observation bullets. No row disappears — every type and title still renders, and the body is onemem_get_observationaway when the agent actually wants it. That alone is −66%.pinned=20gives the blob a real ceiling. Without it there is still no hard cap, since a user who pins 40 observations to make context better silently makes the injection several times larger.I deliberately did not default
prompts=-1, even though it's another −14%: your v1.19.0 notes describe the context block as "load-bearing", so which sections earn their bytes is your call, not mine. The table above is there so you can pick a stronger default in one line if you want it.🙏 Credit
The
ContextOptions+ thin-wrapper shape is @todie's design from #162, rebased onto currentmainand credited withCo-authored-by:on every commit. #161/#162 were closed on process, not merit; @todie filed first, did the original measurement, and hasn't been active since April. This picks it up and extends it to what #162 predates: the hardcodedRecentPrompts(project, 10)and the unbounded pinned section from #484. If @todie wants to take it back, I'll close this.✅ Verification
Run locally on this branch, plus a clean
upstream/mainworktree as a baseline for comparison:go test ./...— 22 packages ok, 0 failures, identical to the baseline rungo test -tags e2e ./internal/server/...— okgofmt -lclean ·go vet -tags e2e ./internal/server/...cleanbash -nand executed against an isolated daemon (ENGRAM_DATA_DIRpointed at a snapshot, never the live database)📋 Note for review
The three commits are meant to be separable: store (mechanism), server (transport), hooks (the only behavior change). Happy to reshape any of it — including switching to a hard server-side ceiling that no caller can exceed, if you'd rather the bound not be up to the caller at all.
Summary by CodeRabbit
GET /contextto support query-driven, per-section caps for observations, prompts, sessions, and pinned items, plus acompactmode to reduce inline preview content.0/invalid values use legacy defaults,>0caps results, and<0omits the section (including its header).GET /contextAPI documentation for the expanded query interface and defaulting/error behavior.