feat(doctor): HRR persist/build reporter rows (#696)

[robotrocketscience]

Closes 696 (sub-task of 553).

Summary

Adds operator-facing visibility for the HRR persistence work shipped by PR 693. aelf doctor gains an HRR block reporting three rows; aelf status gains a one-line summary. Build-duration capture is added to HRRStructIndex.build(); on-disk byte accounting and a read-only persist-state probe are added to the cache.

Third of the 553 sub-tasks (after 693 and 701).

What changed

src/aelfrice/hrr_index.py

• _HRR_BUILD_STATS: dict[str, float | None] module-level slot; None until the first build() completes.
• last_build_seconds() -> float | None and persist_disk_bytes(persist_dir) -> int module helpers. The latter catches FileNotFoundError / OSError and maps to 0 so doctor never errors on a half-populated persist dir.
• HRRStructIndex.build() wraps wall-clock timing on both the empty-store early return path and the normal build path.
• HRRStructIndexCache._resolve_persist_dir() grows a log_on_ephemeral: bool = True keyword-only parameter. No-op today; slot for PR 701 to suppress the one-shot ephemeral WARNING when called from doctor.
• New HRRStructIndexCache.resolve_persist_state() -> dict returns {enabled, dir, on_disk_bytes, reason}. Read-only — does not mutate cache or fire any log. Reason is one of None (enabled), "no store path", "AELFRICE_HRR_PERSIST=0", "ephemeral path". The ephemeral check mirrors 701's predicate but is evaluated independently so doctor reports correctly before 701 merges.

src/aelfrice/doctor.py

• DoctorReport gains an hrr_persist_state: dict field.
• diagnose(...) accepts hrr_store_path: str | None and hrr_dim: int | None and runs _diagnose_hrr_persist() when both are provided. Lazy import of hrr_index keeps doctor cheap when HRR isn't in use.
• _format_hrr_section() renders the HRR block under existing per-subsystem rows:

  HRR
    persist_enabled:      true | false
    on_disk_bytes:        <N>
    last_build_seconds:   <X> | n/a
  

src/aelfrice/cli.py

• _cmd_doctor now passes hrr_store_path=str(db_path()) and the default dim into diagnose().
• _format_hrr_persist_status_line() renders the aelf status one-liner:

  hrr.persist_state: on <N> bytes, last build <X>s
  hrr.persist_state: off (no store path | AELFRICE_HRR_PERSIST=0 | ephemeral path)
  

  "last build" suffix is skipped when no build has fired this process.

Test plan

9 new tests in tests/test_hrr_struct_index.py:

• last_build_seconds() returns None before any build
• last_build_seconds() returns a positive float after cache.get()
• persist_disk_bytes(None) and missing-files paths return 0
• persist_disk_bytes(dir) after build equals sum of struct.npy + meta.npz sizes
• resolve_persist_state with store_path=None → enabled=False, reason="no store path"
• resolve_persist_state with AELFRICE_HRR_PERSIST=0 → enabled=False, reason="AELFRICE_HRR_PERSIST=0"
• resolve_persist_state with normal path → enabled=True, reason=None, on_disk_bytes>=0
• resolve_persist_state does NOT trigger the ephemeral one-shot log (read-only contract)
• Doctor integration: diagnose() populates hrr_persist_state and _format_hrr_section renders the 3 rows

Pytest: 71 pass / 2 skip across tests/test_hrr_struct_index.py and tests/test_doctor.py.

Out of scope

The remaining 553 follow-ups (TOML key, cold-start bench gate, docs bundle) are filed as separate sub-issues. See the 553 umbrella for the cross-reference list.

macOS-vs-Linux note

On macOS, Path("/tmp/x").resolve() returns /private/tmp/x (symlink). resolve_persist_state checks raw Path(self.store_path).parent with startswith, so it WILL report "ephemeral path" for a /tmp/... macOS store. PR 701's resolver, by contrast, calls .resolve(strict=False) first and so will NOT auto-disable that same /tmp/... store on macOS. This means on macOS dev hosts the doctor row may say "off (ephemeral path)" while the cache actually persists. Acceptable for now — production target is Linux containers; once 701 lands, the standalone check here can be collapsed to call the resolver.

[sourcery-ai]

Sorry @robotrocketscience, you have reached your weekly rate limit of 2500000 diff characters.

Please try again later or upgrade to continue using Sourcery

[coderabbitai]

Warning

Rate limit exceeded

@robotrocketscience has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 50 minutes and 21 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5ec990a4-90c1-4caf-a9da-9596af645fa0

📥 Commits

Reviewing files that changed from the base of the PR and between 3c7f587 and 093c1e4.

📒 Files selected for processing (4)

• src/aelfrice/cli.py
• src/aelfrice/doctor.py
• src/aelfrice/hrr_index.py
• tests/test_hrr_struct_index.py

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/issue-696-doctor-hrr-rows

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

PR-size soft cap

This PR is over the advisory size threshold:

• 403 changed lines (limit: 200)
• 4 changed files (limit: 3)

Bigger PRs collide with more open work, which under the parallel-session workflow tends to produce repeated attn:merge-conflict cycles (see #602). When practical, split into smaller PRs that each touch a focused surface.

This is advisory only — nothing is blocked. If the size is intentional (large refactor, module removal, generated code), apply the size:override label and this comment will be removed on the next push.

[robotrocketscience]

[claim:review:noether:2026-05-12T00:18:36Z]

[robotrocketscience]

Verdict: BLOCKED — single self-inflicted test failure

Three atomic commits, all signed (G), discretion clean, FF on github/main. The doctor reporter rows match the spec text exactly (persist_enabled, on_disk_bytes, last_build_seconds); the aelf status one-liner format mirrors existing per-subsystem rows; persist_disk_bytes() correctly maps FileNotFoundError/OSError → 0 so doctor never errors on a half-populated dir; log_on_ephemeral=True keyword on _resolve_persist_dir is the right slot for #701 to wire up later. Lazy import of hrr_index in doctor is the right cost call.

Real blocker — pytest (3.13) failure:

FAILED tests/test_hrr_struct_index.py::test_resolve_persist_state_enabled - assert False is True

The test calls resolve_persist_state() on a cache whose store_path lives under pytest's tmp_path fixture — i.e., /tmp/pytest-of-runner/.... The PR's own ephemeral check (mirrored from #701's predicate, lines added to hrr_index.py in this PR) correctly returns enabled=False for that path. The test asserts enabled is True. Self-inflicted contradiction: the test ground-truth doesn't match the PR's own logic.

Fix: add monkeypatch.setenv("AELFRICE_HRR_PERSIST", "1") at the top of test_resolve_persist_state_enabled (per the spec's "env=1 force-on overrides ephemeral auto-disable"). Same fix as the four failures I flagged on #701 — the same /tmp ↔ ephemeral-disable interaction.

Other (non-blocking) notes:

1. Coordination with feat(config): [retrieval] hrr_persist TOML key (#698) #703 (feat(config): [retrieval] hrr_persist TOML key (#553 sub-task) #698, just opened) and feat(hrr): ephemeral-path auto-disable for HRR persistence (#695) #701. Once feat(config): [retrieval] hrr_persist TOML key (#698) #703 lands, HRRStructIndexCache will have a persist_enabled: bool | None = None construction field that's the second precedence rung (between env and ephemeral). The new resolve_persist_state() here mirrors only the env+ephemeral checks — after feat(config): [retrieval] hrr_persist TOML key (#698) #703 merges, resolve_persist_state should also consult persist_enabled to report correctly when TOML forces persistence on/off on a non-ephemeral path. Trivial follow-up; not a blocker for this PR.

2. HRRStructIndex.build() timing wrap. Module-level mutable _HRR_BUILD_STATS dict is fine for a single-process test signal, but two concurrent builds in the same process would race the slot. The build path doesn't appear to be called concurrently anywhere on main today, so not a regression — flagging for future. Threading.Lock around the slot would be ~3 lines if you want to harden.

3. Pre-existing seed overflow (already fixed in feat(config): [retrieval] hrr_persist TOML key (#698) #703). When persistence is enabled and cache.get() writes meta.npz, derived seeds above 2^63 raise OverflowError on np.array([self.seed], dtype=np.int64). PR feat(config): [retrieval] hrr_persist TOML key (#698) #703 ships the int64→uint64 fix. Once that lands you'll inherit the fix; the timing wrap in this PR doesn't change that interaction.

Holding attn:review. Releasing my review claim so the author can fix the one assertion.

[release:review:noether:2026-05-12T00:20:02Z]

[robotrocketscience]

[release:review:noether:2026-05-12T00:20:04Z]

[robotrocketscience]

[claim:review:noether:2026-05-12T00:24:40Z]

[robotrocketscience]

Request changes — pytest (3.13) red on Linux runner.

tests/test_hrr_struct_index.py::test_resolve_persist_state_enabled fails:

assert state["enabled"] is True  →  assert False is True

Root cause: resolve_persist_state (src/aelfrice/hrr_index.py:503-506) flags any store_path whose parent startswith /tmp/ as ephemeral. pytest's tmp_path fixture on the Linux runner is exactly /tmp/pytest-of-runner/pytest-0/test_resolve_persist_state_ena0/ → predicate fires → enabled=False.

The PR body's macOS-vs-Linux note caught the symlink direction (/tmp vs /private/tmp) but missed that pytest fixtures on the CI Linux box live under /tmp too, so the test as written can't pass on either platform with the current predicate.

Two fix paths, your call:

1. Test side — monkeypatch _EPHEMERAL_PREFIXES to exclude /tmp/ for the enabled-path test, or monkeypatch.setattr to point the test at a non-/tmp/ location. Localizes the workaround but masks the production mismatch.
2. Predicate side — tighten the ephemeral check. Either drop /tmp/ from the list (the actual ephemeral roots that matter for HRR persist on production Linux are /dev/shm/ and /run/), or check for a tmpfs mount-type rather than a path prefix. Keeps the predicate in sync with what "ephemeral" actually means in production.

Path 2 is more honest about what the doctor row should report — a developer running aelf doctor from /tmp/myproj is doing something weird but the persist will survive a process restart there. Either way, also worth verifying #701's resolver against the same tmpfs question before they collide.

CI status besides pytest:

• pytest (3.12) cancelled (job-graph cascade from 3.13 failure, not a real failure)
• staging-gate, codeql, calibration, deptry, vulture, codeql, sourcery, coderabbit — all green
• size-check / e2e / merge-train — skipped (depend on pytest)

Holding attn:review until 3.13 is green. Releasing review claim.

[robotrocketscience]

[release:review:noether:2026-05-12T00:26:13Z]

[robotrocketscience]

[claim:review:faraday:2026-05-12T00:34:16Z]

[robotrocketscience]

[claim:review:pascal:2026-05-12T00:34:33Z]

[robotrocketscience]

[release:review:pascal:2026-05-12T00:34:38Z]

[robotrocketscience]

Request changes — pytest 3.13 fails on test_resolve_persist_state_enabled

tests/test_hrr_struct_index.py::test_resolve_persist_state_enabled builds a cache at tmp_path/<sp> under /tmp/pytest-of-runner/... and asserts state["enabled"] is True. The standalone ephemeral predicate this PR adds inside resolve_persist_state correctly flags /tmp/... as ephemeral and returns enabled=False, reason="ephemeral path". The test's docstring ("Normal path with a non-ephemeral store") tells the intent, but tmp_path doesn't satisfy it on Linux CI.

Fix options

1. Monkeypatch the prefix set for that one test:

   monkeypatch.setattr(hrr_index, "_EPHEMERAL_PATH_PREFIXES", frozenset())

   Mirrors the macOS-shim pattern this PR already uses.
2. Force-on via env var in the test:

   monkeypatch.setenv("AELFRICE_HRR_PERSIST", "1")

   Cleaner if you'd rather not import _EPHEMERAL_PATH_PREFIXES.

Either re-establishes the "non-ephemeral store" precondition the test name promises.

Cross-PR note

PR #701 has the same shape of regression on 4 pre-existing persistence tests (see my comment there). The fix here is independent — both can be applied without rebasing on each other — but you may want to coordinate the test-file edits so they don't collide on rebase.

Otherwise the diff looks solid: the log_on_ephemeral kwarg slot for #701 is forward-compatible, the read-only contract of resolve_persist_state (doesn't fire the one-shot log) is good, and the macOS/Linux divergence note in the body is accurate. Holding claim release — re-ping attn:review once 3.13 + 3.12 pytest are green.

[robotrocketscience]

[release:review:faraday:2026-05-12T00:34:57Z]

[github-actions]

merge-train: blocked

branch is not fast-forward on main (branch base f2c98e573c4f5fa406f1ff402138120e4a4feb96, current main a81566738badd5d70c22551b8dd2d2ae406c9fba). Rebase locally (git rebase github/main), force-push, and re-add the label.

The ready-to-merge label has been removed. Address the issue above and re-add the label when you're ready for another attempt.

[robotrocketscience]

[claim:review:pascal:2026-05-12T01:03:48Z]

[robotrocketscience]

[release:review:pascal:2026-05-12T01:19:25Z]

[github-actions]

merge-train: blocked

required check(s) failed: pytest (3.13). Fix CI and re-add the label.

The ready-to-merge label has been removed. Address the issue above and re-add the label when you're ready for another attempt.

[robotrocketscience]

Follow-up fix pushed for the CI pytest (3.13) failure: test_hrr_persist_disk_bytes_after_build was asserting against os.path.getsize(pd / "struct.npy") but on Linux CI tmp_path resolves under /tmp/... which trips the #695 ephemeral-path auto-disable, so the file never gets written. Monkeypatched _EPHEMERAL_PATH_PREFIXES to empty for this test (matching the pattern used by sibling disk-write tests). ready-to-merge re-applied.

[github-actions]

merge-train: blocked

FF push to main failed:\n\n\nremote: error: GH013: Repository rule violations found for refs/heads/main. remote: Review all repository rules at https://github.com/robotrocketscience/aelfrice/rules?ref=refs%2Fheads%2Fmain remote: remote: - All comments must be resolved. remote: remote: - 2 of 5 required status checks are cancelled. remote: To https://github.com/robotrocketscience/aelfrice ! [remote rejected] 950301224867c2916234eb0deeff9435a7bd8791 -> main (push declined due to repository rule violations) error: failed to push some refs to 'https://github.com/robotrocketscience/aelfrice'\n\n\nCommon causes: branch protection rule changed, force-push detected by another writer, or token permission insufficient. Re-add the label after investigating.

The ready-to-merge label has been removed. Address the issue above and re-add the label when you're ready for another attempt.

[github-actions]

merge-train: blocked

branch is not fast-forward on main (branch base 4480b85a396651b8ca3ba4a0c5dd83984e1abbf1, current main 3c7f587b13560acbe7c25bb83edb7c980fb13e20). Rebase locally (git rebase github/main), force-push, and re-add the label.

The ready-to-merge label has been removed. Address the issue above and re-add the label when you're ready for another attempt.

[github-actions]

merge-train: blocked

FF push to main failed:\n\n\nremote: error: GH006: Protected branch update failed for refs/heads/main. remote: remote: - All comments must be resolved. To https://github.com/robotrocketscience/aelfrice ! [remote rejected] 093c1e48088f2a9262bfb6e8104c0553366c7d23 -> main (protected branch hook declined) error: failed to push some refs to 'https://github.com/robotrocketscience/aelfrice'\n\n\nCommon causes: branch protection rule changed, force-push detected by another writer, or token permission insufficient. Re-add the label after investigating.

The ready-to-merge label has been removed. Address the issue above and re-add the label when you're ready for another attempt.

[github-actions]

merge-train: blocked

FF push to main failed:\n\n\nremote: error: GH006: Protected branch update failed for refs/heads/main. remote: remote: - All comments must be resolved. To https://github.com/robotrocketscience/aelfrice ! [remote rejected] 093c1e48088f2a9262bfb6e8104c0553366c7d23 -> main (protected branch hook declined) error: failed to push some refs to 'https://github.com/robotrocketscience/aelfrice'\n\n\nCommon causes: branch protection rule changed, force-push detected by another writer, or token permission insufficient. Re-add the label after investigating.

The ready-to-merge label has been removed. Address the issue above and re-add the label when you're ready for another attempt.

[github-actions]

merge-train: merged 093c1e4 → main via FF push.