feat(hook): an off-switch for the injected memory block, and a line that names it (#1359) - #1465
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
|
Warning Review limit reached
Next review available in: 7 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe PR adds an environment and TOML switch for memory-block injection. Disabled output preserves retrieval and cadence processing while suppressing exposure records. The memory hint, configuration documentation, tests, and ChangesMemory block switch
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant UserPromptSubmit
participant ConfigResolver
participant Retrieval
participant SessionRing
participant PromptOutput
UserPromptSubmit->>ConfigResolver: resolve memory-block setting
UserPromptSubmit->>Retrieval: retrieve hits with exposure mode
Retrieval-->>UserPromptSubmit: return retrieval results
UserPromptSubmit->>SessionRing: advance cadence counter
alt memory block enabled
UserPromptSubmit->>PromptOutput: emit memory block and hint
else memory block disabled
UserPromptSubmit->>PromptOutput: suppress memory block
end
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
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 |
Reviewer's GuideAdds a configurable off-switch for the UserPromptSubmit Sequence diagram for UserPromptSubmit with memory block switchsequenceDiagram
actor User
participant UserPromptSubmit as user_prompt_submit
participant MemoryStore as ups_store
participant HookSearch as search_for_prompt
participant Writers as exposure_writers
participant Prompt as stdout
User ->> UserPromptSubmit: submit prompt
UserPromptSubmit ->> UserPromptSubmit: load_user_prompt_submit_config(start)
UserPromptSubmit ->> UserPromptSubmit: emit_memory_block = memory_block_enabled(start)
UserPromptSubmit ->> MemoryStore: _retrieve(prompt, budget, store, record_exposure=emit_memory_block)
MemoryStore ->> HookSearch: search_for_prompt(store, prompt, record_exposure)
HookSearch ->> HookSearch: record_retrieval(...)
HookSearch -->> MemoryStore: hits
MemoryStore -->> UserPromptSubmit: hits
alt emit_memory_block == True
UserPromptSubmit ->> Writers: _substitute_exploration_slots(...)
UserPromptSubmit ->> Writers: _record_injection_events(...)
UserPromptSubmit ->> Writers: _ring_append_ids(injected_ids)
UserPromptSubmit ->> Writers: _record_touches(...)
UserPromptSubmit ->> Prompt: <aelfrice-memory> block + coverage + MEMORY_BLOCK_HINT
else emit_memory_block == False
UserPromptSubmit -->> Writers: skip _substitute_exploration_slots
UserPromptSubmit -->> Writers: skip _record_injection_events
UserPromptSubmit ->> Writers: _ring_append_ids([]) %% advance next_fire_idx only
UserPromptSubmit -->> Writers: skip _record_touches
UserPromptSubmit ->> Prompt: MEMORY_BLOCK_HINT only
end
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/aelfrice/doctor.py (1)
791-792: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winDo not silently catch every resolution error.
_diagnose_memory_block()converts both lazy import failures and unexpected failures frommemory_block_enabled()intoNone._format_memory_block_section()then skips the row entirely, soaelf doctorcan hide a resolver regression while still reporting the rest of the diagnosis. Catch only expected fail-soft exceptions, or log unexpected exceptions before returningNone.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/aelfrice/doctor.py` around lines 791 - 792, Update _diagnose_memory_block() so it does not silently swallow all exceptions from lazy imports or memory_block_enabled(); catch only the expected fail-soft exception types, or log unexpected failures before returning None. Preserve _format_memory_block_section()’s handling of genuine unavailable-memory-block results while ensuring resolver regressions remain visible.Source: Linters/SAST tools
docs/user/CONFIG.md (1)
18-18: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueShorten the overview bullet and keep the detail in the schema section.
Every other bullet in this list is one to three sentences. This bullet is roughly 700 words and repeats almost all of the
[memory_block]schema comment at Lines 290-312. The "What it does" list is the scan surface, so a reader looking for the key name and the env var must read the full paragraph first.Keep the default, the two spellings, the precedence rule, and one sentence on scope here. Leave the suppression inventory and the
--cold-forand mid-session-flip consequences in the schema section, which already states them.The content is accurate. Each claim checks out against
memory_block_enabledand the gated write sites insrc/aelfrice/hook.py.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/user/CONFIG.md` at line 18, Shorten the [memory_block] overview bullet to the key behavior only: retain the default enabled state, both configuration spellings, environment-variable precedence, and one sentence stating that disabling suppresses the injected <aelfrice-memory> envelope. Remove the detailed suppression inventory and --cold-for or mid-session-flip consequences, leaving those details in the existing schema section.src/aelfrice/hook.py (1)
1442-1470: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueBind
injected_idsbefore thetryblock.The
exceptat Line 1455 sets_next_fire = -1but leavesinjected_idsunbound if the comprehension at Line 1443 raises. Line 1470 then readsinjected_ids. The read is unreachable today becauseandshort-circuits on_next_fire >= 1, but the safety depends on the operand order rather than on the binding.This module already states the opposite convention for the same failure class. See the comment at Lines 1151-1159, which binds
retrieval_queryon every path so a control-flow analyser does not reach an unbound name.♻️ Proposed binding
+ injected_ids: list[str] = [] try: injected_ids = [ h.id for h in hits if getattr(h, "id", None) ]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/aelfrice/hook.py` around lines 1442 - 1470, Initialize injected_ids before the try block that builds it, using the same safe default expected when hit extraction fails. Keep the successful comprehension unchanged, and ensure the later belief_touches condition can reference injected_ids on every exception path without relying on short-circuit operand order.tests/test_hook_memory_block_switch_1359.py (1)
261-275: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the malformed-TOML and unreadable-file branches.
memory_block_enabledhas four fail-soft branches that all returnTrue. This test covers the wrong-typed value atsrc/aelfrice/hook.pyLines 518-524. Three remain uncovered:
OSErroronread_bytes, Lines 502-504.tomllib.TOMLDecodeError, Lines 509-511.- A non-dict
[memory_block]value, Lines 513-514.The malformed-TOML branch is the most reachable of the three, because a user hand-edits this file. A regression that raised instead of degrading would reach the hook's outer handler and suppress the block silently, which inverts the intended default.
💚 Proposed additional cases
def test_no_config_and_no_env_is_enabled(tmp_path: Path) -> None: assert memory_block_enabled(start=tmp_path, env={}) is True + + +def test_malformed_toml_degrades_to_enabled(tmp_path: Path) -> None: + """A hand-edit that breaks the file must not silently disable the block.""" + (tmp_path / ".aelfrice.toml").write_text( + "[memory_block\nenabled = false\n", encoding="utf-8", + ) + serr = io.StringIO() + assert memory_block_enabled(start=tmp_path, env={}, stderr=serr) is True + assert "malformed TOML" in serr.getvalue() + + +def test_non_dict_section_degrades_to_enabled(tmp_path: Path) -> None: + (tmp_path / ".aelfrice.toml").write_text( + 'memory_block = "on"\n', encoding="utf-8", + ) + assert memory_block_enabled(start=tmp_path, env={}) is True🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_hook_memory_block_switch_1359.py` around lines 261 - 275, Add test coverage for the remaining fail-soft branches in memory_block_enabled: simulate an OSError while reading the configuration file, provide malformed TOML that raises tomllib.TOMLDecodeError, and provide a valid TOML file where [memory_block] is not a dictionary. Assert each case returns True and verifies the expected diagnostic output where applicable, while preserving the existing wrong-type and no-config coverage.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_hook_memory_block_switch_1359.py`:
- Around line 97-109: Update the test helper _run to capture and return stderr
alongside stdout, then adjust test_env_zero_suppresses_the_block and
test_toml_false_suppresses_the_block to assert the returned stderr does not
contain “Traceback”. Preserve the existing return-code and stdout assertions
while ensuring swallowed hook failures cannot make these suppression tests pass.
---
Nitpick comments:
In `@docs/user/CONFIG.md`:
- Line 18: Shorten the [memory_block] overview bullet to the key behavior only:
retain the default enabled state, both configuration spellings,
environment-variable precedence, and one sentence stating that disabling
suppresses the injected <aelfrice-memory> envelope. Remove the detailed
suppression inventory and --cold-for or mid-session-flip consequences, leaving
those details in the existing schema section.
In `@src/aelfrice/doctor.py`:
- Around line 791-792: Update _diagnose_memory_block() so it does not silently
swallow all exceptions from lazy imports or memory_block_enabled(); catch only
the expected fail-soft exception types, or log unexpected failures before
returning None. Preserve _format_memory_block_section()’s handling of genuine
unavailable-memory-block results while ensuring resolver regressions remain
visible.
In `@src/aelfrice/hook.py`:
- Around line 1442-1470: Initialize injected_ids before the try block that
builds it, using the same safe default expected when hit extraction fails. Keep
the successful comprehension unchanged, and ensure the later belief_touches
condition can reference injected_ids on every exception path without relying on
short-circuit operand order.
In `@tests/test_hook_memory_block_switch_1359.py`:
- Around line 261-275: Add test coverage for the remaining fail-soft branches in
memory_block_enabled: simulate an OSError while reading the configuration file,
provide malformed TOML that raises tomllib.TOMLDecodeError, and provide a valid
TOML file where [memory_block] is not a dictionary. Assert each case returns
True and verifies the expected diagnostic output where applicable, while
preserving the existing wrong-type and no-config coverage.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 76942649-9dde-4047-bcaf-c898cc81cdcf
📒 Files selected for processing (7)
CHANGELOG/v4.mddocs/user/CONFIG.mdsrc/aelfrice/doctor.pysrc/aelfrice/hook.pysrc/aelfrice/hook_search.pytests/test_hook_memory_block_switch_1359.pytests/test_hook_search.py
|
Both review threads addressed in CodeQL ( CodeRabbit (assert stderr carries no traceback) — taken, and it is a better suggestion than it looks. The hook fails soft: an exception on any of the four writers the suppressed path touches becomes a non-fatal stderr line with 24 passed. |
|
[claim:review:Setr:2026-08-10T17:30:46Z] |
|
[claim:review:Garsecg:2026-08-10T17:37:02Z] |
|
[release:review:Garsecg:2026-08-10T17:37:07Z] |
|
[claim:review:idnn:2026-08-10T17:38:20Z] |
|
[release:review:idnn:2026-08-10T17:38:25Z] |
Review — the mechanics are right; the switch's headline guarantee is defeated one turn laterReviewed adversarially. Unusually well tested — every arm carries an enabled control, the shipped 97-char hint is pinned by value separately from the mechanism, and the On #1461: the What survived refutation is one missed writer, and it defeats exactly the guarantee the switch is sold on.
Reproduced end-to-end against the PR head on a one-belief store with
The eviction is permanent and source-agnostic: Same root cause, second symptom: DispositionOne schema addition fixes both: stamp an explicit Do not blank
|
robotrocketscience
added
attn:unblock
#1359. AELFRICE_MEMORY_BLOCK, or [memory_block] enabled in .aelfrice.toml, stops UserPromptSubmit writing the <aelfrice-memory> block to stdout. Env over TOML in both directions, tri-state like the AELFRICE_BFS convention, default on. The gap between aelf scope-out and aelf uninstall had nothing in it. Retrieval, the correction lane, the relevance sweeper, aelf rebuild and the SessionStart baseline all keep running; a suppressed fire records an empty rendered_block so the audit log's token count does not claim an injection that never happened.
#1359. One line appended after the block, built and placed exactly like the shipped #857 coverage line: outside </aelfrice-memory>, so the block's own bytes and the audit accounting that splits on the framing tags are unchanged. Unconditional, unlike the coverage line, because the point is to reach a user who has never read the docs. Measured cost: 97 characters / 99 UTF-8 bytes / 25 estimated tokens per emitting fire, ~1% of the 2400-token default retrieval budget.
#1359. A Memory block section rendering injection: enabled|disabled, following the existing HRR / persist_enabled row. The disabled line names both spellings of the switch, since a user reaching doctor to ask why nothing is injected needs the answer there. aelfrice.hook is imported lazily -- it pulls the retrieval stack, which doctor otherwise never pays for.
Drives UserPromptSubmit end to end on both states: disabled writes nothing while the correction lane and the relevance sweeper still fire, enabled writes today's bytes plus exactly the hint, verified by stripping the suffix and asserting the remainder still ends at </aelfrice-memory>. Env-beats-TOML is asserted in both directions and through the live hook, not just the resolver. The hint's 97 chars / 99 bytes / 25 tokens are pinned as literals: it is spent on every block-emitting turn, so growth must be deliberate.
… var Both the section list and the schema block in docs/user/CONFIG.md, plus the Unreleased CHANGELOG entry. States what the switch does NOT stop -- retrieval, the correction and relevance lanes, hook_audit.jsonl, aelf rebuild, the SessionStart baseline -- because 'turn off memory' is the reading it would otherwise get.
The #1359 off-switch blanked stdout but left `_record_injection_events`, the session ring and `_record_touches` firing, so a suppressed turn claimed the model had seen beliefs whose block never reached the prompt. The #779 Layer-3 sweeper resolves pending injection events against the next assistant transcript, which cannot reference text that was never injected, so every one of those rows would have resolved referenced=0 — negative evidence manufactured by an off switch. Gate all three writes on the same `emit_memory_block` answer. The hook audit still records the fire, with tokens 0 and beliefs[] intact: it is the record of what the hook did, not a claim about exposure.
Nothing splits the rendered block on its framing tags — `_write_hook_audit_record` takes `tokens` from `_audit_tokens_from_block(rendered_block)` over the whole string — so the hint is inside the audited count, not outside it. Measured on this branch across four one-belief fires: +24 audited tokens per emitting fire, +25 when the pre-hint block length is a multiple of 4; a seeded three-belief fire audits at 213 with the hint against 188 without. And the ratio was against the wrong constant: the UPS hook passes DEFAULT_HOOK_TOKEN_BUDGET = 1500 as an explicit kwarg, which `resolve_token_budget` ranks above the 2400 CLI default, so 25 tokens is 1.7%, not ~1%.
"Nothing else stops" was wrong. The #578 first-prompt session-start sub-block and #871's <cadence-resume> are embedded by `_format_hits_with_session_start`, so they ride inside the same envelope and go with it; #870's in-session <cadence-checkpoint> is written outside it and does not. Both docs now say so. Also: the switch suppresses the exposure-evidence writes, the 1.7%/1500 correction, the audited-token rise (+24, +25 when the pre-hint block length is a multiple of 4) that obliges #1382 to re-take its baseline, and the narrowing of "every emitted block" to the two UPS <aelfrice-memory> emit paths — the SessionStart <aelfrice-baseline> block does not carry the hint.
The `elif gate_skip:` site is the hook's second <aelfrice-memory> emit path and had zero coverage: reverting it to pre-#1359 left all 14 tests green. Two tests now drive a shape-gated prompt ("ok", below the 12-char `_MIN_PROMPT_LEN`) on a session's first prompt in both switch states, pinning the branch via the audit record's `prompt_shape_gate_skip` field. A third pins that a suppressed fire writes no injection_events row, no belief_touches row and no session-ring entry, with the enabled fire as an in-test control so the zeroes cannot come from a dead fixture. A fourth pins that the hint is inside the audited token count and by how much.
`_write_telemetry` was the fourth writer on the suppressed path and the only one still unguarded: `total_chars` stayed the sum of the retrieved hits' content lengths, and that field is what `aelf doctor` renders as `injection size p50/p95: N chars`. One report could therefore print an injection size beside the `Memory block / injection: disabled` line this branch added to answer "is this thing on?". Same treatment as the hook_audit carve-out: the fire is still recorded — n_returned, n_l0 and n_l1 keep saying what retrieval found — but the injected-size field is zeroed, because nothing was injected. The test drives both switch states end to end and asserts the rendered doctor report, not just the telemetry record.
The MEMORY_BLOCK_HINT docstring and the CHANGELOG entry both quoted "a seeded three-belief fire audits at 213 with the hint against 188 without". No seed and no script ship with it, and an equivalent three-belief fire gives a different pair, so the number is not reproducible from anything in the tree. The load-bearing claim beside it already is exact: for a pre-hint block of L characters the delta is ceil((L+97)/4) - ceil(L/4), which is 25 iff L % 4 == 0 and 24 otherwise. The decorative pair is replaced by that rule in both places, and a new test sweeps it over every L in [0, 4000) against the audited estimator itself so the published statement has an in-tree derivation.
`search_for_prompt` writes one `feedback_history` row per hit tagged `source='hook'`, and `EXPOSURE_ONLY_FEEDBACK_SOURCES` is exactly that set — the row IS this codebase's exposure record, and it was still written when the #1359 switch suppressed the block. Its live consumer is `store.exploration_pool` (#1176), which draws from beliefs with no `feedback_history` and no `injection_events` row: measured on a fresh store, `exploration_pool('banana')` returned ['F1'] before a suppressed fire and [] after it, so gating injection_events alone did not save it and the belief was evicted from the never-shown pool having never been shown. Under AELFRICE_EXPOSURE_UPDATES_POSTERIOR=1 the same fire moved alpha 1.0 -> 1.1. Plumb the switch through `_retrieve` into `search_for_prompt` as `record_exposure`, defaulting True so no other caller changes. Retrieval still runs — the correction and relevance lanes read its hits — and skipping the whole `record_retrieval` call keeps the audit row and the `last_retrieved_at` mirror in agreement.
Three tests, each with an enabled-fire control in the same test so a
zero cannot come from a dead fixture. Two drive the hook end-to-end: a
suppressed fire writes no `feedback_history` row, leaves
`last_retrieved_at` NULL and leaves the belief in
`exploration_pool('banana')`, where the enabled fire empties it; and
under AELFRICE_EXPOSURE_UPDATES_POSTERIOR=1 a suppressed fire leaves
alpha at 1.0 where the enabled fire moves it to 1.1. The third pins the
`search_for_prompt` contract directly — `record_exposure` defaults True,
and False still returns the hits, so the lanes downstream of the switch
keep their input. Mutation-checked: dropping the kwarg at the hook call
site fails both hook tests, dropping the conditional in
`search_for_prompt` fails the unit test.
CHANGELOG and CONFIG.md both said a suppressed fire records no exposure evidence and enumerated three writers. A fourth existed: the `source='hook'` feedback_history row `search_for_prompt` writes per hit, which `EXPOSURE_ONLY_FEEDBACK_SOURCES` defines as the exposure record and `store.exploration_pool` reads to find never-shown beliefs. It is guarded now, so the claim is true — but the enumeration has to say so, and the exploration-pool consequence is the reason the guard is worth its plumbing. Also records that retrieval still runs and that the call is skipped whole, keeping the audit row and the `last_retrieved_at` mirror in agreement.
The list named the <cadence-checkpoint> case but omitted two other aelfrice-authored blocks that also reach stdout outside the envelope with the switch off — <aelfrice-phantom-opportunity> (#980) and <aelfrice-phantom-promotion-opportunity> (#1132 Q2), both default-off, verified by driving a zero-hit prompt with AELFRICE_MEMORY_BLOCK=0 and [phantom_generation] enabled = true and watching the note land on an otherwise empty stdout. It also omitted the UPS telemetry JSONL, which still gets a row per suppressed fire; with no `suppressed` field, that row is only legible as a pair (n_returned intact, total_chars 0), so the doc says to read them together.
…ance `session_ring.append_ids` does two jobs: it records this fire's injected-id dedup set, and it bumps `next_fire_idx`. Guarding the whole call for #1359 suppressed both, and the counter is the input `cadence.should_fire` / `would_fire_p1` and the UPS `p3_velocity` branch read — each requires it to have advanced. So with the block off the in-session `<cadence-checkpoint>` this switch documents as surviving could never fire under `p1_every_k_turns` or `p3_velocity`; measured, five suppressed fires left the ring file uncreated and `next_fire_idx` absent, against 1,2,3,4,5 with the block on. The ids stay suppressed — they are an injection record, and honouring them would make the next PreToolUse fire dedup against beliefs the model never saw — but the call now runs on both paths with an empty list, which persists the bump and records nothing. `belief_touches` is exposure credit and keeps its own guard, now the only thing holding that row off a suppressed fire.
`_substitute_exploration_slots` (#1279) runs upstream of every #1359 guard and takes two writes: it claims the store-level exploration fire counter and writes an `exploration_events` row naming the belief drawn and the ones displaced to pay for it. On a suppressed fire that row says a never-injected belief was substituted into a pack that never reached the prompt — the state the function's own docstring calls pointless, since evidence accrues on exposure — and it pollutes the coverage instrument the lane exists to produce. Measured with the lane forced on, the suppressed fire wrote a row with the same drawn/displaced payload as the enabled control while `injection_events` stayed 0. Guarded at the call site like the other injection writers rather than plumbed through, because the substitution has no purpose when nothing is emitted.
Five gaps between the published claim and the tree. The "what keeps running" list named `aelf rebuild`, the CLI, but not the per-turn `rebuild_logs/<session-id>.jsonl` row a suppressed fire still writes. "Records no exposure evidence" was stated flat, and two retrieval- exposure records sit outside the switch by design: with the opt-in `[implicit_feedback] enqueue_on_retrieve = true`, `retrieve()` still enqueues one `deferred_feedback_queue` row per hit (inert since #1162 made the sweeper audit-only, but a record that a belief was retrieved), and the PreToolUse agent-context lane writes its own rows for its own envelope. The `last_retrieved_at` stamp shares the suppressed row's transaction per the #1373 invariant, so with the block off permanently `aelf stale --cold-for` reads those beliefs as never retrieved — the right trade, now stated rather than implied. `is_session_first_prompt` consumes the session's first-prompt slot before the switch is resolved, correctly, so re-enabling mid-session never restores the #578 sub-block for that session. And both surfaces now describe the ring split and the exploration ledger the two preceding commits changed.
The #1359 comment on the session-ring append cited a `_maybe_run_cadence_checkpoint` that does not exist and counted three consumers where there are two dispatchers. The counter feeds `_maybe_run_ups_cadence_checkpoint`'s P1 and `p3_velocity` branches and `_maybe_fire_cadence_checkpoint` on the Stop side, all of which reach `cadence.would_fire_p1`'s positive-index condition. Comment only.
…switch leaves behind The exploration-pool and posterior figures were measured against the unfixed code and read as if they described what ships; they now say so and carry the after-state. CONFIG.md's what-keeps-running list gains session_injected_ids.json and its lock, which a suppressed fire creates once the ring's two jobs are split.
…acebacks CodeQL flagged aelfrice.hook imported both ways; the function-local re-import was the other half. The hook fails soft, so an exception on any of the four suppressed-path writers becomes a non-fatal stderr line with rc still 0 — stdout alone cannot tell that from suppression.
The suppressed retrieval fire's hook_audit row was unpinned: dropping `body = ""` and suppressing only the stdout write left every test in the file green while `aelf tail` reported a 699-char / 175-token block for a fire that injected nothing — falsifying the shipped CONFIG.md claim of `tokens: 0`. The gate-skip path already pinned this; the retrieval path, the one carrying beliefs, did not. Both halves run, so the zero means suppression and not a dead fixture, and `beliefs[]` is asserted non-empty on both to keep the audit the record of the fire.
Every doctor assertion in the file passed a non-existent user_settings, so all three only exercised format_report's no-settings.json early return. Deleting the _format_memory_block_section call from the main body — the branch every install with a settings.json takes — left 87 tests green across this file and the four doctor files. The new test takes the scanned path and asserts the early-return sentinel is absent, so it cannot pass by silently falling back to the pinned branch.
The disclosed consequence of a permanently suppressed last_retrieved_at stamp named only `aelf stale --cold-for`, which reports. `aelf review` acts: list_review_candidates orders last_retrieved_at NULLS FIRST, so those beliefs lead the weekly keep/remove/lock checkpoint, and _cold_days falls back to creation age when both stamps are NULL — a 70-day-old belief retrieved yesterday renders `70d cold`, measured here against a two-belief store. Named explicitly rather than left inside "every other recency consumer".
The hint docstring cited `_CHARS_PER_TOKEN = 4` as "the project's convention"; hook.py has `_CORE_CHARS_PER_TOKEN`, and the count the 25 figure comes from is `_audit_tokens_from_block`'s own 4.0. Value unchanged, symbol corrected. The doctor field comment claimed a silent absence is not an answer while the formatter returns silently on None, so it now says what None means; `_diagnose_memory_block` discloses that its env half reads doctor's process, not the hook's settings.json env.
f7d3b4a to
a531880
Compare
robotrocketscience
added
the
ready-to-merge
github-actions
Bot
removed
the
ready-to-merge
|
merge-train: merged a531880 → |
2 participants
Closes #1359.
The ratified scope (2026-08-06 ruling 7) was the cheap half: an off-switch plus a line on the block that names it. The in-UI minimize/expand half stays struck — hook stdout is rendered by the host and there is no collapse-state API to call.
What ships
AELFRICE_MEMORY_BLOCK=0, or[memory_block] enabled = falsein.aelfrice.toml, with the env var winning in both directions.aelf doctorgrows aMemory block: injection enabled|disabledrow following the existingHRR / persist_enabledprecedent, so "is this thing on?" is answerable without reading a config file.Both
<aelfrice-memory>emit paths — the retrieval path and the shape-gate-skip path that carries only the session-start sub-block — carry one appended line namingaelf tailand the switch, built and placed exactly like the shipped #857 coverage line, outside</aelfrice-memory>so the beliefs the model reads are unchanged. Cost: 97 chars / 99 UTF-8 bytes / 25 estimated tokens per emitting fire — 1.7% ofDEFAULT_HOOK_TOKEN_BUDGET = 1500, which is what the UPS hook passes, not the 2400 CLI default (resolve_token_budgetranks an explicit caller kwarg above it).The line is inside the audited block, so
hook_audit.jsonltokensrises by +24 per emitting fire, +25 when the pre-hint block length is a multiple of 4. That is the exact rule, not a sampled pair:ceil((L+97)/4) - ceil(L/4)is 25 iffL % 4 == 0, swept over everyLin[0, 4000)by a test. Correct — the audit records what was injected and this line is injected — but it moves exactly the per-turn injected-token baseline #1382 is funded against, so that baseline must be re-taken after this lands.The expensive part was finding every writer
An off-switch is only honest if nothing downstream still claims the model saw the beliefs. Reading call sites found four writers across four review rounds; a SQL
trace_callbackon every connection matchingINSERT|UPDATE|DELETE|REPLACE, plus a filesystem diff of the temp tree, run once with the switch on and once off, found all 17 in one pass. Guarded:injection_events,belief_touches, the injected-id ring entry,exploration_events(#1279's slot), and thesource='hook'feedback_historyrow.The
feedback_historyrow is the one with a permanent consequence.models.EXPOSURE_ONLY_FEEDBACK_SOURCESis exactly{'hook'}, so that row is this codebase's exposure record, andstore.exploration_pool(#1176) selects beliefs with nofeedback_historyand noinjection_eventsrow — "never been shown". Against the unfixed code, one suppressed fire tookexploration_pool('banana')from['F1']to[]: gatinginjection_eventsalone did not save it, and the belief left the never-shown pool having never been shown. It needed the switch plumbed through_retrieveintohook_search.search_for_prompt(record_exposure, defaultTrue, so no other caller moves).A guard falsified this branch's own claim, and that is why the ring is split.
session_ring.append_idsdoes two jobs — records the fire's injected-id dedup set, and bumpsnext_fire_idx. Guarding the whole call suppressed both, andnext_fire_idxis whatshould_fire/would_fire_p1and thep3_velocitybranch read. So<cadence-checkpoint>— which the entry claims survives suppression — could never fire underp1_every_k_turnsorp3_velocitywhile the block was off. The counter counts fires, and a suppressed fire is still a fire, so it now advances on both paths while the id list stays behind the switch. Measured across five fires:next_fire_idx[1,2,3,4,5]with the block off andring == [], against[1,2,3,4,5]andring == ['F1']with it on.Prices paid, stated rather than buried
last_retrieved_atgoes with the exposure row. They share a transaction and fix(ingest): make the ingest write group atomic and single-clocked (#1157 §3/§7) #1373 says they agree, so with the block off permanently those beliefs read as never-retrieved toaelf stale --cold-forand every other recency consumer.--cold-formeans "cold since you turned the block off". Splitting the pair would have the store stamp a read it simultaneously denies.<cadence-resume>are embedded by_format_hits_with_session_startand go with it. feat: UPS-side cadence — inject rebuilder output via additionalContext (#749 P1 follow-up to #869) #870's<cadence-checkpoint>and the two default-off phantom notes are written outside and do not. That split is documented, and filed as its own question in hook: AELFRICE_MEMORY_BLOCK suppresses <cadence-resume> but not <cadence-checkpoint> — one feature, two behaviours under one switch #1461.[implicit_feedback] enqueue_on_retrieve = trueenqueues adeferred_feedback_queuerow per hit from insideretrieve()(inert since [Umbrella] Inert, unreachable, and decorative mechanisms — graph substrate and the delete list #1162 made the sweeper audit-only — if that sweeper is ever made mutating again, this becomes a real exposure path and UI: show user the memory block to demonstrate how it works #1359 must be revisited), andhook_agent_contextrecords exposure for its own envelope. So "records no exposure evidence" is scoped to the injection writers rather than stated flat.is_session_first_promptruns before the switch is resolved and consumes the slot; leaving it unguarded is correct, becauseaelf scope-outresolves against the same file'ssession_idkey.Tests
Every arm carries an in-test enabled control, so a zero cannot mean a dead fixture. Both halves of the ring split are pinned independently — mutating it either way (guard the whole call again; or record ids on a suppressed fire) turns a different test red, and neither assertion alone survives the wrong fix.
_record_toucheskept its own guard, which was redundant whileinjected_idswas empty and is now load-bearing, so it is mutation-checked separately.Full suite: 7,654 passed, 70 skipped, 71 xfailed.
Summary by Sourcery
Add a configurable off-switch and hint line for the injected
<aelfrice-memory>block, ensure suppression does not affect retrieval or correction lanes but prevents exposure accounting from claiming unseen injections, and surface the switch state inaelf doctorand documentation.New Features:
AELFRICE_MEMORY_BLOCKenv var and[memory_block] enabledTOML setting to control emission of the<aelfrice-memory>block, with env overriding config.aelf tailand the off-switch.aelf doctoras aMemory blocksection.Bug Fixes:
feedback_history,injection_events,belief_touches, exploration events) or changing belief posteriors, keeping exploration pool and recency semantics correct.Enhancements:
record_exposureparameter, used by the memory-block off-switch but preserving existing callers’ behaviour.Documentation:
[memory_block]configuration section, env override behaviour, and the operational consequences of disabling the memory block in user config docs.Tests:
Summary by CodeRabbit
New Features
AELFRICE_MEMORY_BLOCKenvironment variable.aelf doctor.Documentation
Bug Fixes