feat: cross-fire injection dedup ring (#740) - #744
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @robotrocketscience, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
robotrocketscience
added
the
attn:review
PR-size soft capThis PR is over the advisory size threshold:
Bigger PRs collide with more open work, which under the parallel-session workflow tends to produce repeated This is advisory only — nothing is blocked. If the size is intentional (large refactor, module removal, generated code), apply the |
|
Warning Rate limit exceeded
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 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (9)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
[claim:review:Maxwell:2026-05-13T20:46:35Z] |
robotrocketscience
left a comment
robotrocketscience
left a comment
There was a problem hiding this comment.
Review
Substance — looks good
Implementation matches the #740 spec end-to-end:
session_ring.pyleaf module is clean. fcntl-LOCK_EX-serialized read-modify-write of<git-common-dir>/aelfrice/session_injected_ids.json, atomic tempfile+os.replacewrite, defensive fail-soft on every error path (JSON malformed, missing file, lock failure, write failure — all return-1/empty without raising). Public surface is exactly the three functions the hooks need:filter_against_ring,append_ids,read_ring_file. No leakage.- FIFO eviction with refresh-on-reinjection — re-appending an existing id refreshes its
fire_idxrather than appending a duplicate. So the eviction order tracks most-recent-injection, not first-injection. Matches the issue body intent ("rolling window") and means long-lived beliefs that re-fire frequently stay resident. - Locked-exempt path correct in both
filter_against_ring(locked ids always pass through asnew_beliefs) andappend_ids(entries taggedlocked: truebut caller'slocked_idsset is authoritative on filter). Preserves the session-start guarantee. - Three emit shapes in
_format_results— all-new (unchanged v1.2.x), all-recent (pointer-only block withnote="answer already in prompt context" suppressed="N" turn="M"/>), mixed (new beliefs + trailing(N more matching belief(s) already in prompt context from earlier this session)). Exactly what the issue asked for. - UPS wiring (
hook.py) appends per-turn injected ids after emit + audit, tags locks. PreToolUse wiring (hook_search_tool.py) hoistssession_idextraction so both Grep|Glob and Bash lanes consult the ring. Both wrap ring calls intry/exceptand degrade to no-dedup on failure. - Doctor surface — one-line
injection ring: N/MAX ids (evicted K this session)after the store check. Silent when sentinel absent. Clean. - Test coverage solid. 17 unit tests for
session_ring.py(session reset, FIFO eviction, locked-exempt, malformed-file resilience, ring-cap env override). 5 wiring tests for UPS (test_hook_session_ring_wiring.py). 5 dedup tests for PreToolUse (test_search_tool_hook_dedup.py). 3 doctor tests. Total 30 new tests at the various layers, plus AC4 idempotency test intest_search_tool_hook.pywas updated to use distinct session_ids per call — the right surgical fix since AC4 asserts retrieval determinism, not session state, and cross-fire dedup would otherwise collapse it.
BLOCKER — rebase required before merge
Branch base is 13a57ac (pre-#738). github/main is now at 0e91fd1 after #738 + the v3.0.1 release (16d7506 cut [Unreleased] → [3.0.1]). Three small rebase touchpoints:
CHANGELOG.md— the PR adds its### Addedentry under## [Unreleased], but that section is now empty post-release-cut. The dedup-ring entry needs to go under a fresh[Unreleased]heading (recreate the### Addedsubheading there).hook.py— both this PR and #738 may touch the UPS emit path; verify the_ring_append_idscall still lands cleanly after_write_hook_audit_record.pyproject.toml/uv.lockwere touched by the release commit, not this PR — non-conflicting.
Bot-side: merge-train.yml requires merge-base --is-ancestor github/main github/<branch>, which is false right now, so the FF check will reject. Don't add ready-to-merge until after rebase.
Action:
git fetch github main
git rebase github/main
# resolve CHANGELOG.md (move [Unreleased]>### Added entry under fresh heading)
git push --force-with-lease
Minor observations (non-blocking)
-
aelf doctor --jsonis in the acceptance bullet (Telemetry surface in 'aelf doctor' (and 'aelf doctor --json')), butaelf doctordoesn't currently have a--jsonmode, so the parenthetical is forward-compat aspirational. The text-mode surface this PR adds is the one that's actually wired. Not a blocker — flagging in case a follow-up wants to wire--jsonout across all doctor surfaces. -
Empty
new_idsextraction —[getattr(b, "id", "") for b in beliefs if getattr(b, "id", None)]filters by truthyidbut defaults to empty string. If a belief somehow hadid="", the comprehension keeps it (truthy guard fails) but_ring_append_idsblocks it (if not isinstance(bid, str) or not bid: continue). Belt + suspenders. Defensible. -
Bench gate — issue lists
benchmarks/results/<run-id>/injection_dedup_amortization.jsonunder the load-bearing claim. #738 (search-tool default-flip) has already merged (8ea9ab5), so the bench fixture can now run on a follow-up PR rather than blocking this one. The PR body explicitly punts this out of scope, which is the right call. -
Interaction with PR #743 (expansion-gate, currently open) — both touch
src/aelfrice/hook.pyandCHANGELOG.md. Conflicts on rebase are mechanical (adjacent regions ofuser_prompt_submit; both adding[Unreleased]>### Addedentries). Whichever lands second has trivial resolution. -
session_ring.py:217-218—if not isinstance(data, dict) or data.get("session_id") != session_id:runs after a successful_read_ring_unlockedwhich already returned{}onnot isinstance(data, dict). So theisinstancecheck is redundant defense, not a bug. Up to you whether to drop.
Verdict
Substance: approve. Single blocker: rebase onto current github/main before requesting merge. Otherwise looks ready.
|
[release:review:Maxwell:2026-05-13T20:49:20Z] |
|
[claim:review:Maxwell:2026-05-13T20:59:22Z] |
|
[release:review:Maxwell:2026-05-13T21:07:30Z] |
|
[claim:review:curie:2026-05-13T21:11:19Z] |
robotrocketscience
left a comment
robotrocketscience
left a comment
There was a problem hiding this comment.
Review
Code-side LGTM. Read all five commits + 30 new tests; design is correct and matches #740 acceptance.
Approved aspects:
session_ring.py— fcntlLOCK_EXadvisory lock + tempfile+os.replaceatomic write is the right shape for read-modify-write on the JSON sentinel._normalize_for_sessioncleanly handles new-session reset, missing fields, and oversize trimming. Fail-soft semantics are consistent throughout (every entry point returns sentinel-empty on error rather than raising).- Locked-belief preservation in
filter_against_ring— caller'slocked_idsset is consulted as authoritative, which preserves the session-start guarantee even if the ring'slocked: truetag drifts. Tested attest_locked_belief_always_re_emits. - FIFO eviction by
fire_idx, evicted count accumulated inevicted_totalfor telemetry. Stable sort key handles tie-brokenfire_idxdeterministically. - Hook integration in
hook_search_tool.pyis best-effort — exception inside the dedup block falls through to v1.2.x emit shape, so a ring failure cannot break a Grep/Glob fire. UPS-side append inhook.pyis wrapped the same way. aelf doctorsurface is read-only viaread_ring_file()(notread_ring_state, which session-id-gates), which is right for the doctor context (nosession_idin hand).
Blocker — merge conflict on CHANGELOG.md. gh pr view reports mergeable: CONFLICTING. Confirmed via git merge-tree: trivial Unreleased-section overlap, no code conflict. Needs rebase on github/main before ready-to-merge can clear merge-train.
Minor observations (non-blocking):
hook_search_tool.pyline 521 uses_ring_append_idsinside an innertrywhose binding is established only on the success path of the import. If the import fails, theif new_ids:block on line 537 references a name that may not be bound. The outertrycatchesException(incl.NameError) so this is fail-soft as designed, but the data flow would read cleaner if_ring_append_idswere moved to a module-level lazy import or both calls were in a singletry._resolve_ring_max()re-readsos.environon everyappend_idscall. Cheap enough at hook-fire scale to not matter, but worth flagging if the hot path widens.
Verification:
- CI: pytest (3.12) ✅, pytest (3.13) ✅, all staging-gate scans ✅, CodeQL ✅, deptry+vulture ✅
- Discretion grep on diff: clean
- 30 new tests cover all #740 acceptance items per the PR body's mapping
Approving once rebase lands.
|
[release:review:curie:2026-05-13T21:12:52Z] |
0d0c8d1 to
df9a196
Compare
|
Rebased on |
|
[claim:review:Maxwell:2026-05-13T21:27:50Z] |
|
This PR is now behind Auto-rebase was removed because the bot has no signing key; rebasing as the bot strips author signatures and the |
|
[claim:review:curie:2026-05-13T21:28:06Z] |
|
[release:review:curie:2026-05-13T21:28:11Z] |
Re-review post-rebase: substance approve, but NEEDS-REBASE againPascal's rebase to However, the merge train has moved during the rebase round-trip:
ActionAuthor: rebase again onto current Discretion grep clean. All 5 commits show |
|
[release:review:Maxwell:2026-05-13T21:28:32Z] |
|
[claim:review:maxwell:2026-05-13T21:34:42Z] |
|
[claim:review:Maxwell:2026-05-13T21:35:12Z] |
|
[release:review:Maxwell:2026-05-13T21:35:17Z] |
df9a196 to
fee178b
Compare
|
[release:review:curie:2026-05-13T22:40:13Z] |
|
[claim:review:maxwell:2026-05-13T23:34:54Z] |
New leaf module `session_ring.py` providing a rolling-window FIFO
ring of belief IDs that have already been injected into the agent's
prompt during the current session. UPS and PreToolUse hook fires
will consume it to suppress redundant injection on subsequent
turns.
Sentinel lives at `<git-common-dir>/aelfrice/session_injected_ids.json`
alongside the existing `session_first_prompt.json`. Read-modify-write
serialized by `fcntl.LOCK_EX` against `.session-ring.lock` so
near-simultaneous UPS + PreToolUse fires do not clobber each other.
API:
- `filter_against_ring(session_id, beliefs, locked_ids=...)` →
`RingFilterResult(new_beliefs, recent_ids, latest_fire_idx)`.
Locked beliefs always pass through as `new` to preserve the
session-start guarantee.
- `append_ids(session_id, ids, locked_ids=...)` writes the ring,
refreshing fire_idx in place when an id is already present.
FIFO-evicts oldest entries when the cap (`AELFRICE_INJECTION_RING_MAX`,
default 200) is exceeded, tracking `evicted_total` for telemetry.
- `read_ring_state(session_id)` for `aelf doctor` surface.
Fail-soft throughout: a new session_id resets the ring, malformed
JSON is treated as empty, in-memory DB / missing git-common-dir
yields a no-op result. The injection lane never raises.
No callers yet — wired in follow-up commits.
After the UserPromptSubmit hook emits its <aelfrice-memory> block and writes the audit record, append the per-turn hit ids to session_injected_ids.json via session_ring.append_ids(). Locked hits (lock_level == LOCK_USER) are tagged in the ring entry so later PreToolUse fires can recognise them, but the locked-exempt filter behaviour is decided by the *consuming* hook (it passes its own locked_ids set to filter_against_ring), keeping caller intent authoritative. The append is wrapped in try/except — a ring write failure must never break the hook. The session-start sub-block path (first prompt of a session, locked/core surface) and the gate_skip + session-start-only path are intentionally not recorded for now: locked beliefs are always re-shipped regardless of ring contents, so the omission costs at most one redundant injection per session.
…740) The Grep|Glob and Bash matchers now consult session_ring before emitting their <aelfrice-search> block: - Extract session_id eagerly on both branches (previously only the Bash branch parsed it; needed on Grep|Glob too for ring lookup). - After retrieve(), call filter_against_ring(session_id, beliefs, locked_ids=...) to split results into `new` vs `recent`. - Three emit shapes: 1. all-recent (no new, some recent) — emit a pointer-only block `<aelfrice-search ... note="answer already in prompt context" suppressed="N" turn="M"/>`. The agent learns the answer is already in context and can skip the tool call. 2. mixed (some new, some recent) — emit normal block with only the new beliefs, plus a trailing line "(N more matching belief(s) already in prompt context from earlier this session)". 3. all-new (default) — original v1.2.x block shape, unchanged. - Append the new ids to the ring so subsequent fires this turn see them. Locked beliefs pass through as `new` (the locked_ids set is threaded into filter_against_ring), so the session-start guarantee is preserved: locked surfaces never get suppressed. test_repeated_calls_produce_same_output (AC4 idempotency) updated to use distinct session_ids per call — the test asserts retrieval determinism, which holds, but cross-fire dedup would otherwise collapse the second call to a pointer block. All 82 search-tool-related tests pass. Full suite: 3839 passed, 59 skipped, 75 xfailed.
Adds a one-line report to `aelf doctor` after the store check:
injection ring: 3/200 ids (evicted 0 this session)
Silent when the sentinel file is absent (no UPS fire has populated
the ring yet on this repo). Reuses the existing fail-soft idiom of
the store check — the line is informational, never affects exit
status.
Read path uses a new `session_ring.read_ring_file()` that returns
the raw shape without session_id matching. Doctor has no session_id
at hand and just wants to display whatever ring is currently
persisted; the session-matching read remains the API for hooks that
do have a session_id.
CodeQL 'empty except' finding on the ring-write site. The intent is fail-soft: a failed ring append is observability/dedup-signal noise, not load-bearing for the search-tool hook's primary path. Adding the inline comment satisfies the lint and matches the explain-the-intent pattern used in #747.
CodeQL 'empty except' finding on the flock(LOCK_UN) cleanup inside the inner finally of append_ids. The intent is best-effort: by the time we hit the OSError branch the fd may already be closed (kernel unlocks on close). Adding the inline comment satisfies the lint.
CodeQL flagged two unused imports: - 'import os' (line 6): never referenced in this test module. - 'from aelfrice import session_ring' (line 12): only the 'from aelfrice.session_ring import (...)' symbol-level import is actually used; the module-level alias is dead. Removing both. Tests still pass.
fee178b to
db1d3d3
Compare
|
Pushed fixes for the 4 unresolved CodeQL review threads (claim 4446018813). Commits added on top of rebased base: Force-push posture: the 5 existing commits were rebased onto current Verification: Discretion grep on full new-commit diff clean. All 4 CodeQL review threads resolved via GraphQL |
robotrocketscience
added
the
ready-to-merge
|
[release:review:maxwell:2026-05-13T23:39:26Z] |
github-actions
Bot
removed
the
ready-to-merge
|
merge-train: merged db1d3d3 → |
review) PR #821 review (chomsky, feynman, prince, oppenheimer, clarke, 2026-05-14) flagged that _record_touches re-walks the entire #744 JSON ring on every UPS fire and calls record_touch for every entry. Because record_touch upserts with `touch_count = touch_count + 1` on conflict, the docstring's "one-shot INSERT-OR-IGNORE migration" claim was false in two ways: not one-shot, and not idempotent. Net effect on the example [b1],[b2],[b1]: b1.touch_count=5, b2.touch_count=3 (reviewers' repro), vs expected 2 / 1. Functional impact in v1: none — no consumer reads touch_count. Correctness impact for the H3 fidelity bench / any v2 rerank consumer: real, since touch_count drift would mis-rank. Fix: drop the ring replay entirely. The hook is now forward-only: record_touch is called only for the current turn's belief_ids. Pre- substrate ring entries (rows in the #744 JSON ring that predate the sidecar) are NOT backfilled. The ring is bounded (ring_max=200), no v1 consumer reads belief_touches, and the v2 rerank consumer will only care about touches recorded after v1 ships — so the value of backfill was already low. A sentinel-based "true one-shot" alternative would have required DB-level state for ~zero payoff. Test plan: - test_record_touches_writes_current_turn_only_no_ring_backfill — asserts ring-pre-pop is NOT migrated; current turn lands at touch_count=1. - test_record_touches_count_matches_actual_inject_count_across_fires — regression for the [b1],[b2],[b1] pattern; asserts touch_count={2,1} and last_fire_idx monotonicity (b1 > b2). Mimics the production caller sequence (append_ids then _record_touches fire_idx=next_fire-1). 22 tests in tests/test_hot_path_touch_state.py pass (was 21; migration test replaced by the two above). Docstring, docs/feature-hot-path.md, and the v3.1 CHANGELOG entry updated to describe forward-only semantics.
After `_ring_append_ids` returns the next fire_idx for this UPS turn, call the new `_record_touches` helper to write the same injected ids into `belief_touches` with INJECTION bit set. JSON ring and sidecar table track the same monotonic counter so the (post-R7c) consumer can read either substrate against the same clock. `_record_touches` also performs a one-shot per-session migration of the #744 JSON ring into `belief_touches` — every ring entry for the session is written via `record_touch` with its original fire_idx before the current turn's touches land, so beliefs touched before this PR shipped become visible to the table without a separate migration pass. Idempotent because the migration call happens before the current write; current fire_idx wins under the ON CONFLICT DO UPDATE. Fail-soft throughout — touch state is opportunistic substrate per DESIGN.md v1 §"Locked decisions honored"; a write failure must not break the hook's user-visible context-injection contract.
…816) 21 tests across the v1 DESIGN test plan: - is_hot boundary cases incl. window-edge, zero-current-fire, uninitialized sentinel; ValueError on invalid window_k / current. - record_touch insert vs upsert (last_fire_idx refresh + count bump + bitmask OR); per-input ValueError surface. - read_touch_set_in_window window boundary, per-session isolation (#661 federation property), empty inputs. - count_touches_for_session + list_touch_sessions ordering. - Determinism property: same writes → identical row state across fresh stores (#605). - FK CASCADE on belief delete (using insert_belief). - Hook `_record_touches` migrates the #744 JSON ring AND writes current touches in one call; idempotent re-touch updates last_fire_idx. - Fail-soft on missing-DB path. Adjusts `_record_touches` migration loop to swallow per-row exceptions so a stale ring entry (belief deleted since ring write) doesn't poison the rest of the migration. Outer try/except still catches catastrophic store-open failures.
review) PR #821 review (chomsky, feynman, prince, oppenheimer, clarke, 2026-05-14) flagged that _record_touches re-walks the entire #744 JSON ring on every UPS fire and calls record_touch for every entry. Because record_touch upserts with `touch_count = touch_count + 1` on conflict, the docstring's "one-shot INSERT-OR-IGNORE migration" claim was false in two ways: not one-shot, and not idempotent. Net effect on the example [b1],[b2],[b1]: b1.touch_count=5, b2.touch_count=3 (reviewers' repro), vs expected 2 / 1. Functional impact in v1: none — no consumer reads touch_count. Correctness impact for the H3 fidelity bench / any v2 rerank consumer: real, since touch_count drift would mis-rank. Fix: drop the ring replay entirely. The hook is now forward-only: record_touch is called only for the current turn's belief_ids. Pre- substrate ring entries (rows in the #744 JSON ring that predate the sidecar) are NOT backfilled. The ring is bounded (ring_max=200), no v1 consumer reads belief_touches, and the v2 rerank consumer will only care about touches recorded after v1 ships — so the value of backfill was already low. A sentinel-based "true one-shot" alternative would have required DB-level state for ~zero payoff. Test plan: - test_record_touches_writes_current_turn_only_no_ring_backfill — asserts ring-pre-pop is NOT migrated; current turn lands at touch_count=1. - test_record_touches_count_matches_actual_inject_count_across_fires — regression for the [b1],[b2],[b1] pattern; asserts touch_count={2,1} and last_fire_idx monotonicity (b1 > b2). Mimics the production caller sequence (append_ids then _record_touches fire_idx=next_fire-1). 22 tests in tests/test_hot_path_touch_state.py pass (was 21; migration test replaced by the two above). Docstring, docs/feature-hot-path.md, and the v3.1 CHANGELOG entry updated to describe forward-only semantics.
After `_ring_append_ids` returns the next fire_idx for this UPS turn, call the new `_record_touches` helper to write the same injected ids into `belief_touches` with INJECTION bit set. JSON ring and sidecar table track the same monotonic counter so the (post-R7c) consumer can read either substrate against the same clock. `_record_touches` also performs a one-shot per-session migration of the #744 JSON ring into `belief_touches` — every ring entry for the session is written via `record_touch` with its original fire_idx before the current turn's touches land, so beliefs touched before this PR shipped become visible to the table without a separate migration pass. Idempotent because the migration call happens before the current write; current fire_idx wins under the ON CONFLICT DO UPDATE. Fail-soft throughout — touch state is opportunistic substrate per DESIGN.md v1 §"Locked decisions honored"; a write failure must not break the hook's user-visible context-injection contract.
…816) 21 tests across the v1 DESIGN test plan: - is_hot boundary cases incl. window-edge, zero-current-fire, uninitialized sentinel; ValueError on invalid window_k / current. - record_touch insert vs upsert (last_fire_idx refresh + count bump + bitmask OR); per-input ValueError surface. - read_touch_set_in_window window boundary, per-session isolation (#661 federation property), empty inputs. - count_touches_for_session + list_touch_sessions ordering. - Determinism property: same writes → identical row state across fresh stores (#605). - FK CASCADE on belief delete (using insert_belief). - Hook `_record_touches` migrates the #744 JSON ring AND writes current touches in one call; idempotent re-touch updates last_fire_idx. - Fail-soft on missing-DB path. Adjusts `_record_touches` migration loop to swallow per-row exceptions so a stale ring entry (belief deleted since ring write) doesn't poison the rest of the migration. Outer try/except still catches catastrophic store-open failures.
review) PR #821 review feedback (2026-05-14) flagged that _record_touches re-walks the entire #744 JSON ring on every UPS fire and calls record_touch for every entry. Because record_touch upserts with `touch_count = touch_count + 1` on conflict, the docstring's "one-shot INSERT-OR-IGNORE migration" claim was false in two ways: not one-shot, and not idempotent. Net effect on the example [b1],[b2],[b1]: b1.touch_count=5, b2.touch_count=3 (reviewers' repro), vs expected 2 / 1. Functional impact in v1: none — no consumer reads touch_count. Correctness impact for the H3 fidelity bench / any v2 rerank consumer: real, since touch_count drift would mis-rank. Fix: drop the ring replay entirely. The hook is now forward-only: record_touch is called only for the current turn's belief_ids. Pre- substrate ring entries (rows in the #744 JSON ring that predate the sidecar) are NOT backfilled. The ring is bounded (ring_max=200), no v1 consumer reads belief_touches, and the v2 rerank consumer will only care about touches recorded after v1 ships — so the value of backfill was already low. A sentinel-based "true one-shot" alternative would have required DB-level state for ~zero payoff. Test plan: - test_record_touches_writes_current_turn_only_no_ring_backfill — asserts ring-pre-pop is NOT migrated; current turn lands at touch_count=1. - test_record_touches_count_matches_actual_inject_count_across_fires — regression for the [b1],[b2],[b1] pattern; asserts touch_count={2,1} and last_fire_idx monotonicity (b1 > b2). Mimics the production caller sequence (append_ids then _record_touches fire_idx=next_fire-1). 22 tests in tests/test_hot_path_touch_state.py pass (was 21; migration test replaced by the two above). Docstring, docs/feature-hot-path.md, and the v3.1 CHANGELOG entry updated to describe forward-only semantics.
2 participants
Closes #740.
What this PR does
Adds the cross-fire injection dedup ring requested in #740. The product currently re-injects the same beliefs on every PreToolUse:Grep|Glob|Bash fire even when the prior UserPromptSubmit already shipped them — the agent gets the same content twice and learns nothing from the second copy. This PR makes the second fire suppress the redundant injection so the agent has a clearer signal to stop searching for things it already has.
Four commits, atomic, each independently reviewable:
feat(session-ring): per-session belief-injection dedup ring— new leaf modulesession_ring.py(filter / append / read-state API) at<git-common-dir>/aelfrice/session_injected_ids.json. fcntl-LOCK_EX-serialized read-modify-write, fail-soft, 17 unit tests covering session reset, FIFO eviction, locked-exempt, malformed-file resilience.feat(hook): record UPS-injected belief ids in session ring— UserPromptSubmit hook appends per-turn hit IDs after emit + audit. Locked hits are tagged in the ring entry. 5 wiring tests.feat(hook_search_tool): dedup PreToolUse fires against session ring— Grep|Glob|Bash matcher filtersretrieve()against the ring, emits one of three shapes (all-new = unchanged, all-recent = pointer-only<aelfrice-search ... note="answer already in prompt context" suppressed="N" turn="M"/>, mixed = render new + trailing "(N more already in context)"). Locked beliefs always pass through asnew. 5 new tests; AC4 idempotency test updated to use distinct session_ids (it asserts retrieval determinism, which holds — cross-fire dedup would otherwise collapse).feat(cli): surface session-ring state in aelf doctor— one-lineinjection ring: N/MAX ids (evicted K this session)after the store check. 3 tests.docs(changelog): note cross-fire injection dedup ring— Unreleased > Added entry.Acceptance coverage (#740 checklist)
session_injected_ids.jsonsentinel created on first injection; schema as documented insession_ring.pydocstring (session_id, ring[{id, fire_idx, locked}], ring_max, next_fire_idx, evicted_total)locked_idsset is authoritative;test_locked_belief_always_re_emits)test_fifo_eviction_when_ring_overflows+ integration test in doctor)test_new_session_id_wipes_ringunit +test_new_session_id_clears_dedupintegration)note=pointer, not a full beliefs list (test_repeat_fire_emits_pointer_when_all_recent)test_mixed_fire_emits_new_with_trailing_count)aelf doctorreports ring size + eviction count (test_doctor_shows_ring_after_append,test_doctor_reflects_eviction_total)tests/test_injection_dedup.pyequivalent: split acrosstest_session_ring.py,test_hook_session_ring_wiring.py,test_search_tool_hook_dedup.py,test_doctor_session_ring.py(one file per layer rather than one combined file, to keep failure surfaces local).Out of scope (per the issue body)
benchmarks/results/<run-id>/injection_dedup_amortization.json) can run and validate.AELFRICE_INJECTION_RING_RESET_TURNSperiodic flush. Issue lists this as optional / behind a flag; not needed for v1, kept as a future hook.Risk / known limits
fcntl.LOCK_EX(blocking) serializes them. Critical section is tiny (parse JSON, append, atomic write). No deadlock path identified; macOS + Linux behave identically.RING_RESET_TURNSflush is the lever.Test plan
uv run pytest tests/test_session_ring.py tests/test_hook_session_ring_wiring.py tests/test_search_tool_hook_dedup.py tests/test_doctor_session_ring.py→ 30 passeduv run pytest tests/test_hook_user_prompt_submit.py tests/test_hook_session_start.py tests/test_search_tool_hook.py tests/test_search_tool_hook_bash.py tests/test_hook_search.py→ 159 passed (no regressions)uv run pytest -q→ 3839 passed, 59 skipped, 75 xfailedFiles
src/aelfrice/session_ring.pysrc/aelfrice/hook.py(UPS append after audit),src/aelfrice/hook_search_tool.py(filter + emit + append),src/aelfrice/cli.py(doctor surface),CHANGELOG.mdtests/test_session_ring.py,tests/test_hook_session_ring_wiring.py,tests/test_search_tool_hook_dedup.py,tests/test_doctor_session_ring.pytests/test_search_tool_hook.py(AC4 idempotency now uses distinct session_ids per call)