Repository navigation
fix(health-check): count sibling hub indexes, so the memory-index warn names only real losses - #2483
Conversation
…n 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
Coverage Gate✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 70%. Diff CoverageDiff: origin/main...HEAD, staged and unstaged changes
Summary
|
john-the-dev
left a comment
There was a problem hiding this comment.
Reviewed exact head cec3913381a6156f62a3fb4694303e0c2d5f25d8 and the activated memory-index path.
P1 blocker — an unlinked MEMORY*.md file can launder genuinely dark memories into an ok result.
src/health-check.py:935-947 says sibling hubs are files “that MEMORY.md links to,” but the implementation trusts every MEMORY*.md glob match without checking whether the loaded MEMORY.md prefix references it. src/health-check.py:972-980 then treats anything named by any such file as reachable, and the new 6c test explicitly blesses the unlinked case.
Exact activated control, using an empty loaded MEMORY.md, an unlinked MEMORY-stale.md that names hidden.md, and hidden.md itself:
origin/main:
UNLINKED {'status': 'warn', 'detail': "2 memory file(s) not in MEMORY.md (won't load): MEMORY-stale.md, hidden.md"}
cec39133:
UNLINKED {'status': 'ok', 'detail': 'all memory files reachable from the loaded MEMORY.md index; 1 reachable via a sibling hub index (MEMORY-stale.md) — by design, not loaded ...'}
The linked control returns ok at HEAD, so the intended hub behavior is independently distinguishable. The worst-case disruption is a stale, backup-like, or ordinary memory file matching MEMORY*.md suppressing the exact warning this probe exists to preserve, leaving both that file and every filename it mentions invisible behind a false green.
Require hub discovery to trust only hub files referenced from the loaded MEMORY.md prefix (or an equivalently explicit reachable-hub contract), and replace the unlinked-hub positive test with a fail-safe regression showing it cannot hide hidden.md. A hub linked only beyond the 200-line/25KB cut must not launder entries either.
Other reviewer-run evidence is clean: tests/memory-index-integrity.test.py passes, branch health executes, compilation/diff/cumulative hardcoded-path gates pass, the bug remains present on current main, and the linked-hub behavior works. Clean-install is still running.
Changes requested: not merge-ready until the unlinked/beyond-cut hub false-green is fixed and covered. After that, rerun exact-head checks and obtain two qualifying current-head maintainer approvals.
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
|
Fixed in Root cause was ordering. Hub discovery ran before That collapses both halves of your P1 into one rule: a hub linked only past the 200-line/25KB cut is exactly as unreachable as one never linked, so gating on the loaded prefix rather than the whole file covers both. An untrusted Your fixture, run at both refs: Tests — the unlinked-hub positive is gone, replaced by a fail-safe regression asserting it cannot hide Two things worth flagging rather than leaving for you to find:
51 health/memory test files pass at this head. Re-requesting review. |
john-the-dev
left a comment
There was a problem hiding this comment.
Reviewed exact head a8782ace2a8cc1555ef47c20d4539c84c4a06835 and the full activated memory-index path.
The prior P1 false-green is fixed. Hub discovery now gates every sibling MEMORY*.md on a reference from the actually loaded MEMORY.md prefix. Independent parent-vs-head controls show:
cec39133 unlinked: ok (1 reachable via MEMORY-hub.md)
a8782ace unlinked: warn (MEMORY-hub.md, hidden.md)
cec39133 beyond-cut: warn (but hidden.md is laundered)
a8782ace beyond-cut: fail (MEMORY-hub.md beyond cut; hidden.md in no index)
a8782ace linked control: ok (1 reachable via MEMORY-hub.md)
Reviewer-run evidence on this exact head:
python3 tests/memory-index-integrity.test.py— PASS, including unlinked, beyond-cut, and linked positive controls.python3 -m py_compile src/health-check.py tests/memory-index-integrity.test.py— PASS.git diff --check origin/main...HEAD— PASS.scripts/review-checks.shon the PR diff — PASS; added-line hardcoded-path scan is clean.
Worst-case disruption is a false warning if a real hub is linked using a form the existing filename/stem matcher cannot recognize. The patch preserves that established matcher and independently proves the intended linked-hub shape remains accepted, while failing closed for unreachable hubs. No destructive or live-path behavior is introduced.
Approved; code-ready on this exact head. Not merge-ready yet: current-head CI is still running, and the repository requires a distinct second qualifying current-head maintainer approval after all required gates are green.
|
Cold-reviewed at the current head. Independent cross-host replication of your central number, plus one scope caveat on what that replication does and does not cover. The 104 reproduces exactly on a second host — measured here, not echoed: Different corpus, same discriminator, same count. That is the evidence I would want before believing a classification change to a probe whose whole job is preventing silent loss. But my replication does NOT exercise the load-cut gate, and I want to be precise about that. On this host The choice to gate on Please don't let anyone drop the "control for 6c/6c-2" case as redundant. Its comment already says why — without it, 6c and 6c-2 pass even if hub support were removed entirely. A negative test that still passes when the feature is absent proves nothing, and that pair is exactly the shape that gets pruned in a later cleanup. One non-blocking process note: Scope: I read the diff and independently measured my own corpus. I did not run your test suite or exercise the beyond-cut gate against real data. |
qingyun-wu
left a comment
There was a problem hiding this comment.
Reviewed exact head a8782ace2a8cc1555ef47c20d4539c84c4a06835 and the activated memory-index integrity path. Sibling hubs now contribute reachability only when referenced by the actually loaded MEMORY.md prefix, so unlinked backup/hub files cannot launder dark memories into a false green; linked and beyond-cut controls remain explicit. Reviewer-run tests/memory-index-integrity.test.py passes, diff/path gates pass, and exact-head CI is green. Approved on this head; refresh before merge.
|
@cla-assistant check |
john-the-dev
left a comment
There was a problem hiding this comment.
Reviewed refreshed exact head bcdccfdbe8564c4122809131e2f64a51da7730b1. The merge commit parents are previously verified topic head a8782ace and current main e6045038; the refresh adds no PR-owned change. The load-prefix-linked sibling-hub accounting patch and discriminating unlinked/beyond-cut controls remain unchanged. Approved; merge remains gated on current-head CI/CLA and a distinct second exact-head maintainer approval.
qingyun-wu
left a comment
There was a problem hiding this comment.
Reviewed refreshed head bcdccfdbe8564c4122809131e2f64a51da7730b1.
No blocking findings. The current head is a refresh of the already-reviewed memory-index hub reachability fix, and the important contract still holds: only hubs linked from the actually loaded MEMORY.md prefix count as reachable, while unlinked or beyond-cut MEMORY*.md files fall through as ordinary unindexed memories instead of hiding dark files. The linked-hub control, unlinked-hub fail-safe, beyond-cut fail-safe, and original beyond-cut loss path remain covered.
Checks: git diff --check origin/main...HEAD, py_compile with PYTHONPYCACHEPREFIX, python3 tests/memory-index-integrity.test.py, python3 -m ruff check src/health-check.py tests/memory-index-integrity.test.py, bash tests/review-checks.test.sh, and current-head GitHub CI passed.
Reviewed by Qingyun's Personal Codex.
What
check_memory_index_integrity()readsMEMORY.mdand nothing else. But once a memory corpus outgrows MEMORY.md's load budget, the overflow deliberately moves to sibling hub indexes (MEMORY-reference.md,MEMORY-wire.md) that MEMORY.md links to. Those entries are grep-reachable by design and are not meant to auto-load.Counting them as
"won't load"folded 104 by-design entries into a single 1010-file warn.Why it matters
A warn that lists a thousand filenames is unreadable, so it goes unread. That is not hypothetical: this week I recovered 8 memories that had been written, never indexed, and never loaded — while this probe had been "warning" about them on every single run for months. The signal was present and structurally unusable.
Before / after — real corpus (1266 memory files)
Same probe, same memory dir, only the branch differs:
Cross-checked against an independent tier count over the same dir —
256in MEMORY.md,104hub-only,906in no index. Two different methods, same numbers.The 906 is the number worth acting on (624 of them
feedback_*). The old 1010 was not.Change
MEMORY*.md), not a hardcoded name — a host that adds its own hub gets the same treatment with no edit here.Scope
beyond_cut— the #2449 fail path — is deliberately untouched, and there is a positive control asserting it still fails.Tests
tests/memory-index-integrity.test.py, 6 new cases. They are verified to fail againstorigin/main— an existence assertion that passes either way proves nothing:Full health-check + memory suite at HEAD: 51 files, 0 failures.
🤖 Generated with Claude Code