Skip to content

emrg: delete the restated memory-index thresholds, don't render them - #1222

Merged
argszero merged 1 commit into
masterfrom
feature/the-thresholds-are-deleted-not-restated
Sep 14, 2026
Merged

argszero merged 1 commit into
masterfrom
feature/the-thresholds-are-deleted-not-restated

Conversation

@argszero

Copy link
Copy Markdown
Owner

What this does

The three numbers a MEMORY.md index is held to have one owneremrg/memory.py:90-92 (INDEX_TITLE_MAX_CHARS, INDEX_COUNT_WARN, INDEX_SIZE_WARN) — and the store states them for itself when it warns. Nine places in static text restated them by hand, four of them wrongly. This PR deletes every restatement; it does not render the numbers from the constants.

Host directive (rant 2026-09-14T13:23:04): "应删除复述,而不是改成从常量渲染;且应一个 PR 扫完所有实例" — delete the restatements rather than rendering them, and cover every instance in one change.

The harm is not untidiness. system.j2:70 is injected into every round of every session, and it said "if a memory index exceeds ~50 entries, consolidate" while the cap it fires on is 100 — so the agent would merge memories it was still meant to keep, and would learn that the numbers in its own instructions cannot be trusted.

Sites deleted (the full scan, one PR)

Site Restated Was it right?
emrg/server/daemon.py embed-truncation notice the cap's size in KB right, but the notice's job is to say what it cut — the cut amount (over) is a runtime measurement and stays
emrg/server/daemon.py reflection hygiene note title ≤512 chars right
emrg/server/daemon.py ⚠️ soft-cap line ~50-entry soft cap wrong (cap is 100)
emrg/server/daemon.py reflection instructions title ≤512 chars right
emrg/server/prompts/system.j2:69 title ≤512 chars right
emrg/server/prompts/system.j2:70 exceeds ~50 entries wrong — the widest-reaching copy
emrg/server/prompts/system.j2:71 may exceed 50 (bare, following on from line 70) wrong — not in the rant's table; found by scanning, deleted for the same reason
emrg/server/open_source_prompt.md:530 ≤512 chars + ~50 entries wrong
emrg/server/evolution_prompt.md:388 ≤512 chars right

Rule intent is kept at every one of those sites — pure index, one short line per entry, update in place — verified on the rendered text, not just the source: system.j2 rendered with its memory branch on (6897 chars), open_source_prompt.md and evolution_prompt.md rendered through the real TaskHandler._build_evolution_prompt (34865 / 32223 chars). None carries a threshold, all keep the intent.

The evolution prompt's separate archiving rule ("keep at most the 50 most recent cycle rows") is a different number for a different thing and is untouched.

Editing emrg/server/evolution_prompt.md is the documented exception — the forbidden-rule allows it when the rant is prompt-specific, and this rant names that file and line.

Code that reads the constant is kept (it is not a restatement)

  • _cap_memory_index's limit = INDEX_SIZE_WARN (emrg: read the embedded index cap from the number the store warns by #1218's change) stays; only the prose in its notice went.
  • The store's own warnings and the ⚠️ line's actual entry count and byte size stay — those are measurements.
  • scripts/archive-memory-index.py held a second copy of the title cap (ROW_MAX_CHARS = 512, plus a docstring saying "the 512-char cap"). It now reads INDEX_TITLE_MAX_CHARS, so the number has one owner and the tool measures the tree it stands in. Its output still reports the row length and the cap, both read from the constant at runtime.

No guard was added

Per the directive as well: with the numbers deleted there is nothing left to police, and a new "no thresholds in prompts" guard would be machinery for a state that no longer exists. Test changes here are only consequences of the deletions — the embed-notice assertion now measures the runtime cut amount instead of the deleted cap size, and three tests that re-spelled a cap read the constant instead.

Verification

  • Acceptance grep over the tracked tree: no static restatement left. The only hits are emrg/memory.py:92 (the definition itself, its single owner), emrg/_stop_all.py:617 and emrg/session.py:30 (unrelated numbers: a dict preallocation, an LLM-log size).
  • pytest tests/ -q1956 passed, 1 skipped.
  • Mutant check on the retargeted assertion: a notice that reports the cap's size as the amount cut (truncated {limit} instead of {over}) is killed; the file was restored byte-exact (sha256 asserted both sides).
  • python3 -m emrg --help, from emrg.client.app import run_client, scripts/check-doc-count.py → all green.
  • scripts/archive-memory-index.py --check on a real index → OK: the index respects the row rules (the new import works both standalone and under the test suite's file-path loader).

Supersedes

The memory store owns the three numbers a MEMORY.md index is held to
(`emrg/memory.py`: the title cap, the entry-count soft cap, the size soft cap)
and states them when it warns. Nine places in static text restated them by
hand, four of them wrongly — the per-round system prompt told every session to
consolidate at 50 entries when the cap it fires on is 100, so the agent would
merge memories it was still meant to keep, and would learn that the numbers in
its own instructions cannot be trusted.

Host directive (rant 2026-09-14T13:23:04): delete the restatements, do NOT
render them from the constants, and do every instance in one change.

Deleted: the reflection hygiene note's title limit; the soft-cap line, which
named an entry count that never fired; the reflection instructions' title
limit; the cap's size in the embed-truncation notice (the amount actually cut
is a runtime measurement and stays); system.j2's three lines (the per-round
injection — the widest-reaching copy); open_source_prompt.md's hygiene bullet;
evolution_prompt.md's row rule. The rule intent — pure index, one short line
per entry, update in place — is kept at every site.

Kept, because they read a constant instead of restating it: the embed cap
reading `INDEX_SIZE_WARN`, the store's own warnings, and the soft-cap line's
actual entry count and byte size. The archiver held a second copy of the title
cap (`ROW_MAX_CHARS = 512`); it now reads the store's constant, so the number
has one owner. No guard was added — with the numbers deleted there is nothing
left to police (also per the directive).

Supersedes #1219 and #1220, which take the render-from-constants route this
directive rejects and each cover only part of the class.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

✅ LGTM — reviewed this cycle (cyc20260914-134826), as Committer.

The claim is that the index thresholds are deleted, not rendered — so I checked the deletion rather than the diff's prose.

Independent verification (detached worktree at head 02fb4065, base == master tip, so the diff is the landing diff):

  1. Acceptance grep over the tracked tree — static restatements of the index caps (512, ~50-entry, 50KB, 100 entries) in index-cap contexts: clean. The only surviving hits are unrelated uses of the same digits: packaging/assets/icon.svg coordinates, blake2b-512, package-lock.json sha512- hashes, emrg/tools/grep_tool.py's own 512KB file-size limit, and the 245125e commit SHA quoted in two merge-tool docstrings.
  2. Including tests/ — same grep: clean. No test left asserting the restated literal, so the string cannot be re-introduced by a later test edit.
  3. The cure is "one owner", not "a better restatement"scripts/archive-memory-index.py now imports INDEX_TITLE_MAX_CHARS from emrg.memory and sets ROW_MAX_CHARS from it, so there is no second copy of the cap to drift. The sys.path insert is needed because the suite loads that file by path (spec_from_file_location) — verified by running the file's own tests, they pass.
  4. Runtime measurements are kept, thresholds are not — the truncation notice now reports truncated N chars (what it actually cut) instead of the cap's own size, and the hygiene note still reports the live {count} entries / {size} bytes while dropping "~50-entry". Deleting the measurement would have been the wrong edit; only the restatement goes.
  5. Focused tests: test_archive_memory_index.py + test_memory.py + test_memory_reflection.py → 55 passed; test_daemon.py -k "cap_memory or embed_cap" → 3 passed. CI on this head is green for both test and test-windows (run 34810173498).

This closes the family the earlier cycles split into #1219/#1220 (both closed, superseded here): prose restatements in the three prompt surfaces plus the code's own second copy of the number.

Not merging as the submitting cycle's reviewer — 1/3 so far.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

✅ LGTM — reviewed this cycle (cyc20260914-142528), as Committer.

Reviewed head 02fb4065 in a detached worktree, so every number below is measured on that tree (confirmed emrg.__file__ resolves inside the worktree, not the main checkout — an editable install otherwise silently measures the wrong tree). CI green both jobs (run 34810173498).

Verified this cycle

1. The acceptance grep is clean on the head's own tree. Scanning the head for the restated index thresholds leaves only two hits, and neither is a restatement: emrg/memory.py:92 is the constant that owns the number, and emrg/session.py:30 is the unrelated LLM-log byte size. Nothing in the two prompt files, daemon.py, or system.j2 states a threshold any more.

2. Every deletion kept the rule. I read all nine removals rather than trusting the summary: what is removed is always the number, never the instruction. Row shape still reads "keep MEMORY.md a pure index: one short line per entry"; the consolidation advice still reads "if the index has grown long, merge/consolidate instead of appending"; the archiver's description still says "a row over the per-row cap". The replacement direction (delete, don't render a second copy from the constant) is followed throughout, and the code that reads the constants is untouched.

3. The archiver really has one owner. scripts/archive-memory-index.py now does ROW_MAX_CHARS = INDEX_TITLE_MAX_CHARS — an alias of the store's constant, not a second copy. I checked the claim is robust to how the file is loaded: as a subprocess from a foreign cwd (the sys.path.insert of the repo root works), and as a module loaded by path (the suite's spec_from_file_location idiom). Both import successfully; the cap has one owner.

4. Focused suite green in the head's tree: 55 passed (test_memory.py, test_archive_memory_index.py, test_memory_reflection.py).

Non-blocking notes

  • The comments added at the deletion sites cite the rant and explain why the number is the store's to state. That is the right place for the reasoning, and it keeps the intent auditable after the number is gone.
  • One thing I deliberately did not ask for: no guard was added to prevent a future re-spelling. That is consistent with the directive — nothing is written to the prompts, so there is nothing to watch. A guard would have to encode the number it exists to keep out.

Correct, minimal, and it removes the failure mode rather than restating it differently. Approving.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

✅ LGTM — reviewed this cycle (cyc20260914-145611), as Committer. This is the 3rd ✅ (after cyc20260914-134826 and cyc20260914-142528), on the same head 02fb4065, with no ❌ in between.

What I verified, in this PR's own tree (detached worktree at 02fb4065)

The host's acceptance grep is clean. Over the tracked tree, the only index-cap-context hits are the single owner, emrg/memory.py:90/92 (INDEX_TITLE_MAX_CHARS = 512, INDEX_SIZE_WARN = 50 * 1024). Everything else the pattern catches is unrelated: icon-512/icon-256 in packaging/ + DEVELOPMENT.md, the 512 * 1024 grep-tool file-size limit, a ~50-minute phrase in a script docstring, and a two-cycle-old commit hash containing 245125e. No template, prompt or daemon string restates a cap.

Every deletion keeps the rule and drops only the number. All nine sites (daemon.py ×4, system.j2 ×3, open_source_prompt.md, evolution_prompt.md) still state the rule — "MEMORY.md stays a pure index (one short line per entry)", "if the index has grown long, merge/consolidate instead of appending" — and the runtime measurements stay where they belong: the embed notice reports truncated {over} chars (what it actually cut) and the hygiene warning reports {store.count} entries / {index_size} bytes. What went is precisely the restatement of a threshold.

A check the earlier two votes did not run: does the code path still follow the constants? A restated number cannot move with a constant; a read of it can, and the real risk in a deletion PR is that the enforcement goes with the text. I drove the actual _maybe_reflect_memory path with a mocked LLM and inspected the rendered hygiene section, four arms (probe: tests/test_probe_1222_145611.py, run in a throwaway worktree, not committed):

  • default constants, small index → no warning block, rule text intact;
  • daemon.INDEX_COUNT_WARN = 0 → the warning fires;
  • daemon.INDEX_SIZE_WARN = 1 → the size arm fires on its own;
  • neither arm names a cap value (512 / 50KB / ~50 / 50 entries / 100 entries absent).

Branch: 4 passed. Master (f15e1b88): only the last arm fails. That is the honest split — the condition always read the constants; what master got wrong was the message text, and this PR is what removes it. So the deletion is behaviour-preserving where behaviour was already right, and the arms prove it rather than assuming it. (First draft of arms B/C asserted the new wording, which failed on master for a wording reason — i.e. measured my own phrasing. Rewritten to assert the warning block's own opening, so they measure the tuning, not the prose. Nothing here is claimed on a probe I did not correct.)

Focused suite at this head: tests/test_memory.py tests/test_archive_memory_index.py tests/test_daemon.py tests/test_memory_reflection.py → 202 passed. scripts/archive-memory-index.py imports INDEX_TITLE_MAX_CHARS and sets ROW_MAX_CHARS from it (sys.path idiom + noqa: E402), so the title cap has one owner instead of five. CI green on both jobs at this head.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

✅ LGTM — cyc20260914-145611, as Committer, cast on the landing tree rather than on the head's CI run.

check-merge-freshness.py 1222 now reports STALE (head 02fb4065, behind_by=1) because #1221 landed as 0aa9d582 after this head was pushed. The tool's own remedy is not to refresh — a rebase moves the head and voids the three valid votes already cast — but to measure the tree this merge would actually produce and say so here. Done:

  • check-merge-order.py 1222 → base 0aa9d582, 0 of 0 pairs conflict, "mergeable, and merging it dirties nothing else" — so no resolution is needed and the head can stay where it is.
  • check-merge-plan-suite.py 1222 → applies cleanly, final tree 634c1abcab12, suite 1957 passed, 2 skipped. That is the same tree and the same result the pair plan produced before #1221 landed (check-merge-plan-suite.py 1221 1222634c1abcab12, 1957/2), which is the expected outcome: #1221 touched scripts/check-vote-count.py and tests/test_check_vote_count.py, and this PR touches emrg/memory.py, emrg/server/* and four other test files — disjoint, so the merged content is unchanged.

For the record on the collected-count arithmetic, since a plan worktree is not the main checkout: master's own tree collects 1957 (1955 passed + 2 skipped in a worktree with no untracked node_modules; 1956 + 1 in the main checkout). The pair tree collects 1959 = master + exactly the two tests #1221 adds. This PR's test changes therefore delete none — the count moves by the other PR's two additions and nothing else.

The three earlier ✅ (cyc20260914-134826, cyc20260914-142528, cyc20260914-145611) were all cast on head 02fb4065 and none of them is void — the head has not moved. This review adds the landing-tree reading on top, so a Committer can merge on evidence about the tree that will exist, not about one that no longer can.

@argszero
argszero merged commit 28da8c6 into master Sep 14, 2026
2 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.

1 participant