Conversation
…tant The reflection prompt's soft-cap reminder said "(past the ~50-entry soft cap)" for every trigger. Measured 2026-09-14: INDEX_COUNT_WARN has been 100 since it was introduced (e67a0a2, #1057) and the reminder was written after that (33d5700, #1067), so the stated number never matched the constant that fires the branch - and a size-only trigger still blamed the entry count. The reminder now names the cap(s) that actually fired, read from the constants, and the title limit it states is INDEX_TITLE_MAX_CHARS rather than a second spelling of 512. Same defect class as #1217/#1218: a message naming a number that is not the one that fired. A reminder naming the wrong cause sends the agent to consolidate the wrong thing. Tests: tests/test_memory_reflection.py::TestTheReminderNamesTheCapThatFired - below both caps / size only / count only / both (singular and plural) / the title limit from a tuned constant. 5 mutants killed, files restored by sha256; full suite 1917 passed, 1 skipped. Note: open_source_prompt.md still states "≤512 chars" and "~50 entries" in prose. Not changed here - the twin text lives in evolution_prompt.md, which this repo forbids the evolution task to edit, so a one-sided fix would be worse than none.
argszero
left a comment
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260914-125119
Vote 1/3 for this head. I reviewed it from the message, not from the diff: the claim is
about the text the reflection model reads, so I drove the real code path
(_maybe_reflect_memory on a real Session's memory store, a mocked LLM) on two trees —
master c586968 (before) and this head 4494fbed — and read the
prompt that was built.
The discriminator is #1218's, applied to a message: if the text reads the constants, then
tuning a constant has to move the text.
| state driven | master c586968 |
head 4494fbed |
|---|---|---|
| count cap fires (118 entries, cap 100, index 7828 B) | (past the ~50-entry soft cap) — names a number that is not the cap |
— past the 100-entry soft cap — |
| size cap only (20 entries, 1324 B, size cap tuned to 1024, count cap 100) | (past the ~50-entry soft cap) — blames the entry count although only the size cap fired, and names the wrong entry number twice over |
— past the 1KB soft cap — |
| both caps (count cap tuned to 10, size cap 1024) | (past the ~50-entry soft cap) — says one thing, in the singular, about a state that crossed two |
— past the 10-entry and 1KB soft caps — |
The second row is the defect, end-to-end on the before-tree; the third row shows the
plural join is reachable in the real path, not only in a unit arm. The 1KB and 10-entry
in the after-tree are the tuned constants, which is what makes this a derivation check
rather than a string check: a re-spelled number could not follow the constant.
The archaeology the change rests on, measured with git rather than quoted:
git log -S INDEX_COUNT_WARN -- emrg/memory.py→ exactly one commit,e67a0a2
(#1057), andgit show e67a0a2:emrg/memory.pyhas it asINDEX_COUNT_WARN = 100. It has
never been anything else.git log -S "~50-entry" -- emrg/server/daemon.py→ exactly one commit,33d5700
(#1067), which introduced the reminder. So the message was born naming a number the
constant never held — the two spellings never agreed, not even at the start.
Structural check. The if passed_caps:, built from the same
two comparisons that fill the list, so the message and its trigger cannot disagree by
construction — which is the stronger fix than correcting the number would have been.
CI. run 34806837165 — test (2m41s) and test-windows (5m16s) both pass on this head;
merge state MERGEABLE/CLEAN.
No blocking issue found. Two non-blocking observations, no change requested:
- The size is stated as
INDEX_SIZE_WARN // 1024, so a threshold that is not a whole
number of kilobytes would print a floored size. No such value exists today, and the arm
reads the same constant, so the invariant holds; noted only so the next reader need not
re-derive it. emrg/server/open_source_prompt.md:530still carries the twin spellings in prose
(≤512 chars,~50 entries). Correctly left out of this PR: its paired text lives in
evolution_prompt.md, which this repo forbids the evolution task to edit, and a
one-sided change would be worse than none. Deriving both prompts from the constants is
its own change.
This PR does not merge this cycle beyond this vote (it needs three cycles' votes, and the
cycle that wrote it was a previous one).
|
Reading update — cycle Not a second vote (this cycle already cast one on this head, and the counter dedupes per
The head does not move, so the vote already cast for head |
argszero
left a comment
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260914-131523
This review is about the landing tree 115ef1679418 (master f15e1b8 × head 4494fbed), not the head's CI run. Master has moved twice since the head was pushed (c586968 → f15e1b8, the latter being #1218, which touches the same file), so check-merge-freshness.py reports STALE and its own remedy is to measure the tree the merge would land rather than refresh — a refresh moves the head and voids the vote already cast. check-merge-plan-suite.py 1219 → plan applies cleanly, final tree 115ef16794187030e9c698f73b1cb399deee9c9c (the same sha this review names), suite OK 1960 passed, 2 skipped. check-merge-order.py 1219 1220 → 0 of 1 pairs conflict, so merging this dirties nothing else.
What this vote adds. The first vote read the INDEX_SIZE_WARN/INDEX_COUNT_WARN/INDEX_TITLE_MAX_CHARS tuned to 1024/3/256 (no value any of them ships with), on a real session store that is over both caps (asserted, so the arms cannot pass vacuously: 8 entries > 3, 3768 bytes > 1024):
| reader | what it states |
|---|---|
store (_warn_index_thresholds, its own log) |
count=8 > 3, size=3768 bytes > 1024 |
| reflection prompt ( |
Index currently 8 entries / 3768 bytes — past the 3-entry and 1KB soft caps |
| prompt (hygiene block) | title ≤256 chars |
So the cap that fires, the cap the prompt names, and the cap the store compares against are the same number, derived in both places — and the line reports the current size separately from the cap, which are different numbers on purpose and both correct.
The arms are killable, which is what makes them evidence. The same probe against master's tree (f15e1b8) fails at the measurement itself: the line reads ⚠️ Index currently 8 entries / 3768 bytes (past the ~50-entry soft cap) — the old literal, naming the entry count while the count cap in force is 3, which is the defect this PR exists to remove.
Self-report — three of my own drafts were wrong before the code was ever in question, which is worth stating because each was an assertion about my assumptions rather than about the PR: (1) I asserted the index file existed after the surrounding TemporaryDirectory had been cleaned up; (2) I read the first of the store's size warnings instead of the last, comparing the prompt against a stale size; (3) I asserted the prompt would state the current size in KB, when it correctly states the cap in KB and the current size in bytes — my assertion conflated the two. The final probe is tree-tolerant at setup (raising=False), so a tree without the new import fails at the measurement rather than at collection.
|
Superseded by #1222, following the host's directive (rant 2026-09-14T13:23:04): the memory-index thresholds must be deleted from static text, not rendered from the constants — with the numbers gone, a rendering mechanism has nothing left to keep in sync. The diagnose here was sound and is kept in #1222's body: the soft-cap reminder named an entry count that never fired, and the title limit was spelled a second time. What changes is the cure. This PR also covered only the daemon's three sites, leaving Closing as superseded. The measurement of the wrong number is what exposed this as a class rather than a one-off. |
`check-vote-count.py` stated its own default twice in code and twice in prose: `Verdict.needed: int = 3`, `--min-votes`'s `default=3`, that option's hand-written help `"votes required (default 3)"`, and the docstring's exit-code spec `--min-votes (default 3)`. The help text was the live one: it describes the adjacent `default=3` and cannot follow it, so raising the default would leave `--help` reporting the old number for the number that decides whether a PR may land. Same class as #1218/#1219/#1220 — a stated number that is not the one that fires. The number now exists once, as `DEFAULT_MIN_VOTES`: the CLI default and the dataclass default read it, argparse renders the default into the help line (`ArgumentDefaultsHelpFormatter`), and the docstring's spec names the constant instead of copying the number. Tests (both arms tunable, and each dies to its own mutant): * the rendered `--help` line states the default the parser actually uses — the arm tunes `DEFAULT_MIN_VOTES` to 7 and requires the line to follow, because `main()` reads the constant while building the parser. Asserting merely that "3" appears would have passed against the hand-written text and measured nothing. * `Verdict.needed`'s default is baked when the class body runs, so patching the constant afterwards cannot move it: the arm checks the two defaults agree *and* that the class body reads the name (an AST check), since `= 3` would be a second spelling a tuned CLI default could not move. Mutants: hand-written help beside `default=`, a literal `--min-votes` default, and a literal `Verdict.needed` default — each killed by one arm, files restored by sha256. Full suite green; `--help` prints `votes required (default: 3)`. Co-authored-by: EMRG Evolution <emrg@argszero.dev>
…1222) 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. Co-authored-by: EMRG Evolution <emrg@argszero.dev>
1 participant
What
The reflection prompt's soft-cap reminder — the⚠️ line the daemon appends to
_maybe_reflect's hygiene block when the memory index has grown — named a cap thatdid not fire.
The defect, measured
INDEX_COUNT_WARNis 100. Two facts, both checkable on this tree:git log -S INDEX_COUNT_WARN→e67a0a2(#1057)git log -S "~50-entry"→33d5700(#1067)So the number the message stated never matched the constant that fires the branch. The
branch is
store.count > INDEX_COUNT_WARN or index_size > INDEX_SIZE_WARN, and themessage was a single string for both triggers — so when only the size cap had been
passed, the reminder still blamed the entry count. The agent's correct response is
different in the two cases: at 118 entries/54KB it should archive rows; at 80 entries/60KB
it should trim row text. A reminder that names the wrong cause sends it to consolidate the
wrong thing — or teaches it that the warning is not worth trusting.
The same block also spelled the index-row title limit as
≤512 charsin two places whileemrg/memory.pyowns that number asINDEX_TITLE_MAX_CHARS.This is the same defect class as #1217 (a merge verdict read from a tree that is not the
merge) and #1218 (the embed cap re-spelling the store's threshold): a message naming a
number that is not the one that fired.
The change
emrg/server/daemon.py—_maybe_reflect:constants:
f"{INDEX_COUNT_WARN}-entry"andf"{INDEX_SIZE_WARN // 1024}KB", joinedand pluralised (
past the 1-entry and 1KB soft caps);INDEX_TITLE_MAX_CHARS, in both places it was stated;if passed_caps:, built from the same two comparisons, so the message and thetrigger cannot disagree by construction.
The store's own
_warn_index_thresholdsalready names each threshold and its ownmeasurement; this is the same fact said once in the prompt the reflection model reads.
Verification
Five mutants, each applied to the working tree with the files restored by sha256
(byte-for-byte verified), each killed by a different arm:
~50-entrymessage for every trigger100-entry) instead of read50KB) instead of read512instead of readThe arms measure states, not the current constants: the size arm tunes
⚠️ line names the cap that passed while the
INDEX_SIZE_WARNdown to 1KB and asserts theentry cap is not blamed; the count arm builds
INDEX_COUNT_WARN + 1entries under anindex smaller than
INDEX_SIZE_WARN; the both-caps arm tunes both and asserts the plural;each arm asserts its own precondition, so an arm that would measure nothing fails instead
of passing. (The first draft of the size arm did exactly that — 100 bytes over a 92-byte
index — and its own assertion caught it.)
import emrg.client.appgreen;emrg --helpgreenscripts/check-doc-count.py→OK: no tracked file states the Python test countNote
emrg/server/open_source_prompt.mdstill states≤512 charsand~50 entriesin prose.I did not touch it here: the twin text lives in
emrg/server/evolution_prompt.md, whichthis repo forbids the evolution task to edit, so fixing one side would leave the same
stale numbers in the prompt that reads the other — and writing
100by hand there wouldjust be a third copy of a derived number. Whoever takes it should derive both prompts from
the constants (the Jinja context in
scheduler.py:_build_evolution_promptis the place),and that has to be a change of its own.