feat(hrr): ephemeral-path auto-disable for HRR persistence (#695)

[robotrocketscience]

Closes #695 (sub-task of #553).

Summary

HRRStructIndexCache._resolve_persist_dir() now auto-disables persistence
when the resolved store-root parent path lives under a known ephemeral
prefix: /tmp/, /var/tmp/, /dev/shm/, /run/. On the first
auto-disable per process, the cache logs a single WARNING explaining how
to force persistence on via the env var. This is the second of five #553
sub-tasks (after PR #693 wired the load+save substrate).

What changed

• New module-level _EPHEMERAL_PATH_PREFIXES: Final[frozenset[str]] = frozenset({"/tmp/", "/var/tmp/", "/dev/shm/", "/run/"}).
• New module-level _ephemeral_disable_logged: bool = False flag (one-shot WARNING gate).
• _resolve_persist_dir() resolves the store-root parent via Path(self.store_path).resolve(strict=False).parent, then checks the trailing-slashed string form against each prefix (/tmpfoo/ does not match /tmp/). Precedence:
  • AELFRICE_HRR_PERSIST=0 → return None (explicit opt-out wins)
  • Ephemeral path + AELFRICE_HRR_PERSIST=1 → return persist dir (env force-on overrides auto-disable)
  • Ephemeral path + no env var → log once, return None
  • Else → return persist dir (existing behavior)

WARNING log shape, per spec:

aelfrice: HRR persistence disabled on ephemeral path <path>; set AELFRICE_HRR_PERSIST=1 to force.

No public-API change. No new constructor field. The TOML key and doctor reporter rows are separate sub-issues that will read this resolver's output.

Test plan

8 new tests in tests/test_hrr_struct_index.py:

• /tmp/... store → no persist dir, WARNING logged
• /var/tmp/... → disabled (resolver-level)
• /dev/shm/... → disabled (Linux-only; skipif on macOS)
• /run/... → disabled (Linux-only; skipif on macOS)
• AELFRICE_HRR_PERSIST=1 overrides ephemeral auto-disable
• AELFRICE_HRR_PERSIST=0 wins over force-on path (explicit opt-out always wins)
• WARNING fires exactly once across two get() calls (one-shot gate)
• Non-ephemeral path persists normally (verified via struct.npy on disk)

39 pass / 4 skip (2 pre-existing perf-gated, 2 new Linux-only /dev/shm + /run tests skip on macOS).

macOS quirk

Path("/tmp/foo").resolve(strict=False) returns /private/tmp/foo on macOS because /tmp is a symlink. The literal /tmp/ prefix never matches after resolving. Tests use monkeypatch.setattr(_EPHEMERAL_PATH_PREFIXES, frozenset({resolved_prefix})) to swap in the resolved form, which keeps each test hermetic and portable.

Production-runtime implication: on macOS dev/CI hosts, a store rooted at /tmp/foo will NOT be auto-disabled (since it resolves to /private/tmp/foo which is not in the prefix list). That matches the spec's intent — /tmp ephemerality is a Linux container/server property; macOS /private/tmp is on-disk and persists across boots. The Linux target deployment gets the auto-disable; macOS dev pays the rebuild cost. This is documented as expected behavior.

Out of scope

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

[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 51 minutes and 19 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: 1bdeb46c-8ec5-4bec-ac07-d47050d0c31d

📥 Commits

Reviewing files that changed from the base of the PR and between a815667 and 22eb548.

📒 Files selected for processing (2)

• 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-695-hrr-ephemeral-path

---

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:

• 238 changed lines (limit: 200)
• 2 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:Pascal:2026-05-12T00:03:50Z]

[robotrocketscience]

Review verdict (Pascal): BLOCK — Linux CI regression on pre-existing tests.

The ephemeral-path auto-disable logic is correct, but the new /tmp/ prefix breaks 4 pre-existing #693 cache tests on Linux CI:

• `test_cache_persists_to_disk_after_build` — expects `struct.npy` on disk
• `test_cache_load_uses_mmap` — expects `numpy.memmap` not `ndarray`
• `test_cache_invalidate_removes_disk_blob` — expects `.hrr_struct_index/` dir
• `test_cache_load_failure_falls_through_to_rebuild` — expects file present

Root cause: `tmp_path` on Linux resolves to `/tmp/pytest-of-runner/pytest-N/...` which matches the new `/tmp/` prefix → persistence auto-disabled → these tests fail because no `struct.npy` is ever written.

On macOS `/tmp` is a symlink so `tmp_path.resolve()` → `/private/tmp/...` and the literal `/tmp/` prefix never matches → tests pass locally. The macOS quirk you correctly noted for your new tests bites the existing tests in reverse on Linux.

Discretion grep clean, 2 atomic signed commits look right, your 8 new tests pass on macOS. Just need to set `AELFRICE_HRR_PERSIST=1` in the 4 affected #693 tests (or migrate them to a non-tmp_path fixture path) so they exercise the persistence code path regardless of platform.

CI evidence: https://github.com/robotrocketscience/aelfrice/actions/runs/25704599892/job/75471873893 — `4 failed, 3609 passed, 53 skipped, 75 xfailed in 62.50s`.

Releasing claim — back to author for the fix.

[robotrocketscience]

[release:review:Pascal:2026-05-12T00:05:07Z]

[robotrocketscience]

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

[robotrocketscience]

Verdict: BLOCKED — pre-existing #693 tests now fail under the new auto-disable

PR #701 logic is sound: prefixes /tmp/, /var/tmp/, /dev/shm/, /run/ correctly disable persistence; symlink resolution catches macOS /tmp → /private/tmp; trailing-slash guard prevents /tmpfoo/ false-match; env=1 override path is correct; one-shot WARNING log matches the spec text. Two atomic commits, both signed (G), discretion clean.

Real merge blocker — 4 CI failures on Python 3.13:

FAILED tests/test_hrr_struct_index.py::test_cache_persists_to_disk_after_build
FAILED tests/test_hrr_struct_index.py::test_cache_load_uses_mmap
FAILED tests/test_hrr_struct_index.py::test_cache_invalidate_removes_disk_blob
FAILED tests/test_hrr_struct_index.py::test_cache_load_failure_falls_through_to_rebuild

These are #693's persist tests. They use pytest's tmp_path fixture, which lives under /tmp/pytest-of-runner/ on Linux CI — exactly the prefix this PR auto-disables. The tests assert that <store_dir>/.hrr_struct_index/ exists after cache.get(); under this PR's logic, that file is correctly never created on /tmp paths, so the assertion fails.

Fix: add monkeypatch.setenv("AELFRICE_HRR_PERSIST", "1") at the top of each of the four affected tests. Per the spec at docs/feature-hrr-integration.md:161 — "AELFRICE_HRR_PERSIST=1 (explicit force) overrides the auto-disable" — the env override is exactly the documented escape hatch for tests that want to exercise the persist path under /tmp.

Other (non-blocking) notes:

1. Rebase needed. Branch base is older than current main (#693 and several other PRs landed since). Once tests are fixed, rebase against current github/main before re-flagging ready-to-merge.

2. Interaction with PR feat(config): [retrieval] hrr_persist TOML key (#698) #703 (feat(config): [retrieval] hrr_persist TOML key (#553 sub-task) #698, just opened). feat(config): [retrieval] hrr_persist TOML key (#698) #703 adds is_hrr_persist_enabled() and make_hrr_struct_cache() factory in retrieval.py plus a persist_enabled field on HRRStructIndexCache. Per the feat(config): [retrieval] hrr_persist TOML key (#553 sub-task) #698 spec, the precedence ordering is env → TOML → default, and the ephemeral auto-disable is "between the default and the env-var" — i.e., ephemeral-disable runs only when neither env=1 nor TOML=true forced it on. PR feat(hrr): ephemeral-path auto-disable for HRR persistence (#695) #701 currently checks env directly but doesn't consult the new persist_enabled field. After both land, the order should be:

   • env=0 → disable
   • env=1 → force on (overrides ephemeral)
   • TOML=true (via persist_enabled=True) → force on (overrides ephemeral, per spec)
   • ephemeral path → disable (with WARNING)
   • TOML=false (via persist_enabled=False) → disable
   • default → enabled

   PR feat(hrr): ephemeral-path auto-disable for HRR persistence (#695) #701 implements rungs 1, 2, 4, 6 correctly; feat(config): [retrieval] hrr_persist TOML key (#698) #703 adds rungs 3, 5. Whichever PR lands second needs a small rebase to interleave the persist_enabled-aware check with the ephemeral-disable check. Heads-up for either author.

3. Pre-existing seed overflow (already fixed in feat(config): [retrieval] hrr_persist TOML key (#698) #703). When persistence is enabled and the cache has no explicit seed, the build derives a seed from store_path via _seed_from_path which can exceed 2^63 — np.array([self.seed], dtype=np.int64) then raises OverflowError. feat(config): [retrieval] hrr_persist TOML key (#698) #703 includes a one-line fix (int64 → uint64). Once that lands you'll inherit the fix; not a blocker for feat(hrr): ephemeral-path auto-disable for HRR persistence (#695) #701.

Holding attn:review until the four tests are updated. Releasing my review claim so the author / a sister session can rework.

[release:review:noether:2026-05-12T00:17:54Z]

[robotrocketscience]

[release:review:noether:2026-05-12T00:17:55Z]

[robotrocketscience]

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

[robotrocketscience]

Holding — pytest (3.13) red on Linux runner. 4 failures, all same root cause.

FAILED test_cache_persists_to_disk_after_build      — struct.npy not on disk
FAILED test_cache_load_uses_mmap                     — got ndarray, not memmap
FAILED test_cache_invalidate_removes_disk_blob       — persist dir never created
FAILED test_cache_load_failure_falls_through_to_rebuild — struct.npy missing

Captured log says it plainly:

WARNING aelfrice.hrr_index: HRR persistence disabled on ephemeral path
    /tmp/pytest-of-runner/pytest-0/test_cache_persists_to_disk_af0;
    set AELFRICE_HRR_PERSIST=1 to force.

Root cause: the new _EPHEMERAL_PATH_PREFIXES includes /tmp/, and pytest's tmp_path fixture on Linux runners is exactly /tmp/pytest-of-runner/.... The four pre-existing tests above predate this PR and assume persistence happens by default — they need monkeypatch.setenv("AELFRICE_HRR_PERSIST", "1") to opt back into persistence under the new auto-disable.

The macOS-quirk note in the body is correct as far as it goes — /tmp resolves to /private/tmp on macOS so the prefix never matches locally. That's why the test suite passed on the dev machine: it can't trip the predicate locally. On the Linux CI box the resolve is a no-op, so the predicate fires and every persist-relying test that uses tmp_path fails.

Fix shape (your call):

1. Test side — set AELFRICE_HRR_PERSIST=1 (or monkeypatch _EPHEMERAL_PATH_PREFIXES = frozenset()) at the top of every test that asserts persist-on-disk behavior. Cheapest fix; matches the spec intent that "Linux containers" auto-disable while explicit force-on persists.
2. Test fixture side — introduce a session-scoped autouse fixture that forces AELFRICE_HRR_PERSIST=1 for the whole test_hrr_struct_index.py module (since the whole module is about persist behavior), with the new auto-disable tests explicitly delenv'ing.

Either fix wants a sister consideration: this same blind spot bit PR #704 (test_resolve_persist_state_enabled fails for the identical reason — the standalone predicate copy in #704 also matches /tmp/). Land the fix here first; #704 inherits the resolved predicate path once the macOS-vs-Linux ambiguity is settled.

Other CI: staging-gate, codeql, calibration, deptry, vulture, sourcery, coderabbit — green. pytest 3.12 cancelled (cascade from 3.13 failure). Releasing review claim.

[robotrocketscience]

[release:review:noether:2026-05-12T00:27:30Z]

[robotrocketscience]

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

[robotrocketscience]

Request changes — CI regression

The new auto-disable triggers on tmp_path (rooted under /tmp/), so 4 pre-existing persistence tests fail under pytest (3.13):

• test_cache_persists_to_disk_after_build — struct.npy never written (warning visible in log).
• test_cache_load_uses_mmap — second get() rebuilds in-memory instead of mmap-loading.
• test_cache_invalidate_removes_disk_blob — .hrr_struct_index/ never created.
• test_cache_load_failure_falls_through_to_rebuild — same, then write_bytes(b"not a numpy file") hits FileNotFoundError on missing dir.

Root cause: the new tests in this PR work around the prefix check by monkey-patching _EPHEMERAL_PATH_PREFIXES (per the macOS quirk in the body), but the pre-existing persistence tests inherit tmp_path under /tmp/pytest-of-runner/... on Linux CI and aren't opted in to force-on. The auto-disable correctly short-circuits them — which is the regression.

Fix options (author's call)

1. Per-test opt-in — add monkeypatch.setenv("AELFRICE_HRR_PERSIST", "1") to each of the 4 affected pre-existing tests. Smallest diff, most explicit.
2. Module-level autouse fixture in tests/test_hrr_struct_index.py that force-ons AELFRICE_HRR_PERSIST=1 and uses monkeypatch.delenv(...) for the auto-disable tests that need the unset state. Cleaner if more persistence tests land later.
3. Monkeypatch _EPHEMERAL_PATH_PREFIXES to frozenset() for the pre-existing tests — symmetric with the macOS pattern already in this PR.

Option 1 is the most surgical; option 2 future-proofs. Either works.

Holding the claim release for now — re-ping attn:review after green pytest on 3.12 + 3.13. The product diff itself looks right (env precedence, one-shot WARN gate, trailing-slash guard against /tmpfoo/); the only blocker is the test regression.

[robotrocketscience]

[release:review:faraday:2026-05-12T00:33:30Z]

[robotrocketscience]

[claim:review:pascal:2026-05-12T00:37:14Z]

[robotrocketscience]

Reviewed by pascal session. Found + fixed a regression in this PR; rebased and pushed.

Regression

pytest (3.13) was failing on this PR because the new ephemeral auto-disable matches /tmp/ on Linux CI runners, where tmp_path resolves to /tmp/pytest-of-runner/.... Six pre-existing PR #693 persistence-substrate tests use tmp_path with delenv("AELFRICE_HRR_PERSIST") and the default-on assumption:

Hard failures (4):

• test_cache_persists_to_disk_after_build — asserts struct.npy exists; auto-disable returns None → no file written.
• test_cache_load_uses_mmap — same root cause; build path runs instead of mmap-load.
• test_cache_invalidate_removes_disk_blob — asserts persist dir exists post-build.
• test_cache_load_failure_falls_through_to_rebuild — depends on a prior write.

Silent vacuous passes (2 — bigger concern because they'd ship green):

• test_cache_loads_from_disk_on_second_construct — compared np.testing.assert_array_equal(a.struct, b.struct). Both builds were in-memory deterministic builds (same seed=7) so equality held without any persistence round-trip.
• test_cache_byte_equality_persist_round_trip — compared cold.probe(...) == warm.probe(...). Same vacuous-pass mechanism.

These two would have passed CI without actually exercising the persistence path. Worth catching now.

Fix

Added a single test commit test(hrr): force AELFRICE_HRR_PERSIST=1 in legacy persist tests on tmp_path — switches the six tests from delenv to setenv("AELFRICE_HRR_PERSIST", "1") so they keep exercising the substrate independently of the new ephemeral heuristic. The env-force-on path is the cleanest opt-out from auto-disable (it's already exercised by the new test_cache_env_force_on_overrides_ephemeral test).

Rebase

Branch was BEHIND main. Rebased feat/issue-695-hrr-ephemeral-path onto current github/main, force-pushed with --force-with-lease=feat/issue-695-hrr-ephemeral-path:8f55fe3f... (the SHA at fetch time). Post-rebase head: 5a6cde6. All 3 commits signed (G). FF-able on main.

Verifications

• tests/test_hrr_struct_index.py tests/test_hrr.py: 50 passed, 4 skipped (2 perf-gated, 2 Linux-only on /dev/shm + /run skipif macOS).
• Discretion grep on full diff vs main: clean.
• All commits SSH-signed.

Re-adding ready-to-merge after CI on the new SHA settles.

Design note (non-blocking)

The macOS quirk in the PR body is accurate — /tmp resolves to /private/tmp on macOS, so the auto-disable doesn't fire on dev machines. Worth mentioning in docs/CONFIG.md (probably part of the #553 docs bundle sub-issue) so operators on macOS aren't surprised when their /tmp-rooted bench runs persist to disk.

[robotrocketscience]

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

[robotrocketscience]

CI green on the rebased SHA — all 21 required checks pass (pytest (3.12), pytest (3.13), secrets/pattern/history scans, deptry, vulture, typos, CodeQL, label/size/commit-msg/pr-title/pr-body, release-docs-check). Merge gate met.

Filed #707 as a v3.0 rook follow-up to harden the two vacuous-pass assertions (test_cache_loads_from_disk_on_second_construct, test_cache_byte_equality_persist_round_trip) — the env-force fix in this PR routes them through persistence, but the equality checks themselves would still green-light a future silent-disable regression. Out of scope here; clean substrate change to land separately.

Adding ready-to-merge.

[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] f094cedfccfa44635e2bb7556f8ec518a2870c22 -> 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 22eb548 → main via FF push.