Skip to content

docs(agent): the memory rule pointed at a header the build removed - #154

Merged
geisten merged 1 commit into
mainfrom
docs/agent-memory-rule
Sep 25, 2026
Merged

geisten merged 1 commit into
mainfrom
docs/agent-memory-rule

Conversation

@geisten

@geisten geisten commented Sep 21, 2026

Copy link
Copy Markdown
Owner

What

.agent/AGENT.md told contributors to route all dynamic memory through deps/geist/heap.h, "resolved via -Ideps/geist". Neither half is reachable any more:

  • heap.h lives in libgeist's private src/base/, not in its public include directory.
  • -I$(GEIST_DIR) was dropped with the arena wrapper in v0.3.1 so geistshell does not depend on engine internals — the Makefile comment above CPPFLAGS says exactly that, and CPPFLAGS now carries -I$(GEIST_DIR)/include only.

So the rule is unfollowable as written: no file in the tree includes heap.h, and the handful of places that genuinely need dynamic memory call malloc directly — the only option the build leaves them.

Change

State the architecture that actually holds:

  • Caller-provided buffers and explicit workspace/scratch memory are the default, and that is what makes the spine replayable and portable to constrained targets.
  • heap.h is deliberately out of reach; do not reintroduce the include path or vendor a copy.
  • malloc needs a reason written at the call site. Today that is the macOS process table, whose size only the kernel knows (src/machine/backend_macos.c, which carries such a comment), plus file slurping in the CLI surface.
  • Prefer freeing in the allocating function; where ownership transfers, say so and name who releases it.

No code changes — this only makes the rule match the build.

Testing

Not applicable: .agent/AGENT.md is not a build input.

Two things this PR deliberately leaves alone

  • The two malloc sites in src/cli/main.c carry no reason at the call site, which the rule asks for (old wording and new). Adding those comments is a separate, tiny change.
  • Unlike geistlib, which machine-checks its allocation-free claim with heap_alloc_count() in tests, geistshell has no CI gate for this rule — it holds by review. Worth considering, but out of scope here.

Found while reading the runtime; the README's pin line was stale the same way, but fix/workdir-and-reserved-memory-names already fixes that, so this PR does not touch it.

🤖 Generated with Claude Code

https://claude.ai/code/session_01LkBr39Z2JxHU1SL63VyPC9


Generated by Claude Code

AGENT.md told contributors to route dynamic memory through deps/geist/heap.h,
resolved via -Ideps/geist. That header is in libgeist's private src/base/, and
that include path was dropped with the arena wrapper in v0.3.1 so geistshell
depends on the engine's public headers only. The rule was therefore
unfollowable: no file includes heap.h, and the few places that genuinely need
dynamic memory call malloc directly.

State the actual architecture — caller-provided buffers by default, heap.h
deliberately out of reach, and the narrow cases where malloc is allowed with a
reason written at the call site.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LkBr39Z2JxHU1SL63VyPC9
@geisten
geisten force-pushed the docs/agent-memory-rule branch from b9564a8 to ff1d512 Compare September 25, 2026 17:54

geisten commented Sep 25, 2026

Copy link
Copy Markdown
Owner Author

Rebased onto main (f90f5b7) — it had moved a long way since this branch was cut, and the stale Pi leg was from before the runner came up.

The rebase also invalidated one of this PR's own claims, so I corrected it in the same commit. The text said malloc/calloc had exactly one justified home in the core (the macOS process table). Since #155 that is no longer true: the geistd adapter allocates four vocabulary-sized buffers in src/model/model_adapter.c, sized by what the served model reports at open time. Both are now named.

Re-verified against current main rather than taken from memory:

claim status
-I$(GEIST_DIR) no longer reaches the engine's private heap.h holds — CPPFLAGS carries -I$(GEIST_DIR)/include only
justified allocations in the core was wrong, corrected: two, not one

A document that misstates the codebase is the exact failure this PR exists to fix, so it seemed worth not shipping a second instance of it.


Generated by Claude Code

@github-actions

Copy link
Copy Markdown

Raspberry Pi 5 — ff1d512e

job success · run 36170032008

Eval-gated improvement


=== eval-gated self-improvement benchmark =====================
train suite     : examples/eval/bench/train.spg
held-out suite  : examples/eval/bench/holdout.spg  (5 cases)
---------------------------------------------------------------
held-out pass   : 3/5  ->  5/5   (baseline -> after learning)
lessons         : 2 proposed, 1 kept, 1 reverted
regressions kept: 0  (invariant: final >= baseline)
---------------------------------------------------------------
benchmark: PASS

Context cost

lessons  geistshell_bytes/tick  full_index_bytes/tick 
1        72                     90                    
4        72                     360                   
8        72                     720                   
16       72                     1454                  
32       72                     2233                  

@geisten
geisten merged commit 206e54f into main Sep 25, 2026
5 checks passed
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.

2 participants