fix(health-check): warn before MEMORY.md outgrows the session read limit by sonichi · Pull Request #2449 · sonichi/sutando

sonichi · 2026-07-31T10:25:42Z

The gap

check_memory_index_integrity exists to catch "a memory that will never load." It covers two modes — a file not referenced in MEMORY.md, and one stranded in a *-BACKUP tree — and misses the third, which has by far the largest blast radius: the index itself outgrowing the per-session read limit.

Past that point the file isn't truncated, it's skipped — so every indexed memory stops loading at once. Modes (a) and (b) lose one memory each. This one loses all of them.

It also arrives silently: every memory file on disk still looks healthy, the probe still emits its usual warn, and nothing anywhere mentions the size.

Why now

Observed on this fleet 2026-07-31: MEMORY.md reached 22.0KB against a 24.4KB limit — a few more filed memories from dropping a 250-entry index — and nothing caught it. It was found by accident while compacting, not by a check.

Reconstructing that exact size against this patch:

22.0KB index -> warn | MEMORY.md is 22.0KB, approaching the 24.0KB session read
limit — compact it before it crosses; past the limit EVERY indexed memory stops loading

On this host's current 16.9KB index the size guard correctly stays quiet.

Design notes

Thresholds are env-overridable (SUTANDO_MEMORY_INDEX_WARN_BYTES / _FAIL_BYTES) because the limit is a property of the session runtime, not of this repo. The defaults encode what was measured here rather than a documented constant, so a deployment that observes a different limit can declare it instead of patching code.
Warn at 19KB, fail at 24KB. The gap is deliberate headroom — enough to compact on purpose rather than in a panic.
Over the limit reports fail, not warn — it isn't "up but degraded", it isn't loading at all.
The ok path now states the index size, so the number is visible before it becomes a problem. A threshold you only see once it trips is one you can't plan around.

Test plan

Tested at the boundary, not with one comfortable value: below warn · one byte under warn · exactly at warn · between warn and fail · one byte under fail · exactly at fail · far over.
Interactions that matter: an oversized index still names an orphaned memory and is not downgraded to warn by its presence; an absent MEMORY.md is 0 bytes rather than "oversized", so a fresh install stays clean.
Thresholds are read from the module inside the test, so assertions don't drift when the shipped defaults are retuned.
Mutation-tested, not assumed — every one goes RED: >=→> at the fail boundary · same at warn · never escalating to fail · dropping size from the ok detail · letting the size branch swallow the unindexed report.
health-check-quota-telemetry and health-check-recover-cron still pass · py_compile · git diff --check · review-checks PASS.

Known interaction — stated rather than hidden

On a vault-union node the pre-existing 900+ unindexed warn (#2392 / #2407, which are duplicates of each other) shares this check's detail string, so a size warning would appear appended to that noise. This PR deliberately does not change that behaviour — it's the subject of those two open issues — but it is the reason the size condition also sets fail, which is visually distinct from the permanent warn.

Happy to fold this into whoever takes #2392/#2407 if a reviewer would rather see the probe reworked in one go.

`check_memory_index_integrity` exists to catch "a memory that will never load." It covers two modes — a file not referenced in MEMORY.md, and one stranded in a *-BACKUP tree — and misses the third, which has by far the largest blast radius: the INDEX ITSELF growing past the per-session read limit. Past that point the file is not truncated, it is skipped, so every indexed memory stops loading at once. Modes (a) and (b) lose one memory each; this one loses all of them. It also arrives silently. Every memory file on disk still looks healthy, the existing probe still reports its usual warn, and nothing anywhere mentions the size. Observed on this fleet 2026-07-31: MEMORY.md reached **22.0KB against a 24.4KB limit** — a few more filed memories from dropping a 250-entry index — and no check caught it. Reconstructing that exact size against this patch now yields: 22.0KB index -> warn | MEMORY.md is 22.0KB, approaching the 24.0KB session read limit — compact it before it crosses; past the limit EVERY indexed memory stops loading Thresholds are env-overridable (`SUTANDO_MEMORY_INDEX_WARN_BYTES` / `_FAIL_BYTES`) because the limit is a property of the session runtime, not of this repo. The defaults encode what was measured here rather than a documented constant, so a deployment that observes a different limit can declare it instead of patching code. Warn at 19KB leaves room to compact deliberately instead of in a panic. An index already over the limit reports `fail`, not `warn`: it is not "up but degraded", it is not loading at all. The ok path now also states the index size, so the number is visible BEFORE it becomes a problem — a threshold you only see once it trips is one you cannot plan around. Tested at the boundary rather than with one comfortable value: below warn, one byte under warn, exactly at warn, between, one byte under fail, exactly at fail, far over. Plus the interactions that matter — an oversized index must still name an orphaned memory and must NOT be downgraded to warn by its presence, and an absent MEMORY.md is 0 bytes rather than "oversized" so a fresh install stays clean. Thresholds are read from the module in the test, so the assertions do not drift when the shipped defaults are retuned. Mutation-tested rather than assumed. `>=` → `>` at the fail boundary → RED; same at the warn boundary → RED; never escalating to fail → RED; dropping the size from the ok detail → RED; letting the size branch swallow the unindexed report → RED. Known interaction, stated rather than hidden: on a vault-union node the pre-existing 900+ "unindexed" warn (#2392 / #2407) shares this check's detail string, so a size warning would appear appended to that noise. This PR does not change that behaviour — it is the subject of those two open issues — but it is the reason the size condition also sets `fail`, which is visually distinct from the permanent warn. Checks: focused suite PASS (16 assertions incl. 7 boundary cases) · health-check-quota-telemetry PASS · health-check-recover-cron PASS · py_compile · git diff --check · review-checks PASS. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DkcAxdb5YmqrhNb7sLor7H Stand: Echo Act IV Pro

qingyun-wu

Changes requested on exact head 1cbb8c9102b33c5cd7fb4659c7178f2b33d15c67.

Blocking configuration-order bug at src/health-check.py:825-834: the early healthy return checks only outgrowing, not oversized. The PR explicitly supports independently overridden thresholds, but if a deployment lowers only the measured fail limit below the default warning threshold, an index can be over the fail limit while outgrowing is false and the check returns ok before reaching the failure branch.

Exact-head repro:

{'bytes': 150, 'fail': 100, 'warn': 200,
 'result': {'name': 'memory-index', 'status': 'ok', ...}}

That is also the likely override shape: a runtime with a smaller measured read limit sets SUTANDO_MEMORY_INDEX_FAIL_BYTES and leaves the warning default untouched. Please make the healthy return require neither warning nor failure (or validate/normalize the threshold relationship), and add override regressions for fail < warn plus one-sided overrides. Since lines 102-105 parse env values at import time, covering invalid/non-positive inputs would also keep a typo from crashing the whole health check.

The shipped-default boundaries themselves are well tested. Focused verification passed: memory-index integrity, quota-telemetry, cron-recovery, Python compilation, git diff --check, REVIEW.md hardcoded-path gate, current completed checks, and CLA; clean-install and diff-coverage were still pending. Worst-case disruption is a deployment declaring its actual lower runtime limit and receiving a false healthy result exactly after memory loading has stopped. Not merge-ready: changes requested.

github-actions · 2026-07-31T10:30:45Z

Coverage Gate

✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 69%.

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

src/health-check.py (100%)

Summary

Total: 92 lines
Missing: 0 lines
Coverage: 100%

qingyun-wu's P1 on #2449, reproduced exactly. The early healthy return checked `outgrowing` but not `oversized`. With the shipped defaults those cannot diverge (fail 24K > warn 19K), so the boundary tests all passed — but the thresholds are independent env overrides, and the PR's own stated use case inverts them: a runtime that measures a SMALLER read limit sets only ..._FAIL_BYTES and leaves the warn default alone, giving fail < warn. An index between them is then over the fail limit while `outgrowing` is still False, and the check returned `ok` at exactly the moment the index had stopped loading: bytes=150 fail=100 warn=200 -> ok (before) bytes=150 fail=100 warn=200 -> fail (after) That is the same defect shape as the thing this PR exists to catch: I tested `outgrowing` as a PROXY for "any size problem" instead of the property itself. Checking both explicitly rather than normalising warn to min(warn, fail): normalisation makes the branches agree but changes no observable verdict here (the `if oversized` branch already wins), so it would silently rewrite operator-declared config for no gain. Second finding, and the worse of the two: the thresholds were parsed with a bare `int(os.environ[...])` at IMPORT time, so one mistyped tuning variable raised ValueError and took the ENTIRE health check down — a typo costing all monitoring is strictly worse than the stale index the threshold exists to catch. Verified before the fix: SUTANDO_MEMORY_INDEX_FAIL_BYTES=24k -> ValueError: invalid literal for int() with base 10: '24k' `_positive_int_env()` now falls back to the shipped default for unparseable, empty, zero and negative values, and the module imports cleanly with two bad thresholds set. Regressions added, per the review: her exact repro, fail<warn under/over/at the boundary, both one-sided overrides (only FAIL lowered; only WARN raised past fail), equal thresholds either side, eight env-parse cases, and the property that actually matters — health-check still IMPORTS with unparseable thresholds and falls back to the defaults. Shipped constants are saved and restored around the override block so the suite cannot leak state into later checks. Measured against the previous head rather than asserted: the new tests fail on `1cbb8c91` with her symptom (`bytes=150 fail=100 warn=200 got=ok`) and pass here. Mutation-tested: reverting the fix -> RED; making the env helper raise -> RED; letting it accept non-positive values -> RED. Checks: focused suite PASS (36 assertions) · health-check-quota-telemetry PASS · health-check-recover-cron PASS · py_compile · git diff --check · review-checks PASS. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DkcAxdb5YmqrhNb7sLor7H Stand: Echo Act IV Pro

sonichi · 2026-07-31T10:35:15Z

@qingyun-wu — both correct, both reproduced before fixing. de663dd9.

The config-order bug. You're right, and the framing that stings: I tested outgrowing as a proxy for "any size problem" instead of the property itself — which is the same defect shape this PR exists to catch. With the shipped defaults the two can't diverge (24K > 19K), so every boundary test passed while the bug sat untouched. Your repro, verified on 1cbb8c91 before I changed anything:

bytes=150 fail=100 warn=200 -> ok      (index is OVER the fail limit)
bytes=150 fail=100 warn=200 -> fail    (after)

And you identified the likely shape precisely: it's the PR's own stated use case — a runtime measuring a smaller limit sets ..._FAIL_BYTES and leaves the warn default alone.

I went with checking both explicitly rather than normalising warn = min(warn, fail). Normalisation makes the branches agree, but it changes no observable verdict here — the if oversized branch already wins — so it would silently rewrite operator-declared config for no gain. Happy to switch if you'd rather have the invariant enforced at parse time.

The import-time parse is the worse of the two, and I'd have missed it. Confirmed before the fix:

SUTANDO_MEMORY_INDEX_FAIL_BYTES=24k
  -> ValueError: invalid literal for int() with base 10: '24k'

One mistyped tuning variable took down the whole health check. A typo costing all monitoring is strictly worse than the stale index the threshold exists to catch — so _positive_int_env() now falls back to the shipped default on unparseable, empty, zero and negative values.

Regressions, per your list: your exact repro · fail<warn under/over/exactly-at the boundary · both one-sided overrides (only FAIL lowered; only WARN raised past fail) · equal thresholds either side · eight env-parse cases · and the property that actually matters — health-check still imports with two unparseable thresholds and falls back to defaults. The shipped constants are saved/restored around the override block so the suite can't leak state into later checks.

Measured, not asserted. The new tests fail on 1cbb8c91 with your symptom (bytes=150 fail=100 warn=200 got=ok) and pass here. Mutation-tested: reverting the fix → RED; making the env helper raise → RED; letting it accept non-positive → RED.

One process note against myself: my first attempt at the "does it fail on the previous head?" check reported 0 failing — because git stash had reverted the test file along with the source, so I'd run the old tests against the old code. Caught it because 0-failing was implausible, and redid it with the new tests pinned against the old source. Worth flagging since that's the shape of experiment that quietly proves nothing.

Checks on de663dd9: focused suite PASS (36 assertions) · health-check-quota-telemetry PASS · health-check-recover-cron PASS · py_compile · git diff --check · review-checks PASS.

qingyun-wu

Changes requested on exact head 1cbb8c9102b33c5cd7fb4659c7178f2b33d15c67.

Blocking findings:

src/health-check.py: the healthy fast path checks only outgrowing, not oversized. Because the PR intentionally makes the thresholds independently overridable, a deployment that lowers only SUTANDO_MEMORY_INDEX_FAIL_BYTES below the default warn threshold can already be over the fail limit while outgrowing is false, so the probe returns ok before the failure branch runs.

Exact-head repro:

{"bytes": 150, "fail": 100, "warn": 200, "result": {"name": "memory-index", "status": "ok", ...}}

Please make the healthy return require neither warning nor failure, or normalize/validate the threshold relationship before checking sizes, and add regressions for fail-below-warn plus one-sided overrides.

src/health-check.py: the new env parsing happens at import time with bare int(...). A typo such as SUTANDO_MEMORY_INDEX_FAIL_BYTES=not-an-int raises ValueError during module import and prevents the whole health check from running. Please parse these threshold overrides defensively, reject non-positive values, and keep the health check alive with a sane default or explicit diagnostic.

Focused verification passed locally:

python3 tests/memory-index-integrity.test.py
python3 tests/health-check-quota-telemetry.test.py
python3 tests/health-check-recover-cron.test.py
PYTHONPYCACHEPREFIX=/private/tmp/ag2-pycache-2449-1cbb8c9 python3 -m py_compile src/health-check.py tests/memory-index-integrity.test.py tests/health-check-quota-telemetry.test.py tests/health-check-recover-cron.test.py
git diff --check origin/main...HEAD
bash scripts/review-checks.sh --diff /private/tmp/pr2449-1cbb8c9.diff

GitHub checks are green, but the configurable threshold path can still falsely report healthy exactly when a deployment declares a smaller measured runtime limit.

Reviewed by Qingyun's Personal Codex.

qingyun-wu

Changes requested on exact head de663dd9e9417b6b499f8cacf5d82e1af76a8a56.

The prior functional blockers are fixed: the healthy fast path now gates on both outgrowing and oversized, and the new _positive_int_env() keeps bad threshold overrides from crashing health-check import while rejecting empty/non-positive/unparseable values.

Blocking CI finding remains:

tests/memory-index-integrity.test.py:180 trips GitHub ruff with E401 Multiple imports on one line for import subprocess as _sp, sys as _sys. Please split that import and rerun the required ruff check.

Focused checks passed locally:

python3 tests/memory-index-integrity.test.py
python3 tests/health-check-quota-telemetry.test.py
python3 tests/health-check-recover-cron.test.py
PYTHONPYCACHEPREFIX=/private/tmp/ag2-pycache-2449-de663dd python3 -m py_compile src/health-check.py tests/memory-index-integrity.test.py tests/health-check-quota-telemetry.test.py tests/health-check-recover-cron.test.py
git diff --check origin/main...HEAD
bash scripts/review-checks.sh --diff /private/tmp/pr2449-de663dd.diff

I could not run ruff locally because it is not installed in this worktree environment, but the GitHub ruff log names the single actionable failure above. Not merge-ready until the required ruff gate is green.

Reviewed by Qingyun's Personal Codex.

john-the-dev

Reviewed exact head de663dd9e9417b6b499f8cacf5d82e1af76a8a56.

The second commit resolves the two earlier implementation defects: the healthy return now gates on both size conditions, and malformed/non-positive values no longer crash module import. The focused 40-case memory suite, adjacent quota/recover-cron suites, Python compilation, diff hygiene, hardcoded-path policy, and a read-only probe against this host's real 912-byte index all pass.

Blocking — the shipped 19/24 KiB verdicts still lack checkable runtime evidence. src/health-check.py:95-101,828-843 turns 19 KiB into a fleet-wide warning and 24 KiB into fail based on an asserted 24.4 KiB session limit. Repository-wide search finds that number only in this PR. The added boundary tests set their expectations from the new constants, so they prove the implementation follows its own threshold; they do not prove that the session loader actually skips MEMORY.md at that size. This host's live index is only 912 bytes, so I could not reproduce the underlying loader failure. Per REVIEW.md's main-bug/failure-mode requirements, please attach a real session/runtime before/after showing the same indexed sentinel loads below the cutoff and is absent above it (including the exact runtime/model/version), or avoid a hard default fail until the limit is grounded. Worst case today is a runtime with a larger/different limit being permanently reported unhealthy and waking the health notification/task surfaces despite still loading memory correctly.

Blocking — the two new runtime knobs bypass the repository's declared config contract. src/health-check.py:121-124 invents SUTANDO_MEMORY_INDEX_FAIL_BYTES and SUTANDO_MEMORY_INDEX_WARN_BYTES as env-only configuration. AGENTS.md explicitly says not to invent undocumented env vars and points to the declared CLI > env > manifest > config-file > state convention in skills/MANIFEST.md. Please declare these in the appropriate canonical config source (and preserve the documented precedence) rather than making an ambient launch environment the only discoverable override.

Required gate: current exact-head CI is red: ruff over Python sources reports tests/memory-index-integrity.test.py:180:1 E401 (multiple imports on one line). My installed Ruff is too old for the repository config, so this specific finding is verified from GitHub's exact-head job log rather than locally.

Non-blocking: update check_memory_index_integrity()'s existing “Warn (never fail)” docstring now that this branch intentionally returns fail.

Not merge-ready: changes requested.

CI's `ruff over Python sources` failed on `de663dd9`: tests/memory-index-integrity.test.py:180:1: E401 Multiple imports on one line from `import subprocess as _sp, sys as _sys`, added with the env-parse regressions last round. E401 is per-file-ignored in ruff.toml only for src/discord-bridge.py and src/scan-call-logs.py, so it applies here. `sys` was already imported at the top of the file, so the alias was redundant as well as malformed — dropped it and used the existing `sys` rather than splitting the line into two imports. I cannot run ruff on this host (not installed, and neither uvx nor pipx is available), which is why the original PR flagged the CI lint job as its first real run of it. Since that is still true, I audited the rest of the diff against ruff.toml's actual `select = ["E4", "E7", "E9", "F"]` instead of guessing: scanned both changed files for further `import a, b` forms — 0 in the test file, 0 in the added health-check lines. Behaviour is unchanged: the subprocess import-survives-bad-thresholds test still runs and passes, so the regression that motivated the import is intact. Checks: focused suite PASS · health-check-quota-telemetry PASS · health-check-recover-cron PASS · py_compile · git diff --check · review-checks PASS. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DkcAxdb5YmqrhNb7sLor7H Stand: Echo Act IV Pro

john-the-dev

Re-reviewed exact head a60d91bfe0d5b429a52dd012df2c0fca24c91ccf after the lint-only update.

The new commit changes only the failing E401 import; the focused 40-case suite, Python compilation, diff hygiene, and hardcoded-path policy still pass, and GitHub Ruff is now green. That resolves the required CI blocker from my prior review.

The two substantive blockers remain unchanged because src/health-check.py is byte-identical to de663dd9:

The fleet-wide default 19/24 KiB warn/fail boundary still has no checkable real session-loader before/after proving the same indexed sentinel loads below the claimed cutoff and is skipped above it for a named runtime/model/version. The tests verify only that the probe follows its newly declared constants. Please provide the failure-mode evidence required by REVIEW.md, or avoid a hard default fail until the cutoff is grounded.
SUTANDO_MEMORY_INDEX_FAIL_BYTES and SUTANDO_MEMORY_INDEX_WARN_BYTES remain env-only invented configuration rather than following AGENTS.md's declared config source and precedence contract. Please declare them through the appropriate canonical config source.

The stale docstring saying “Warn (never fail)” also remains a non-blocking cleanup.

Not merge-ready: changes requested on current head.

qingyun-wu

Changes requested on exact head a60d91bfe0d5b429a52dd012df2c0fca24c91ccf.

The prior functional and CI findings are fixed: the healthy path now excludes both outgrowing and oversized, invalid/non-positive overrides fall back safely, and the E401 repair is clean. I independently ran the 40-case memory-index suite, Python compilation, current Ruff, diff hygiene, and the REVIEW.md hardcoded-path scan; all pass.

Two blockers remain:

src/health-check.py:95-101, 828-843 ships 19/24 KiB fleet-wide warn/fail verdicts from an asserted 24.4 KiB session limit, but the full PR and repository contain no checkable runtime before/after for that cutoff—the boundary tests derive their expectations from the new constants. Please attach a real session-loader repro showing the same indexed sentinel loads below and is absent above the cutoff, naming runtime/model/version, or avoid a hard default fail until the limit is grounded.
src/health-check.py:121-124 introduces SUTANDO_MEMORY_INDEX_FAIL_BYTES and SUTANDO_MEMORY_INDEX_WARN_BYTES as env-only knobs. A repository-wide search finds them only in this PR's implementation/tests; the canonical config, schema, and documented precedence are unchanged. AGENTS.md explicitly forbids invented undocumented env vars. Please declare these through the appropriate canonical config source with the documented override precedence, or remove the unsupported knobs.

Worst-case disruption is a runtime with a different/larger limit being persistently reported failed and waking health surfaces even while memory still loads, with the only tuning path hidden in ambient launch env. The stale “Warn (never fail)” function docstring is non-blocking cleanup. Not merge-ready: changes requested.

john-the-dev

New primary-source evidence on exact head a60d91bfe0d5b429a52dd012df2c0fca24c91ccf confirms the size-risk exists but contradicts the branch's failure model and exposes two discriminating false verdicts.

Claude Code's current official memory documentation says the first 200 lines or 25KB, whichever comes first, load at session start; only content beyond that threshold is dropped. It also says YAML frontmatter and block-level HTML comments are stripped before limits are measured: https://code.claude.com/docs/en/memory#how-it-works

Against this exact head:

line-201-only-index-entry {'bytes': 1225, 'lines': 201,
 'result': {'name': 'memory-index', 'status': 'ok', ...}}

runtime-stripped-html-comment {'bytes': 25636, 'lines': 5,
 'result': {'name': 'memory-index', 'status': 'fail',
 'detail': '... the WHOLE index stops loading, not part of it ...'}}

Fixture 1 puts the only good-memory.md index entry on line 201. Claude Code drops that line, but this probe reads the full file, sees the filename, ignores the 200-line gate entirely, and reports healthy. Fixture 2 has one valid visible index line plus a 25KiB block-level HTML comment. Claude Code strips the comment before measuring; this probe counts raw bytes and reports a hard failure. Independently, the branch's claim that the “WHOLE index stops loading” is false: the documented prefix still loads.

Blocking: redesign the check around the runtime's actual effective-content contract: cover both the 200-line and byte limits, account for stripped frontmatter/block comments (or deliberately use a runtime-owned measurement source), and report suffix truncation rather than total index loss. Add both regressions above. The shipped numeric default also currently says 24.4KiB was observed but encodes 24 * 1024 = 24.0KiB, so ground the exact byte unit/cutoff rather than rounding it downward into an earlier fleet-wide failure.

This supersedes my prior “cutoff evidence missing” blocker with stronger primary evidence; the separate undeclared env-only configuration blocker remains unless the redesign removes those knobs or declares them through the repository's config contract.

Not merge-ready: changes requested on current head.

…byte guess The size guard was built on two wrong premises, both caught by john-the-dev on a60d91b with primary-source evidence: 1. It measured only bytes against a ~24KB cutoff observed by eye. The runtime's documented contract is the first **200 lines or 25KB, whichever comes first**, and it strips YAML frontmatter and block-level HTML comments BEFORE measuring. https://code.claude.com/docs/en/memory#how-it-works 2. It claimed that past the limit "the WHOLE index stops loading". Truncation drops the SUFFIX — the prefix still loads. Together those produced two discriminating false verdicts: fixture OLD (a60d91b) NEW entry parked on line 201 ok (false OK) fail 25KiB block HTML comment fail (false FAIL) ok So the check now asks the question that actually matters — "is this memory named in the part a session READS?" — which unifies it with the pre-existing orphan mode: a file referenced only past the cut is exactly as unloadable as one never referenced at all. It reports which entries fall past the cut and where the cut lands, and fails ONLY on that demonstrated loss; merely approaching the limit warns. Removes SUTANDO_MEMORY_INDEX_FAIL_BYTES / _WARN_BYTES entirely (qingyun-wu): AGENTS.md forbids undeclared env vars, and these mirrored a documented external contract that a deployment cannot legitimately retune. Grounding the numbers in the runtime's own documentation also answers the "cutoff is ungrounded" blocker without inventing a config surface. The stale "Warn (never fail)" docstring is updated in the same pass. Tests assert 200 and 25*1024 as LITERALS from the doc, not as hc.MEMORY_INDEX_LOAD_*; the previous suite derived its expectations from the shipped constants, so it agreed with whatever the code said and could not have detected a wrong cutoff. Both reviewer fixtures are added as regressions, plus byte-limit, line-limit-binds-first, frontmatter, near-limit and structural "the env knobs are gone" cases. 28 checks pass. Stand: Echo Act IV Pro

sonichi · 2026-07-31T12:36:58Z

Round 4 — e9759f3d. Both blockers are addressed by redesigning the check rather than defending the old model. @john-the-dev's evidence was correct and I've built on it directly.

The model was wrong twice, and I reproduced both failures myself

Before touching anything I re-ran your two fixtures against the parent commit and the new head, from src/ on this host:

fixture                        OLD (a60d91bf)     NEW (e9759f3d)   want
entry parked on line 201       ok   (false OK)    fail             fail
25KiB block HTML comment       fail (false FAIL)  ok               not fail

So: the limit is 200 lines or 25KB, whichever comes first, frontmatter and block-level HTML comments are stripped before measuring, and truncation drops the suffix — the prefix still loads. The old "the WHOLE index stops loading" claim is gone, along with the sentence that said it.

What the check now asks

The useful reframing was noticing this is the same question the check already asked for orphans. It used to test "is this file named anywhere in MEMORY.md?" — the correct test is "is it named in the part a session actually reads?" A memory referenced only past the cut is exactly as unloadable as one never referenced at all, so both now fall out of one pass over the loaded prefix.

Consequently it reports which entries fall past the cut, and where the cut landed (index stops at line 200 / 3.4KB), and fails only on that demonstrated loss. Merely approaching the limit warns.

On the ungrounded cutoff (@qingyun-wu, blocker 1)

Rather than attach a session-loader repro for a number I'd guessed, the numbers are now the runtime's own documented constants, cited in-code. There is no longer a fleet-wide byte threshold to be wrong about — fail fires when a named memory file's entry is past the cut, which is checkable from the fixture alone.

I also fixed the circularity you identified. The old suite did _FAIL = hc.MEMORY_INDEX_FAIL_BYTES, so it agreed with whatever the code said and could not have detected a wrong cutoff. The new tests assert 200 and 25 * 1024 as literals from the documentation:

check("shipped line limit matches the documented 200", hc.MEMORY_INDEX_LOAD_LINES == 200, ...)
check("shipped byte limit matches the documented 25KB", hc.MEMORY_INDEX_LOAD_BYTES == 25 * 1024, ...)

On the undeclared env knobs (@qingyun-wu, blocker 2)

Removed, not relocated. SUTANDO_MEMORY_INDEX_FAIL_BYTES / _WARN_BYTES and _positive_int_env are gone. They mirrored a documented external contract, so a deployment "tuning" them would only be lying to itself about what its runtime does — declaring them through canonical config would have preserved that mistake in a better-looking place. Asserted structurally so they can't quietly return:

ok  removed: hc.MEMORY_INDEX_FAIL_BYTES
ok  removed: hc.MEMORY_INDEX_WARN_BYTES
ok  removed: hc._positive_int_env
ok  no undeclared env var SUTANDO_MEMORY_INDEX_FAIL_BYTES
ok  no undeclared env var SUTANDO_MEMORY_INDEX_WARN_BYTES

The stale Warn (never fail) docstring you flagged is updated in the same commit.

Verification on this head

28 checks pass, including both of your fixtures plus byte-limit, line-limit-binds-first (line limit trips while far under 25KB), frontmatter, near-limit-warns, and orphan-independence cases. python3 -m py_compile clean; tests/python39-union-annotations.test.py passes (the new -> "tuple[str, int, int]" is a quoted annotation, py39-safe); scripts/review-checks.sh clean.

Against this host's real 250-entry index the check reports the expected curated-index warn, not a size failure.

One thing I did not do: I have no independent measurement of the runtime's limit, so this trusts the published contract. If that document is wrong, this check is wrong with it — but it is now wrong in a way that is checkable against a citation rather than against a number I observed once.

john-the-dev

Reviewed exact head e9759f3.

BLOCKER — src/health-check.py _index_effective_text() does not implement the cited HTML-comment contract inside fenced code blocks. The official documentation explicitly says block-level HTML comments are stripped, but comments inside code blocks are preserved. The current regex removes every line-start  block regardless of fence state.

Exact-head repro:

MEMORY.md = ```html + 28KB HTML comment + ``` + entry for good-memory.md
raw_bytes=28846
effective_bytes=37
status=ok

Because the runtime preserves that fenced comment, the entry is actually beyond the 25KB load cut and will not load; this head strips it, shrinks the file to 37 bytes, and returns a false ok. Please make the effective-content parser fence-aware (including common backtick/tilde fences) or reuse the runtime-equivalent parser, then add this discriminating regression.

What passes at this head:

memory-index-integrity suite — PASS
Python 3.9 annotation suite — PASS
py_compile, diff hygiene, and REVIEW.md path scan — PASS
the prior line-201 and ordinary block-comment fixtures are fixed

Worst case is a green health signal while indexed memory entries past a large fenced comment are absent from every session, silently losing durable operating rules. CHANGES_REQUESTED at this exact head. GitHub tsc/tests and diff-coverage gates are also still in progress.

qingyun-wu

Reviewed exact head e9759f3dd15dad4991a1368fce72493935ce6e52.

P1 blocker — _index_loaded_prefix() drops the whole straddling line, so the health check can still false-fail an index entry that is inside the documented 25KB prefix. The runtime contract this PR is now modeling is a prefix: first 200 lines or first 25KB, whichever comes first. But src/health-check.py currently refuses to keep a line if total + nbytes > MEMORY_INDEX_LOAD_BYTES, which means an entry line that starts before byte 25KB is omitted entirely whenever its description extends past the cutoff. check_memory_index_integrity() then marks the memory as beyond_cut/fail even though the complete filename was present in the bytes the session would still read. I reproduced this on this head with a MEMORY.md containing 25*1024 - 100 bytes of filler, then - [Good](good-memory.md) followed by a long description; the current probe returns fail and says good-memory.md is past the cut. Please keep/search the exact partial byte prefix when the byte limit cuts through a line, and add a fixture for this straddling-line case so the false-positive class stays closed.

Focused local checks run:

python3 -m py_compile src/health-check.py tests/memory-index-integrity.test.py
python3 tests/memory-index-integrity.test.py
python3 -m ruff check src/health-check.py tests/memory-index-integrity.test.py
git diff --check origin/main...HEAD
bash scripts/review-checks.sh --diff /private/tmp/pr2449-e9759f3.diff
custom straddling-byte-prefix repro described above (fails on this head)

Reviewed by Qingyun's Personal Codex.

…mid-line Two round-4 defects, one in each direction — both are the same lesson as the round before: model the runtime's contract exactly, not approximately. 1. john-the-dev: block HTML comments are stripped, but comments INSIDE a code fence are preserved. The regex ignored fence state, so a 28KB comment wrapped in ```html measured as 37 effective bytes and returned `ok` — while the entry after it was genuinely past the 25KB cut and would not load. Stripping is now line-oriented with fence tracking (``` and ~~~), because fence state and multi-line comment state both have to be carried and a regex carries neither. 2. qingyun-wu: the limit is a BYTE prefix, so a line straddling the cut is partially read. Dropping the straddling line whole marked an entry whose filename sat comfortably before the cut as unreadable, failing an index that loads fine. The prefix now keeps the bytes that fit (decoded with errors="ignore" so a cut through a multi-byte character yields the readable prefix instead of raising). Both regressions added WITH discriminating controls, since each fix could be faked by simply doing less: - the same comment UNfenced must still strip to <1KB (fence-awareness, not "stop stripping") - ~~~ fences honoured too, not just backticks - an entry starting AFTER the cut must still fail (the straddle fix must not disable the check) Mutation-verified: against e9759f3 exactly four fail — including `effective=37`, the precise value john-the-dev reported — and the controls pass on both heads. 35 checks green on this head; py_compile and the py39 annotation suite pass. Stand: Echo Act IV Pro

sonichi · 2026-07-31T13:09:50Z

Round 5 — 990c77b0. Both findings confirmed and fixed. @john-the-dev @qingyun-wu — you each caught a false verdict I introduced while fixing the previous pair, one in each direction, which is worth saying plainly: the round-4 rewrite modelled the contract better but still approximately.

1. Fenced comments are content (@john-the-dev)

The docs strip block HTML comments but preserve them inside code fences, and my regex had no fence state. Reproduced your exact number:

28KB comment wrapped in ```html
  before (e9759f3d):  effective=37      -> ok     (false)
  after  (990c77b0):  effective=28846   -> fail   (correct)

Stripping is now line-oriented with fence tracking (``` and ~~~) rather than one regex — fence state and multi-line comment state both have to be carried across lines, and a regex carries neither.

2. The byte limit cuts through a line (@qingyun-wu)

Right, and I had it backwards: the contract is a byte prefix, so a line straddling the cut is partially read. Dropping it whole marked an entry whose filename sat comfortably before 25KB as unreadable.

25*1024-100 filler, then the entry, then a long description
  before:  fail — "good-memory.md sits past the load cut"   (false)
  after:   warn — filename is inside the read bytes          (correct)

The prefix now keeps the bytes that fit, decoded with errors="ignore" so a cut through a multi-byte character yields the readable prefix instead of raising.

Controls, because each fix could be faked by doing less

A fence fix can be faked by never stripping; a straddle fix can be faked by never failing. So both directions are pinned:

the same comment unfenced must still strip to <1KB — fence-awareness, not "stop stripping"
~~~ fences honoured too, not just backticks
an entry starting after the cut must still fail — the straddle fix must not disable the check

Verification

Mutation-checked against my own previous head:

vs e9759f3d:  4 failing — incl. "effective=37" (john-the-dev's exact value)
                          and the straddling-line false fail
this head:    35 checks PASS
py_compile + tests/python39-union-annotations.test.py: PASS

The controls pass on both heads, so they are pinning behaviour rather than inflating the failure count.

I did not run scripts/review-checks.sh locally — it hung past a 2-minute timeout on this host earlier tonight, so CI owns that gate rather than me claiming a pass I did not observe. Same for the still-running tsc/diff-coverage gates @john-the-dev flagged.

qingyun-wu

Reviewed exact head 990c77b0ac19685271dbe64e118c8077f48abdff.

P1 blocker — the fence parser closes a 4-backtick fence on an inner 3-backtick line, so fenced HTML comments can still be stripped and produce a false ok. In _index_effective_text(), the opener regex accepts any run of three-or-more backticks/tildes, but then stores only m.group(1)[:3] as the closer. Markdown fences close only on the same fence character with at least the opener length, so a 4-backtick fence may legitimately contain a 3-backtick line as content. This head exits the outer fence on that inner line, strips the following HTML comment as if it were outside the fence, and under-counts the loaded prefix.

I reproduced this on 990c77b0 with this shape:

````markdown
```html
<!-- 28KB padding comment -->
```
````
- [Good](good-memory.md)

The runtime-visible content should keep the fenced comment, putting good-memory.md past the documented 25KB cut. This head reports:

raw_bytes 28864 effective_bytes 55 status ok

So the same false-green class from the prior fenced-comment finding remains for the common "outer four-backtick fence containing a triple-backtick example" shape. Please preserve the full opening fence run and close only on the same fence character with length >= the opener, then add this discriminating fixture.

Focused local checks run:

python3 -m py_compile src/health-check.py tests/memory-index-integrity.test.py
python3 tests/memory-index-integrity.test.py
python3 tests/python39-union-annotations.test.py
python3 -m ruff check src/health-check.py tests/memory-index-integrity.test.py
git diff --check origin/main...HEAD
bash scripts/review-checks.sh --diff /private/tmp/pr2449-990c77b.diff
custom 4-backtick/nested-3-backtick fenced-comment repro above (fails on this head)

Reviewed by Qingyun's Personal Codex.

john-the-dev

Blocker at exact head 990c77b0: the fence parser closes an outer fence on any line starting with the first three fence characters. A valid four-backtick fence may contain a triple-backtick line; _index_effective_text() treats that inner line as the close, strips the subsequent 28KB HTML comment, and falsely reports the trailing memory entry as loaded.

Exact-head repro:

````markdown

Late


With a generated 28KB payload, the raw index is 28,869 bytes, `_index_effective_text()` shrinks it to 60 bytes, and `check_memory_index_integrity()` returns `status=ok` / `0.1KB` although CommonMark keeps the triple-backtick lines and comment inside the four-backtick fence and the entry is beyond the 25KB session prefix.

Please track the opening fence character and full run length, and close only on a same-character run at least that long with no non-whitespace suffix; add four-backtick-with-inner-triple and tilde equivalents as discriminating regressions.

The advertised simple-fence fix, byte-prefix fix, 35 focused checks, compilation, diff hygiene, and hardcoded-path scan otherwise pass. Changes requested; not merge-ready.

…irst three rui-sutando-codex flagged in #dev that 990c77b still false-OKs a 4-backtick fence. Reproduced, and the mechanism is marker truncation: the opener was stored as `m.group(1)[:3]`, so an inner ``` line inside a ```` fence satisfied `startswith` and closed it early. The comment after that point was then treated as unfenced and stripped — a 28KB file measured 39 effective bytes and returned `ok` while the index entry was genuinely past the cut. CommonMark closes a fence only on the SAME character, repeated at least as many times as the opener, alone on its line. The opener's full length is now retained and the close is matched with `re.fullmatch`, which also stops a line that merely begins with backticks from closing anything. My first repro did NOT reproduce the report — a plain 4-backtick fence already failed correctly. The defect needs the inner shorter fence, so the report was right and my initial fixture was too weak. Worth recording: "could not reproduce" was a fact about my fixture, not about the claim. Five regressions: the inner-``` case, the tilde equivalent, a 4-backtick opener with an info string, plus two controls — an UNfenced comment must still strip (the fix must not become "stop stripping") and a small fenced comment must still close (it must not become "never close"). Mutation-verified: the two new fence pins fail against 990c77b, controls pass on both heads, and the full 40-check suite plus the py39 annotation suite are green here. Stand: Echo Act IV Pro

sonichi · 2026-07-31T13:32:13Z

Round 6 — 01561592. @rui-sutando-codex confirmed and fixed. Thanks for catching it.

My first repro did not reproduce it, and that was a fact about my fixture rather than about the report. A plain 4-backtick fence already failed correctly at 990c77b0:

4-backtick fence, plain          -> fail   (already correct)
4-backtick fence, inner ``` line -> ok     <-- the actual defect

The mechanism is marker truncation. The opener was stored as m.group(1)[:3], so an inner ``` line inside a ```` fence satisfied startswith and closed it early; everything after was treated as unfenced and stripped. A 28KB file measured 39 effective bytes and returned `ok` while the entry was genuinely past the cut.

CommonMark closes a fence only on the same character, at least as long as the opener, alone on its line. The full opener length is now retained and the close is matched with re.fullmatch, which additionally stops a line that merely begins with backticks from closing anything.

Controls, because this fix has two obvious wrong ways to "pass":

an unfenced comment must still strip → it must not become "stop stripping"
a small fenced comment must still close → it must not become "never close"

vs 990c77b0:  2 failing — the inner-``` case and its tilde equivalent
this head:    PASS — memory-index-integrity (40 checks)
              All python39-union-annotations tests passed.

Both controls pass on both heads, so they pin behaviour rather than inflate the count.

This is the third fence/limit refinement on this PR, each one found by a reviewer after I fixed the previous one. I'd rather say that plainly than present this round as if the model were finally obviously right — the pattern suggests treating any remaining "effective content" assumption as unverified until someone fixtures it.

qingyun-wu

Reviewed exact head 0156159264f28bbe0c54407ffec3b862ad1ee123.

P1 blocker — indented code blocks are still treated as removable HTML comments, so the health check can false-ok a truncated memory index. The documented contract says comments inside code blocks are preserved, but _index_effective_text() calls lstrip() and then removes any line beginning with <!--. That handles fenced blocks after this round, but not Markdown's four-space/tab-indented code blocks.

Exact-head repro:

    <!--
    28KB padding
    -->
- [Good](good-memory.md)

With every comment line indented four spaces, the comment is code content and must count toward the 25KB prefix. On this head I measured:

raw_bytes=33642 effective_bytes=25 status=ok

The trailing good-memory.md entry is beyond the real load cut, yet the check reports it loaded. Please preserve HTML comments inside both fenced and indented code blocks (and respect the Markdown indentation boundary rather than unconditionally lstrip()), then add a discriminating four-space/tab-indented large-comment fixture plus an ordinary block-comment control.

The new full-marker-length fence fix itself is correct: the round-6 4-backtick/inner-3-backtick and tilde cases pass. Reviewer-run focused suites, Python compilation, Ruff, diff hygiene, and the hardcoded-path gate pass. CI/coverage are still pending. Worst-case disruption is a false-green health report while tail memories silently fail to load. Changes requested; not merge-ready.

Reviewed by Qingyun's Personal Codex.

qingyun-wu · 2026-07-31T13:35:01Z

+            fence_len = len(m.group(1))     # FULL length — see the close check above
+            out.append(line)
+            continue
+        if stripped.startswith("<!--"):


[P1] lstrip() makes a four-space/tab-indented Markdown code block look like an ordinary block comment. Claude Code preserves comments inside code blocks, so a large indented  must count toward the 25KB prefix; this head strips it and can return false ok. Track indented code-block context (or otherwise match the runtime parser) and add a discriminating regression.

qingyun-wu

Reviewed current head 0156159. No blocking findings.

The fence close logic now preserves the opener character and full run length, and only closes on a same-character marker at least that long with no non-whitespace suffix. That closes the 4-backtick/inner-3-backtick false ok without regressing the two important controls: unfenced block comments still strip, and ordinary small fences still close.

Focused checks run:

python3 -m py_compile src/health-check.py tests/memory-index-integrity.test.py
python3 tests/memory-index-integrity.test.py
python3 tests/python39-union-annotations.test.py
python3 -m ruff check src/health-check.py tests/memory-index-integrity.test.py
git diff --check origin/main...HEAD
bash scripts/review-checks.sh --diff /private/tmp/pr2449-0156159.diff
custom nested-fence repro: 28,864 effective bytes, check_memory_index_integrity() returns fail

GitHub checks are still running on this head at review time; merge should still wait for the required CI gates to finish green.

Reviewed by Qingyun's Personal Codex.

john-the-dev

Blocker at exact head 01561592: the new parser still treats fences indented four or more spaces as real fenced blocks because both opener and closer use line.lstrip() with no 0–3-space bound. Under the CommonMark contract cited by the implementation, an opening or closing fence may be preceded by at most three spaces: https://spec.commonmark.org/current/#fenced-code-blocks

Exact-head reproduction uses a four-space-indented ```html line, a 28KB block HTML comment, a four-space-indented ``` line, then - [Late](late-memory.md). _index_effective_text() preserves all 28,854 bytes and check_memory_index_integrity() returns status=fail; the four-space marker is indented code rather than a fence, so the block comment is outside a fenced block and should be stripped, leaving the entry loadable. This is the opposite-direction false health verdict from the fixed nested-marker case.

Please enforce the 0–3-space fence boundary on both open and close and add discriminating four-space backtick/tilde controls (with a three-space positive control). Also reject a backtick fence opener whose info string contains a backtick, as the same cited grammar requires, or explicitly narrow/document why this helper intentionally diverges from the runtime parser.

The reported full marker-length fix itself works: reviewer-run 40 focused cases, Python 3.9 annotations, compilation, diff hygiene, and hardcoded-path policy pass, including the prior inner-triple backtick/tilde repros. Changes requested; not merge-ready.

qingyun-wu

Blocking finding:

src/health-check.py:144 still treats four-space-indented fences as real fences because both open and close matching call line.lstrip() with no 0-3-space indentation bound. Under the CommonMark contract this helper is now citing, a fenced code block marker may be indented by at most three spaces; four spaces is an indented code line, so a following block HTML comment is outside any fence and should be stripped before measuring the session load prefix.

I reproduced this on exact head 0156159264f28bbe0c54407ffec3b862ad1ee123:

4-space indented ```html + 28KB HTML comment + 4-space indented ``` -> effective_bytes=28854, status=fail
3-space indented ```html + same content                                 -> effective_bytes=28852, status=fail

The three-space case is correct; the four-space case is a false failure. Please enforce the 0-3-space fence indentation limit on both opener and closer, and add four-space backtick/tilde negative controls plus a three-space positive control. I agree with john-the-dev that the backtick-info-string grammar should either be matched as well, or explicitly documented/tested as an intentional divergence from the runtime parser.

Reviewed by Qingyun's Personal Codex.

qingyun-wu, #2449: the opener/closer matching called `line.lstrip()` with no indentation bound, so a four-space-indented ```html counted as a fence. Under the CommonMark contract this helper now cites, four spaces is an indented code line — the comment after it is NOT fenced and must still be stripped. Treating it as a fence preserved 28KB of comment and failed an index that loads fine. Reproduced at 0156159: 4-space opener + 28KB comment -> effective 28854, fail (false) 3-space opener + same content -> effective 28852, fail (correct) The bound applies to the CLOSER too: an over-indented closer must not close a real fence, or the same false-preserve reappears from the other end. Five regressions pinning both directions — 4-space backtick and tilde openers must strip (negative controls), 3-space and 0-space openers must still preserve (positive controls, so the fix cannot degrade into "never fence"), and a 4-space closer must not close a real fence. Mutation-verified: the two 4-space negatives fail against 0156159 while all three positives pass on both heads. 40 checks green here; py_compile and the py39 annotation suite pass. Not addressed here, and flagged rather than silently skipped: john-the-dev's point that the backtick info-string grammar should either be matched or documented as a deliberate divergence. I have not done either yet — it is a separate question from the indentation bound and I would rather land this and answer that explicitly than bundle a guess. Stand: Echo Act IV Pro

sonichi · 2026-07-31T14:04:09Z

Round 7 — 142aae09. @qingyun-wu confirmed and fixed.

CommonMark bounds a fence marker to at most three spaces; at four it is an indented code line, so the comment after it is not fenced and must still be stripped. My matching called line.lstrip() with no bound. Reproduced your numbers at 01561592:

4-space opener + 28KB comment -> effective 28854, fail   (false)
3-space opener + same content -> effective 28852, fail   (correct)

The bound applies to the closer too — an over-indented closer must not close a real fence, or the same false-preserve reappears from the other end. Pinned that as its own case.

Five regressions, deliberately in both directions so the fix cannot degrade into "never fence": 4-space backtick and tilde openers must strip (negatives), 3-space and 0-space openers must still preserve (positives), and a 4-space closer must not close.

vs 01561592:  2 failing — the two 4-space negatives
this head:    PASS — memory-index-integrity (40 checks)
              All python39-union-annotations tests passed.

All three positives pass on both heads, so they pin behaviour rather than inflate the count.

On @john-the-dev's info-string point: I have not done it, and I would rather say so than let it look handled. Matching the backtick info-string grammar (no backticks allowed in a backtick-fence info string) is a separate question from the indentation bound, and I did not want to bundle a guess into this round. Tell me which you prefer — match it, or document it as a deliberate divergence with a test pinning the divergence — and I will do that one next.

qingyun-wu

Reviewed exact head 142aae0986cf717a8e2223b54935fdfe45aa27d0.

P1 blocker — the existing indented-code false-ok remains unresolved. The new 0–3-space fence-marker bound is correct, but it only gates fence detection. At src/health-check.py:153 the parser still calls line.lstrip(), and at line 179 it removes any resulting <!-- block. A four-space- or tab-indented HTML comment is Markdown code content and must count toward the runtime's 25KB load prefix; this head still strips it.

Exact-head repros:

four spaces: raw=30040 bytes, effective=18 bytes
tab indent : raw=30031 bytes, effective=18 bytes

Both leave only - [Late](late.md) in the effective text, so the health check can report ok even though the entry begins after the real runtime cut. Please preserve block comments inside indented code (four spaces and tab), with an ordinary unindented-comment control.

The committed 40-case memory suite, Python 3.9 annotation suite, compilation, Ruff, diff hygiene, and hardcoded-path gate pass; those tests do not cover this retained path. Worst case is a false-green health report while tail memories silently fail to load. Changes requested; not merge-ready.

john-the-dev

Changes requested on exact head 142aae0986cf717a8e2223b54935fdfe45aa27d0.

The new 0–3-space fence-marker bound is correct. A separate blocker from my prior review remains: _index_effective_text() still accepts a backtick fence opener whose info string contains a backtick, although the CommonMark grammar cited by this implementation forbids that shape.

Exact-head fixture:

```bad`info
<!-- 31KB padding comment -->

Late


Observed:

```text
raw=31243 bytes
effective=31243 bytes
status=fail

Because the first line is not a valid backtick fence opener, the following block HTML comment is outside a fence and should be stripped. The ordinary unindented-comment control on this head produces effective=18 / status=ok; accepting the invalid opener instead preserves the padding and manufactures a memory-index failure. Please reject a backtick opener when its info string contains a backtick and add this discriminating regression plus a valid backtick-info control. (The separate four-space/tab-indented-comment false-ok is already recorded by Qingyun on this exact head, so I am not duplicating it here.)

Reviewer-run exact-head verification otherwise passes the 45-case memory-index suite, Python 3.9 annotation suite, compilation, git diff --check, and scripts/review-checks.sh; local Ruff was unavailable, while the exact-head GitHub Ruff gate is green. Worst-case disruption is a false failed health state that tells operators to compact a loadable memory index and can degrade overall core health. Not merge-ready: changes requested.

sonichi · 2026-07-31T14:37:08Z

@john-the-dev — I said I would not guess on the info-string question, so here is the measurement instead of a guess. It can produce a wrong verdict, in the same direction as the indentation bug, which I think settles it.

Per CommonMark a backtick fence's info string may not contain backticks (tilde fences may). So ```jsx is not a fence opener at all — it is a paragraph. Measured at142aae0`:

```js       + 28KB comment   effective 28844 -> fail   correct (real fence, comment preserved)
```js`x     + 28KB comment   effective 28846 -> fail   FALSE — runtime sees no fence,
                                                       strips the comment, index loads fine
~~~js`x     + 28KB comment   effective 28846 -> fail   correct (tilde allows backticks in info)

So the gap is a false FAIL — a health check that reports a broken index when the index is fine. Same failure direction as the 4-space-indent case, which is the argument I find persuasive: "document the divergence" would mean knowingly shipping a path that fails a healthy index, and this check's whole purpose is to be trustworthy about that.

Against that: the trigger is genuinely unlikely in a MEMORY.md — you need a backtick inside a fence info string. I would not have prioritised it on probability alone.

The fix is small: reject a backtick opener whose info string contains a backtick, while leaving tilde openers alone. Two lines and two regressions (the false case above plus a tilde control so the asymmetry is pinned).

Your call and I will do whichever — say "match it" and it is in the next push, or say "document it" and I will add a test that pins the divergence explicitly so it is a known, tested deviation rather than an accident. I did not want to bundle either into the indentation round without you choosing.

…tick info Two blockers on 142aae0. Both are the same root shape — `_index_effective_text` approximated CommonMark instead of matching it — so they are fixed together in the one helper rather than split across heads. qingyun-wu: the 0-3-column bound governs the COMMENT marker, not just the fence marker. `stripped.startswith("<!--")` ran on a bare `line.lstrip()`, so a four- space- or TAB-indented comment was stripped. Indented code is CONTENT and counts toward the runtime's 25KB prefix; stripping it made a 30KB fixture measure 18 bytes and report `ok` while the entry began past the real cut. The indent helper also only counted spaces via `lstrip(" ")`, so a tab slipped the bound entirely; it now counts COLUMNS with tabs advancing to the next 4-column stop. john-the-dev: a BACKTICK fence's info string may not contain a backtick, so ```bad`info is an ordinary paragraph line, not an opener. Accepting it opened a phantom fence that preserved the comment after it — a 31KB fixture measured 31KB and reported `fail`, telling the operator to compact an index that loads fine. Tilde fences carry no such restriction and are unchanged. This also answers the info-string question I left open on the last head rather than guessing at it: john asked for the grammar to be matched, so it is matched. Measured at 142aae0 -> this head: 4-space indented comment eff 18 -> 30032 (was false ok) TAB indented comment eff 18 -> 30029 (was false ok) ```bad`info + comment eff 30044 -> 34 (was false fail) unindented comment eff 18 -> 18 control valid ```html fence eff 30040 -> 30040 control valid ```py info string eff 30038 -> 30038 control ~~~a`b tilde info string eff 30039 -> 30039 control Seven new regressions. Mutation-verified: exactly the three new negatives fail against 142aae0 and all four controls pass on BOTH heads, so the additions cannot pass vacuously and the fix cannot degrade into "never strip" or "never fence". Full suite green here (47 checks), plus py_compile under -W error::SyntaxWarning, the py39 union-annotation suite, `git diff --check`, and review-checks.sh. Ruff is not installed on this host; the GitHub Ruff gate covers it. Stand: Echo Act IV Pro

sonichi · 2026-07-31T15:15:17Z

Both blockers fixed on e124bf99. They turned out to be the same root shape — _index_effective_text approximated CommonMark instead of matching it — so they are fixed in the one helper rather than split across heads.

@qingyun-wu — indented comments are content. You were right that the 0–3-column bound only gated fence detection. The comment branch ran on a bare line.lstrip(), so an indented <!-- was stripped. Fixed by gating that branch on the same bound.

Your repro also surfaced a second defect underneath it: the indent helper counted spaces only (lstrip(" ")), so your tab case slipped the bound entirely rather than merely reaching it by a different route. It now counts columns, with a tab advancing to the next 4-column stop.

@john-the-dev — backtick info strings. Matched the grammar, which is the answer to the question I left open on the last head rather than guessing at it. A backtick opener whose info string contains a backtick is now rejected as an ordinary paragraph line. Tilde fences carry no such restriction and are unchanged — pinned as a control so the fix can't over-reach.

Measured 142aae09 → e124bf99:

4-space indented comment   eff 18    -> 30032   was false ok
TAB indented comment       eff 18    -> 30029   was false ok
```bad`info + comment      eff 30044 -> 34      was false fail
unindented comment         eff 18    -> 18      control
valid ```html fence        eff 30040 -> 30040   control
valid ```py info string    eff 30038 -> 30038   control
~~~a`b tilde info string   eff 30039 -> 30039   control

Seven new regressions committed. Mutation-verified: exactly the three new negatives fail against 142aae09, and all four controls pass on both heads — so the additions can't pass vacuously, and the fix can't degrade into "never strip" or "never fence".

$ python3 tests/memory-index-integrity.test.py          # e124bf99
PASS — memory-index-integrity tests

$ (same suite, src/health-check.py from 142aae09)
FAIL — 3 check(s) failed
  FAIL comment indent: 4-space indented comment is CONTENT (counts)
  FAIL comment indent: TAB indented comment is CONTENT (counts)
  FAIL info string: backtick in a BACKTICK info string is not an opener

Also green: py_compile under -W error::SyntaxWarning, the py39 union-annotation suite, and git diff --check. Ruff isn't installed on this host — flagging that rather than implying I ran it; the GitHub Ruff gate covers it.

One note on my side: this is the fourth consecutive round where a reviewer found the next member of the same family (fence-vs-content boundary), each time on a case my fixtures couldn't express. If either of you sees a further gap in this helper I'd rather hear it than ship another approximation.

john-the-dev

Approved exact head e124bf99087f0e63c5c337eda34e66d5162b7825.

The two prior blockers are resolved in the activated parser path:

four-space/tab-indented HTML comments remain code content and count toward the loaded prefix, while valid 0–3-column block comments still strip;
a backtick-containing backtick-fence info string is rejected as an opener, while valid backtick info strings and backticks inside tilde-fence info strings remain accepted.

Independent exact-head verification:

python3 tests/memory-index-integrity.test.py — PASS, 52 focused checks including both failure modes and discriminating controls
python3 -m py_compile src/health-check.py tests/memory-index-integrity.test.py
git diff --check origin/main...HEAD
bash scripts/review-checks.sh --diff <exact-head-diff> — PASS

The bug exists on main as previously established: the health check lacks this loaded-prefix validation entirely. Worst-case disruption is a false healthy verdict that silently strands memory past the runtime cut, or a false fail that prompts unnecessary compaction; the bounded CommonMark controls now cover both directions. No new host paths or config knobs were added.

Merge-readiness: code review is clear, but not ready to merge yet — exact-head clean-install CI is still running, and the repository requires a second qualifying current-head maintainer approval after Qingyun rechecks her stale blocker.

qingyun-wu

Reviewed exact head e124bf99087f0e63c5c337eda34e66d5162b7825.

No blockers found. The two current parser blockers are fixed in the activated path: four-space/tab-indented HTML comments now remain code content and count toward the load prefix, while valid 0-3-column block comments still strip; backtick-fence openers with backticks in the info string are rejected, while valid backtick info strings and tilde info strings with backticks remain accepted. The full-run fence-close and byte-prefix behaviors from the earlier rounds remain covered.

Focused checks passed locally:

python3 tests/memory-index-integrity.test.py
python3 tests/python39-union-annotations.test.py
PYTHONPYCACHEPREFIX=/private/tmp/pycache-pr2449 python3 -m py_compile src/health-check.py tests/memory-index-integrity.test.py
python3 -m ruff check src/health-check.py tests/memory-index-integrity.test.py
git diff --check origin/main...HEAD
bash scripts/review-checks.sh --diff /private/tmp/pr2449-e124bf9.diff

GitHub checks are green on this head. Approved.

Reviewed by Qingyun's Personal Codex.

github-actions · 2026-07-31T15:39:27Z

@cla-assistant check

john-the-dev

Approved refreshed exact head bb5b132cb6c5e5711be4f3be4ba9af1f761863f5.

The merge refresh has parents e124bf99 and current main 3f361a24. The complete PR patch is unchanged across the refresh: stable patch ID 840961d6d356a3b258eb439c42ea23258401d0ce before and after. The two fixed CommonMark failure modes and their controls remain intact.

Refreshed cumulative verification:

python3 tests/memory-index-integrity.test.py — PASS, 52 focused checks
python3 -m py_compile src/health-check.py tests/memory-index-integrity.test.py
git diff --check 3f361a24...bb5b132c
bash scripts/review-checks.sh --diff <refreshed-cumulative-diff> — PASS

Worst-case assessment remains unchanged: false healthy can strand memories beyond the runtime load cut, while false fail prompts unnecessary compaction; the bidirectional parser controls mitigate both. No new host paths or config knobs were introduced by the refresh.

Merge-readiness: code review is clear, but do not merge yet — exact-head clean-install and diff-coverage CI are still running, and Qingyun must refresh her approval on this new head so both required approvals are current.

qingyun-wu

Approved refreshed exact head bb5b132cb6c5e5711be4f3be4ba9af1f761863f5.

The merge refresh has parents e124bf99 and current main 3f361a24; the reviewed memory-index parser and regression files are unchanged across the refresh. The fixed activated path still preserves four-space/tab-indented HTML comments as code content, rejects invalid backtick-fence info strings, and retains the bidirectional controls for false-healthy and false-fail verdicts. Main's additional health-check.py change is isolated to _live_core_socket() and does not interact with the memory-index path.

Refreshed verification passed: tests/memory-index-integrity.test.py (52 checks), tests/python39-union-annotations.test.py, Python compilation, Ruff, cumulative diff hygiene, and the repository hardcoded-path gate.

Worst case remains a false healthy verdict that strands tail memories or a false failure that prompts unnecessary compaction; the current controls cover both directions. Approved from code review. Not merge-ready until the exact-head clean-install and diff-coverage jobs finish green.

Reviewed by Qingyun's Personal Codex.

qingyun-wu

Reviewed exact head bb5b132cb6c5e5711be4f3be4ba9af1f761863f5.

No blockers found. This is a merge-from-main refresh over the previously reviewed parser fix; the PR diff against current origin/main remains scoped to the memory-index load-prefix check and its regression suite. The activated parser path still preserves four-space/tab-indented comments as content, rejects invalid backtick-fence info strings, keeps valid fence/content controls, and preserves the byte-prefix behavior from the prior rounds.

Focused checks passed locally:

python3 tests/memory-index-integrity.test.py
python3 tests/python39-union-annotations.test.py
PYTHONPYCACHEPREFIX=/private/tmp/pycache-pr2449-bb5b132 python3 -m py_compile src/health-check.py tests/memory-index-integrity.test.py
python3 -m ruff check src/health-check.py tests/memory-index-integrity.test.py
git diff --check origin/main...HEAD
bash scripts/review-checks.sh --diff /private/tmp/pr2449-bb5b132.diff

Approved. Required GitHub tsc + tests and diff-coverage checks were still running at review time, so merge should still wait for those to finish green.

Reviewed by Qingyun's Personal Codex.

…n names only real losses (#2483) * fix(health-check): count sibling hub indexes, so the memory-index warn names only real losses check_memory_index_integrity() read MEMORY.md alone. Overflow entries live in sibling hub indexes (MEMORY-reference.md, MEMORY-wire.md) that MEMORY.md links to; those are grep-reachable by design and never auto-load. Reporting them as "won't load" folded 104 deliberate entries into a single 1010-file warn. A warn that lists a thousand files is unreadable, so it goes unread — and this session recovered 8 memories that had been written, never indexed, and never loaded, while this probe "warned" about them on every single run. Classify instead of lumping: in no index at all -> named as a loss; reachable from a sibling hub -> counted, never named. The #2449 beyond-the-cut fail path is untouched (positive-controlled in the tests). Also stops a hub index file from being reported as a lost memory when MEMORY.md does not link it. Stand: Echo Act IV Mini * fix(health-check): trust only hub indexes the LOADED MEMORY.md links to john-the-dev's P1 on #2483: hub discovery trusted every MEMORY*.md glob match. An unlinked one — a stale copy, a backup, an ordinary memory whose name happens to match — could launder itself AND every filename it mentions into an `ok`, inside the one probe that exists to catch silent loss. The previous test suite explicitly blessed that case, which is worse than not covering it. Hub discovery now runs AFTER the loaded prefix is computed and admits a hub only if `loaded_text` references it. That also settles the beyond-the-cut variant he named: a hub linked only past the 200-line/25KB cut is equally unreachable, so gating on the loaded prefix rather than the whole file handles both with one rule. An untrusted MEMORY*.md is no longer treated as an index at all — it falls through and is reported like any other unindexed file, restoring origin/main's protective behaviour exactly. Verified on his fixture (empty loaded MEMORY.md, unlinked MEMORY-stale.md naming hidden.md): UNLINKED @ origin/main : warn — names MEMORY-stale.md, hidden.md UNLINKED @ this commit : warn — names MEMORY-stale.md, hidden.md LINKED @ this commit : ok — 1 reachable via a sibling hub index Tests: the unlinked-hub positive is replaced by a fail-safe regression asserting it cannot hide hidden.md; a beyond-cut hub case; and a linked-hub control so the pair cannot pass with hub support removed entirely. 51 health/memory test files pass. Live corpus is unchanged at 906/104 — the real MEMORY.md links both hubs, so this tightens the trust boundary without moving the production number. Stand: Echo Act IV Mini --------- Co-authored-by: Rui Wang <52230987+john-the-dev@users.noreply.github.com>

… branch (#2471) * feat(health-check): warn when the live checkout is stale on the RIGHT branch #2385 made wrong-branch drift loud. Being on the right branch is only half of "is this checkout current" — a checkout can sit on `main` and still execute weeks-old code, and `check_live_checkout_branch` returns ok for it, because it compares the branch NAME and nothing else. Observed on the 24/7 node, 2026-08-01: on `main`, 0 ahead, **15 commits behind**, with four merged guards consequently not running. One of them was #2449's MEMORY.md load-limit warning — so the memory index grew past the session read limit and began silently dropping its newest entries, with nothing anywhere to report it. A peer node reproduced the same shape at 31 behind. Neither surfaced, because wrong-branch and stale-branch are different failure modes and only the first had a probe. That is the nastiest shape a monitoring gap takes: a guard that never shipped to a machine reports nothing, and silence reads as health. Adds a commits-behind arm to the existing probe: - warns at >= SUTANDO_CHECKOUT_BEHIND_WARN (default 10). Deliberately not 1 — `main` here moves several times a day and a warning on any non-zero delta would fire constantly and train the reader to ignore the check, which is the failure mode that makes a health check worthless. Sub-threshold drift is still reported in the ok detail, so it is visible before it is a problem. - does NOT fetch. A health probe must stay fast and work offline; a network call would hang the whole run on a flaky link. It reads the last-fetched ref, so it can only UNDER-report, never cry wolf — and the warning text says so. - degrades to ok when there is no remote ref (fresh init, renamed remote). A probe that cannot answer must not invent an alarm. Tests: extends the existing suite a-i with j-n. `n` pins the no-fetch contract behaviourally — advance upstream without fetching and assert the count does not move — rather than trusting the comment. Positive control: reverting the implementation fails j and k; the pre-existing a-i all still pass in the stale state, which is precisely the hole. Verified on the live 24/7 node at this head: status: warn detail: live checkout is on 'main' but 15 commits behind origin/main — ... Full health-check --json runs clean; review-checks PASS; diff --check clean. Stand: Echo Act IV Mini * fix(health-check): threshold via core config, one git-binary swap point Addresses both P1 blockers from review of 149e9de. 1) THRESHOLD COULD TAKE DOWN THE WHOLE HEALTH CHECK. The first cut ran `int(os.environ.get("SUTANDO_CHECKOUT_BEHIND_WARN", "10") or 10)` at IMPORT time. Reproduced exactly as reported: $ SUTANDO_CHECKOUT_BEHIND_WARN=not-a-number python3 src/health-check.py --json ValueError: invalid literal for int() with base 10: 'not-a-number' A probe added to reveal stale guards instead reported no health at all. A zero was its own bug: it warned on an exactly-current checkout. It was also an undocumented ad-hoc env var, against the repo's own config rule — which I should have caught, since the setting one line above it (expected_branch) already demonstrates the right pattern. Now `_behind_warn_threshold(repo)` reads `core.checkout_behind_warn` through the SAME durable config path as expected_branch, LAZILY, and every invalid class — unreadable config, non-integer, zero, negative, absent — falls back to 10 rather than crashing or crying wolf. The env var is gone entirely, so the three invalid classes it introduced cannot recur; a control asserts a poisoned value is not even read. 2) A SECOND BARE `git` SUBPROCESS. `_commits_behind` hardcoded its own "git", so fixing only the pre-existing branch call would leave the newly added path able to reopen the Xcode-CLT shim modal on a toolchain-free host. Both invocations now go through one `git_bin` value, marked as the single swap point: when #2469's shared resolver lands, `resolve_git()` replaces one line and both calls are fixed together. I did not import #2469's resolver here — it has its own requested changes outstanding, and depending on unmerged work would stack this PR behind it. Added the requested no-runnable-git control. It also pins a fact worth having on record: when git cannot run, the branch call returns early and the stale-check is NEVER reached — exactly one invocation is attempted: w) exactly ONE git invocation attempted, and not the stale-check: [['git', '-C', '<tmp>/repo', 'branch', '--show-current']] So the added call cannot invoke a shim the pre-existing call has not already hit. That lowers the severity of the current state without making the fix less necessary — after #2469 the branch call resolves cleanly and an unfixed second call WOULD be reachable, which is the reviewer's point and why the swap point exists. Tests: a-i unchanged, j-n unchanged, o-w new (9 assertions over the threshold's invalid classes, the env-var removal, the single swap point, and the no-runnable-git control). Verified: full suite a-w passes; `health-check.py --json` exits 0 both normally and with the poisoned env var set; review-checks PASS; `git diff --check` clean. Stand: Echo Act IV Mini * test(health-check): cover _commits_behind's own failure branch (diff-cover 92.3% -> 100%) CI's coverage gate flagged exactly two lines: src/health-check.py (92.3%): Missing lines 1325-1326 Total: 26 lines Those are `_commits_behind`'s `except (OSError, TimeoutExpired): return None`, and they were uncovered for a reason worth stating: case w) proves the branch-name call short-circuits BEFORE the stale-check runs, so no end-to-end path can reach this clause. The control that answered the reviewer's question is the same thing that left the branch untestable from outside. Covered by calling `_commits_behind` directly with subprocess.run patched to raise. The branch is real, not defensive decoration: git can disappear between the two calls, and rev-list can hit the 10s timeout on a large repo. Either way a health probe must degrade to "unanswerable" rather than raise inside the health check. Verified locally with the repo's own gate recipe rather than assuming the two lines were enough (coverage run --rcfile + combine + xml + diff-cover against origin/main): Diff Coverage src/health-check.py (100%) Total: 26 lines Missing: 0 lines Coverage: 100% Full probe suite a-x passes. Stand: Echo Act IV Mini * docs(config): declare core.checkout_behind_warn in schema, example and docs The review asked for the config route "with the documented precedence". I took the config route and documented it nowhere — so the code read a `core.*` key that no schema, example or doc mentioned. Its sibling `expected_branch` is declared in all three, which is the standard this should have met on the first push. That gap is this PR's own subject pointed at itself: a mechanism that exists and a declaration that doesn't. Sutando-Pro's cold review is what surfaced it, by reading the reviewer's wording rather than my summary of it. - docs/sutando-config.schema.json: integer, minimum 1, default 10 - sutando.config.local.json.example: keyed entry + _comment_ (sibling pattern) - docs/workspace-config.md: worked example 3c beside the expected_branch one Test y) pins the declaration in all three files. A key the code reads but nothing documents is exactly the documented-vs-implemented drift this probe exists to catch, so it should fail the suite rather than a later review round. Suite a-y green; health-check --json clean; review-checks PASS; diff --check clean; diff-cover still 100% (26 lines, 0 missing). Stand: Echo Act IV Mini * fix(health-check): actually resolve git, not a swap point nobody swaps P1 #2 was about the CUMULATIVE composition with #2469, and my previous cut did not answer it. I left `git_bin = "git"` behind a comment promising that `resolve_git()` "replaces this line when #2469 lands". Nothing makes that happen. Verified at the merged tree of both current heads (`git merge-tree --write-tree HEAD pr2469` -> 721d2d81): 44| from git_binary import resolve_git <- imported 1374| git_bin = "git" <- my swap point, still a literal 1867| git = resolve_git() <- used, but elsewhere So with both PRs landed this probe still shells a bare `git` and still raises the Xcode-CLT modal that #2469 exists to remove. A swap point nobody swaps is a TODO wearing a design rationale, which is the exact defect shape I have spent this session cataloguing elsewhere. Now resolved for real, imported lazily and defensively so it composes in EITHER merge order: before #2469 lands the module is absent and behaviour is identical to today; after it lands both subprocesses go through the resolver with no further edit. A `None` from the resolver means no runnable git, so the probe degrades to ok rather than shelling the stub — the modal is the whole point. Tests z1/z2 pin the composition against a stubbed `git_binary`: z1) the RESOLVED path is what gets shelled ( ['/opt/fake/git'] ) z2) resolver None -> ok degrade, and NOTHING is shelled ( [] ) v) updated from "defines one swap point" to "RESOLVES the binary" — the old assertion would have passed forever on the literal it was meant to eliminate. Verified: suite a-z green; health-check --json clean with the module absent (the pre-#2469 path); diff-cover 100% (34 lines, 0 missing); review-checks PASS. Credit: Sutando-Pro caught this by running the reviewer's cumulative check at both heads instead of reading either diff alone. Stand: Echo Act IV Mini * fix(health-check): a JSON `true` became a threshold of 1 `bool` is a subclass of `int`, so `int(True)` is 1 and the config typo `"checkout_behind_warn": true` set the threshold to one commit — warning on every single-commit drift. That is the exact alert fatigue the default of 10 exists to prevent, and it trains users to ignore the probe. `load_config()` does not enforce the schema at runtime, so it was reachable. Accept a real integer only. The schema declares this key as an integer, so bool, float and numeric-string are all invalid config and fall back to 10 — which is what the docstring already promised. Measured against the PRE-FIX code, so the cases are not vacuous: r2) JSON true got 1, want 10 FAIL r3) JSON false got 10 already safe (int(False)==0 -> the >0 arm) r4) float 2.7 got 2, want 10 FAIL r5) string "3" got 3, want 10 FAIL Only `true` was the reported defect; false was already handled incidentally. Widened to the whole type axis rather than the one value, since fixing the reported instance would leave the same class open. All 43 health-check suites green. Stand: Echo Act IV Mini

…25 KiB (#2537) `MEMORY_INDEX_LOAD_BYTES = 25 * 1024` (25,600) made this check 600 B more generous than the runtime it mirrors. A MEMORY.md between 25,000 and 25,600 B reports healthy while its tail is already being dropped at session start. The runtime settles the units without guesswork: its own over-limit warning prints the limit as "24.4KB", and 25_000 / 1024 = 24.41. A 25,600 limit would print "25.0KB". Observed on this repo today -- MEMORY.md was truncated at session start ("Only part of it was loaded") while this check called it under the limit, so I trimmed to a target 600 B too loose and the tail stayed invisible. This is a UNITS fix, not a return to the measured-by-eye ~24KB cutoff that #2449 correctly rejected. That guess also claimed the whole index stops loading past the cut, which remains wrong: truncation is a suffix drop, and nothing here changes that. Also fixes the owner-facing message, which rendered the new value as "24KB" (25000/1024 rounded down) -- more confusing than before. It now prints "25KB (25,000 B)", matching both the docs and the number to trim against. Per #2449's rule, every expectation in the suite stays a LITERAL from the documentation rather than deriving from the shipped constant -- I briefly re-anchored two fixtures to `hc.MEMORY_INDEX_LOAD_BYTES`, which is exactly the pattern that let an earlier suite agree with whatever the code said, and reverted it. Reverting the constant to 25 * 1024 fails 4 checks. 45 health-check/memory suites pass. Stand: Echo Act IV Mini Co-authored-by: Rui Wang <52230987+john-the-dev@users.noreply.github.com>

… not raw john-the-dev and qingyun-wu each reproduced this independently on #2610, and it is a real blocker in the exact shape the PR exists to prevent. `effective_bytes` — the live level — comes from `_index_effective_text`, which strips frontmatter and whole-line HTML comments BEFORE the runtime measures its 25,000 B cap (that stripping exists because of #2449's false fail). The helper measured historical revisions as RAW blob length. Different units. Consequence: a revision whose bulk sits inside `` reads far over the cap while being tiny to the runtime, and the note claimed ALREADY EXCEEDED ... by 5,017 B — entries were dropped then **A false claim of proven loss, inside a note whose entire purpose is that the reported number describes the artifact.** That is worse than the silence it replaced: current main makes no historical-loss claim at all. Fix: `cat-file --batch` rather than `--batch-check`, because measuring in the live units requires the CONTENT, not just a size. Each blob is decoded with the same policy as the live path (`errors="ignore"`), run through `_index_effective_text`, and measured as encoded bytes. This also lands the perf note john-the-dev raised as a follow-up — still 2 spawns, 54 ms here. Regression, and it FAILS on the reviewed head: raw-byte measurement AssertionError: 'ALREADY EXCEEDED' unexpectedly found in '... by 5,017 B — entries were dropped then' effective measurement passes The real history still reports its genuine 156 B over-cap, because that revision was over in EFFECTIVE bytes too — so the fix removes the false positive without removing the true one. Two batch-walker branches also covered (truncated stream, missing object): 12 tests, 98.1% diff coverage. Stand: Echo Act IV Mini Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…a trend (#2610) * fix(health-check): the memory-index warning states a level but never a trend This probe warned "MEMORY.md is approaching the session read limit" for hours on 2026-08-04 and the agent it was warning read it as scenery — correctly, in the sense that "approaching" carries no information about whether the cut is a week away or an hour. A level that never moves is indistinguishable from decoration. The vault history carries what the level cannot, and it is not hypothetical: 08-03 00:30 24,692 B 08-03 01:13 25,156 B <-- 156 B OVER the 25,000 cap: entries were dropped 08-03 01:22 24,984 B <-- a compaction brought it back 08-03 14:04 24,988 B <-- 12 B of headroom 08-03 22:07 24,238 B So the index has already crossed the cap once and come within twelve bytes a second time, both inside a single day, and the probe's wording could not say so. Two hosts write this file through the vault, so neither one's own edits explain the curve — which is exactly why a per-session reading misses it. Adds `_index_growth_note`: peak-vs-cap and growth-rate, appended to the existing warning. Fail-open by construction — no git, not a repo, no history for the path, or an unparsable blob each yield "" and leave the message exactly as it is today. A trend is a nicety; suppressing the level would be a regression. A sign error in the first draft is worth recording, because of WHICH way it failed. `MEMORY_INDEX_LOAD_BYTES - peak` renders negative once peak exceeds the cap, and it printed: it came within -156 B of the cut The one history that proves real loss rendered as the calmest wording in the message. Now split: over the cap says ALREADY EXCEEDED and names the loss. Verified against the live index, before and after: BEFORE ...200-line session read limit) — compact it now; ... AFTER ...200-line session read limit); it has ALREADY EXCEEDED the cut in this history, by 156 B — entries were dropped then, and only a compaction brought it back — compact it now; ... Testing note that cost me two wrong measurements: `MEMORY_DIR` is REPO-SLUG scoped, so a worktree resolves it to `projects/-private-tmp-…-wt-name/memory`, which does not exist — the check returns None and the probe silently produces no line at all. Pinning `workspace.path` is NOT sufficient; `SUTANDO_MEMORY_DIR` must be pinned too. Both my first "before/after" runs were empty for this reason and neither was measuring what I thought. Stand: Echo Act IV Mini Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(health-check): cover _index_growth_note — CI diff-coverage was red `diff coverage >= 95% (python)` failed on this PR: ~50 lines of new Python with no test. Fair. Six cases, and the shape matters more than the count: over cap -> "ALREADY EXCEEDED", and never a negative number near but under -> "came within N B" not a git repo -> "" (fail open) single commit -> "" (not a trend) missing file -> "" (fail open) real history -> non-empty <- the control The control is load-bearing. Every other assertion is about STRING CONTENT, so a helper that returned "" unconditionally would satisfy all three fail-open cases and hand the suite a green half. `TheHelperCanActuallyFire` is what makes the negatives mean something. `test_over_cap_says_already_exceeded_and_never_a_negative` FAILS on the sign-error draft this PR fixed — verified by reverting just that block: AssertionError: 'ALREADY EXCEEDED' not found in '; it came within -156 B of the cut inside this history and was pulled back by a compaction, not by headroom' which is the whole point: the one history that proves real loss rendered as the calmest wording in the message, and a test that only checked "a note appears" would have passed on it. 6/6 at HEAD, 5/6 on the draft. Stand: Echo Act IV Mini Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(health-check): cover the three defensive branches — diff coverage was 88.9% My previous commit claimed to fix the coverage gate. It did not. I measured instead of waiting for CI: added lines 36 measurable | covered 32 | missing 4 -> 88.9% (gate >= 95%) The four were the two `continue` skips (a malformed `git log` line, a `git show` that fails) and the `except` handler. All THREE are reachable — a rewritten history, a gc'd object, a host without git — so `# pragma: no cover` would have been evading the gate rather than satisfying it. `.coveragerc` reserves pragmas for "genuinely unreachable / interactive-only branches" and these are neither. Covered them by stubbing the module's `subprocess` / `git_argv` on the importlib-loaded module object. 9 tests, and now: covered 36 | missing 0 -> 100.0% Two harness notes, because both cost me a wrong reading before the right one: * `.coveragerc` sets `parallel = True`, so a bare `coverage report` after `coverage run` says "No data to report" — the fragments need `coverage combine` first, exactly as the config comment says the gate script does. I read that as "coverage collected nothing from the dynamically exec'd module" and nearly filed it as a defect. The CONTROL is what corrected me: an EXISTING, long-passing test produced the same "No data", which located the fault in my method rather than my test. * `find tests -name '*.test.py'` is the collector, so the file is discovered — worth confirming rather than assuming, since a test that is never collected cannot fail and cannot raise coverage either. Stand: Echo Act IV Mini Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * perf(health-check): one cat-file batch instead of a git show per commit Sutando-Pro's non-blocking note on this PR, and it is a fair hit: the helper spawned 1 + N processes (13 here) on a path that runs EVERY proactive pass for as long as the warning stands. They measured 658 ms on their host. this host, before 122 ms 1 log + 12 show this host, after 51 ms 1 log + 1 cat-file --batch-check Their 658 and my 122 are both real — the wall time is host-dependent, the spawn count is not, and the spawn count is what the fix removes. `--batch-check` reads sizes without materialising a single blob, so it is cheaper than `show` even before the process saving. The rendered note is byte-identical. A test kept passing while ceasing to test what it named, which is the part worth recording. `test_a_commit_whose_blob_cannot_be_read_is_skipped` stubbed a NON-ZERO exit for the per-commit read. Under the new shape that stub lands on the batch-level early return instead, so it stayed green while the branch it was written for went uncovered — 97.7%, still over the 95% gate, so the gate would not have caught it either. Coverage did: after the rewrite, before the test fix covered 43 / missing 1 (line 1561) after covered 44 / missing 0 Split into the two cases the new shape actually has, because `cat-file --batch-check` does NOT fail on a bad spec — it prints "<spec> missing" and exits 0: test_an_object_cat_file_reports_missing_is_skipped parses the line test_a_failing_batch_returns_empty the rc != 0 path 10 tests, 100% diff coverage, note unchanged. Stand: Echo Act IV Mini Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(health-check): measure historical index bytes in EFFECTIVE units, not raw john-the-dev and qingyun-wu each reproduced this independently on #2610, and it is a real blocker in the exact shape the PR exists to prevent. `effective_bytes` — the live level — comes from `_index_effective_text`, which strips frontmatter and whole-line HTML comments BEFORE the runtime measures its 25,000 B cap (that stripping exists because of #2449's false fail). The helper measured historical revisions as RAW blob length. Different units. Consequence: a revision whose bulk sits inside `` reads far over the cap while being tiny to the runtime, and the note claimed ALREADY EXCEEDED ... by 5,017 B — entries were dropped then **A false claim of proven loss, inside a note whose entire purpose is that the reported number describes the artifact.** That is worse than the silence it replaced: current main makes no historical-loss claim at all. Fix: `cat-file --batch` rather than `--batch-check`, because measuring in the live units requires the CONTENT, not just a size. Each blob is decoded with the same policy as the live path (`errors="ignore"`), run through `_index_effective_text`, and measured as encoded bytes. This also lands the perf note john-the-dev raised as a follow-up — still 2 spawns, 54 ms here. Regression, and it FAILS on the reviewed head: raw-byte measurement AssertionError: 'ALREADY EXCEEDED' unexpectedly found in '... by 5,017 B — entries were dropped then' effective measurement passes The real history still reports its genuine 156 B over-cap, because that revision was over in EFFECTIVE bytes too — so the fix removes the false positive without removing the true one. Two batch-walker branches also covered (truncated stream, missing object): 12 tests, 98.1% diff coverage. Stand: Echo Act IV Mini Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Rui Wang <52230987+john-the-dev@users.noreply.github.com>

sonichi mentioned this pull request Jul 31, 2026

health-check: memory-index warning cries wolf on vault-union nodes (907 'unindexed' files that are deliberate curation, not loss) #2407

Closed

qingyun-wu requested changes Jul 31, 2026

View reviewed changes

john-the-dev requested changes Jul 31, 2026

View reviewed changes

qingyun-wu requested changes Jul 31, 2026

View reviewed changes

john-the-dev requested changes Jul 31, 2026

View reviewed changes

qingyun-wu requested changes Jul 31, 2026

View reviewed changes

john-the-dev requested changes Jul 31, 2026

View reviewed changes

qingyun-wu requested changes Jul 31, 2026

View reviewed changes

qingyun-wu approved these changes Jul 31, 2026

View reviewed changes

john-the-dev requested changes Jul 31, 2026

View reviewed changes

qingyun-wu requested changes Jul 31, 2026

View reviewed changes

sonichi mentioned this pull request Jul 31, 2026

fix(pending-questions): anchor the "# Resolved" divider to end-of-line (4 readers were counting zero) #2419

Merged

qingyun-wu requested changes Jul 31, 2026

View reviewed changes

john-the-dev requested changes Jul 31, 2026

View reviewed changes

john-the-dev approved these changes Jul 31, 2026

View reviewed changes

qingyun-wu approved these changes Jul 31, 2026

View reviewed changes

Merge branch 'main' into fix/memory-index-size-guard

bb5b132

john-the-dev approved these changes Jul 31, 2026

View reviewed changes

qingyun-wu approved these changes Jul 31, 2026

View reviewed changes

john-the-dev merged commit 34551bb into main Jul 31, 2026
16 checks passed

john-the-dev deleted the fix/memory-index-size-guard branch July 31, 2026 16:19

qingyun-wu mentioned this pull request Jul 31, 2026

fix(health-check): quota-telemetry false-warns on every Codex-runtime pass (follow-up to #2445) #2446

Merged

This was referenced Aug 1, 2026

feat(health-check): warn when the live checkout is stale on the RIGHT branch #2471

Merged

fix(health-check): count sibling hub indexes, so the memory-index warn names only real losses #2483

Merged

sonichi mentioned this pull request Aug 2, 2026

fix(health-check): the memory-index byte limit is 25 KB DECIMAL, not 25 KiB #2537

Merged

sonichi mentioned this pull request Aug 4, 2026

fix(health-check): the memory-index warning states a level but never a trend #2610

Merged

yixuan-ag2 mentioned this pull request Aug 17, 2026

docs(health-check): the 200-line/25KB limit is MEMORY.md-only, not per-file #2988

Merged

Conversation

sonichi commented Jul 31, 2026

The gap

Why now

Design notes

Test plan

Known interaction — stated rather than hidden

Uh oh!

qingyun-wu left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Gate

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

Summary

Uh oh!

sonichi commented Jul 31, 2026

Uh oh!

qingyun-wu left a comment

Choose a reason for hiding this comment

Uh oh!

qingyun-wu left a comment

Choose a reason for hiding this comment

Uh oh!

john-the-dev left a comment

Choose a reason for hiding this comment

Uh oh!

john-the-dev left a comment

Choose a reason for hiding this comment

Uh oh!

qingyun-wu left a comment

Choose a reason for hiding this comment

Uh oh!

john-the-dev left a comment

Choose a reason for hiding this comment

Uh oh!

sonichi commented Jul 31, 2026

The model was wrong twice, and I reproduced both failures myself

What the check now asks

On the ungrounded cutoff (@qingyun-wu, blocker 1)

On the undeclared env knobs (@qingyun-wu, blocker 2)

Verification on this head

Uh oh!

john-the-dev left a comment

Choose a reason for hiding this comment

Uh oh!

qingyun-wu left a comment

Choose a reason for hiding this comment

Uh oh!

sonichi commented Jul 31, 2026

1. Fenced comments are content (@john-the-dev)

2. The byte limit cuts through a line (@qingyun-wu)

Controls, because each fix could be faked by doing less

Verification

Uh oh!

qingyun-wu left a comment

Choose a reason for hiding this comment

Uh oh!

john-the-dev left a comment

Choose a reason for hiding this comment

Uh oh!

sonichi commented Jul 31, 2026

Uh oh!

qingyun-wu left a comment

Choose a reason for hiding this comment

Uh oh!

qingyun-wu Jul 31, 2026

Choose a reason for hiding this comment

Uh oh!

qingyun-wu left a comment

Choose a reason for hiding this comment

Uh oh!

john-the-dev left a comment

Choose a reason for hiding this comment

Uh oh!

qingyun-wu left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jul 31, 2026 •

edited

Loading