fix(hook): inject rebuild block at SessionStart(source=compact), not PreCompact (#1031) - #1032
Merged
github-actions[bot] merged 2 commits intoJun 30, 2026
Conversation
…PreCompact (#1031) The PreCompact hook emitted the <aelfrice-rebuild> block as hookSpecificOutput.additionalContext, but the host harness does not accept additionalContext from a PreCompact hook (PreCompact is absent from the canonical list of context-injecting events). The harness rejected the output with a validation error on every /compact and discarded the entire block, making the context-rebuilder a silent no-op. Move the injection to the SessionStart hook on source==compact, which fires after compaction and which the harness honors (same raw-stdout channel the locked baseline already uses). The turns.jsonl log survives compaction, so the recent-turn tail is read identically. Neuter the PreCompact hook's stdout emission (retained only for trigger-mode parity and the dynamic-mode parked trace). trigger_mode=manual still suppresses the automatic path; explicit aelf rebuild is unchanged. Tests for the rebuild emission move from pre_compact to session_start(source=compact).
Reviewer's GuideMoves the context-rebuilder’s Sequence diagram for context-rebuilder injection moving from PreCompact to SessionStartsequenceDiagram
participant Harness
participant pre_compact
participant session_start
participant _build_rebuild_block_from_payload
participant _rebuild_and_format
Harness->>pre_compact: pre_compact(stdin, stdout, stderr)
pre_compact-->>Harness: return 0 (no stdout)
Harness->>session_start: session_start(stdin, stdout, stderr)
session_start->>session_start: _extract_session_id
session_start->>session_start: _parse_pre_compact_payload
alt [source == compact]
session_start->>_build_rebuild_block_from_payload: _build_rebuild_block_from_payload(payload)
_build_rebuild_block_from_payload->>_build_rebuild_block_from_payload: load_rebuilder_config
alt [trigger_mode == threshold]
_build_rebuild_block_from_payload->>_build_rebuild_block_from_payload: _read_recent_for_pre_compact
_build_rebuild_block_from_payload->>_build_rebuild_block_from_payload: db_path
_build_rebuild_block_from_payload->>_rebuild_and_format: _rebuild_and_format(recent, token_budget,...)
_rebuild_and_format-->>_build_rebuild_block_from_payload: rebuild_block
_build_rebuild_block_from_payload-->>session_start: rebuild_block
session_start->>Harness: write rebuild_block to stdout
else [trigger_mode != threshold or skip]
_build_rebuild_block_from_payload-->>session_start: ""
end
else [source != compact]
session_start-->>Harness: baseline-only stdout
end
session_start-->>Harness: return 0
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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 Review limit reached
Next review available in: 52 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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 Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughFixes ChangesRebuild injection migration: PreCompact → SessionStart:compact
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- There are now three separate
_rebuild_blockhelpers (and multiple_payload/_start_compactvariants) in different test modules that are nearly identical; consider centralizing these in a shared test utility to reduce duplication and keep future behavior changes in one place. - In
pre_compact, thestdoutargument is only referenced to appease unused-argument linting; it may be clearer to either rename it to_stdoutor add a brief note near the function signature that the parameter is intentionally unused to avoid confusion for future readers.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There are now three separate `_rebuild_block` helpers (and multiple `_payload` / `_start_compact` variants) in different test modules that are nearly identical; consider centralizing these in a shared test utility to reduce duplication and keep future behavior changes in one place.
- In `pre_compact`, the `stdout` argument is only referenced to appease unused-argument linting; it may be clearer to either rename it to `_stdout` or add a brief note near the function signature that the parameter is intentionally unused to avoid confusion for future readers.
## Individual Comments
### Comment 1
<location path="src/aelfrice/hook.py" line_range="2640-2649" />
<code_context>
+def _build_rebuild_block_from_payload(payload: dict[str, object]) -> str:
</code_context>
<issue_to_address>
**issue (bug_risk):** Align token-budget overrides between baseline rebuild and compact-session rebuild block
In `session_start`, a provided `token_budget` override is honored for the primary baseline rebuild, but `_build_rebuild_block_from_payload` always uses `config.token_budget`. This means the same hook entrypoint yields different behavior before vs. after compaction. If you intend parity, please pass the effective budget into `_build_rebuild_block_from_payload` (or add an optional override parameter) so both paths respect the same override semantics.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![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 (1)
tests/test_context_rebuilder_hook.py (1)
111-160: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffConsider centralizing the shared SessionStart test helpers.
_payload,_rebuild_block, and_start_compactare duplicated near-verbatim acrosstests/test_context_rebuilder_hook.py,tests/test_hook_pre_compact.py, andtests/test_rebuilder_triggers.py(note theeventvshook_event_namekeyword drift between copies). A sharedconftest.py/helper module would prevent divergence as the rebuild-block contract evolves.🤖 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_context_rebuilder_hook.py` around lines 111 - 160, The SessionStart test helper logic is duplicated across multiple test modules, and the copies are already drifting in keyword usage between event and hook_event_name. Centralize `_payload`, `_rebuild_block`, and `_start_compact` into a shared test helper (for example a common helper module or conftest fixture) and update `session_start`-based tests to import and reuse it so the rebuild-block contract stays consistent.
🤖 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 `@src/aelfrice/hook.py`:
- Around line 2718-2721: The stdin read in the compact parsing path is
swallowing exceptions in `raw = sin.read()` without any diagnostic, which hides
failures that affect `session_id` and `source=="compact"` handling. Update the
`try/except` around the read in this hook to log the exception with enough
context before continuing or re-raising as appropriate, using the surrounding
compact/session parsing logic to locate the failure. Keep the behavior in
`hook.py` consistent with the rest of the error handling in this code path and
avoid a bare silent `except`.
---
Nitpick comments:
In `@tests/test_context_rebuilder_hook.py`:
- Around line 111-160: The SessionStart test helper logic is duplicated across
multiple test modules, and the copies are already drifting in keyword usage
between event and hook_event_name. Centralize `_payload`, `_rebuild_block`, and
`_start_compact` into a shared test helper (for example a common helper module
or conftest fixture) and update `session_start`-based tests to import and reuse
it so the rebuild-block contract stays consistent.
🪄 Autofix (Beta)
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
Run ID: 4b9c9cd3-aa10-4d20-b540-527fb09349d1
📒 Files selected for processing (5)
CHANGELOG/v3.mdsrc/aelfrice/hook.pytests/test_context_rebuilder_hook.pytests/test_hook_pre_compact.pytests/test_rebuilder_triggers.py
A read failure dropped session_id and the source/cwd parsing the compact-rebuild path needs; surface it on stderr instead of swallowing silently. Addresses CodeRabbit review.
robotrocketscience
added
the
ready-to-merge
github-actions
Bot
removed
the
ready-to-merge
|
merge-train: merged 28533af → |
4 tasks
robotrocketscience
deleted the
fix/issue-1031-precompact-sessionstart-rebuild
branch
June 30, 2026 13:35
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1031.
Problem
On every
/compact, theaelf-pre-compact-hookfailed the host harness's hook-output validation and its entire<aelfrice-rebuild>block was discarded — the context-rebuilder was a silent no-op on compaction. Observed live:Root cause: the host harness does not accept
additionalContextunderhookSpecificOutputforPreCompact. PreCompact is absent from the canonical list of context-injecting events (SessionStart,UserPromptSubmit,PreToolUse,PostToolUse,Stop, …). The wire shape was copied from the PreToolUse search-tool hook, whereadditionalContextis valid. The PreCompact hook exists only for this injection (transcript rotation is a separate hook), so it was 100% dead weight producing a rejected output.Fix
Move the rebuild-block injection to the SessionStart hook gated on
source == "compact":<aelfrice-baseline>block already ships via its raw stdout), andsource == "compact"fires after compaction completes — verified live (SessionStart:compact succeeded in the same session the PreCompact hook was rejected).turns.jsonllog is append-only and survives compaction, so the recent-turn tail reads identically.manual→ no rebuild;threshold→ emit. The rebuild stays gated onsource == "compact"(a normal startup SessionStart never emits it).dynamic-mode parked trace).aelf rebuild//aelf:rebuildis unchanged (drivesrebuild_v14directly).Tests
pre_compactexit-contract tests retained; new test asserts it stays silent even with turns + hits + threshold mode.pre_compacttosession_start(source="compact")acrosstest_hook_pre_compact.py,test_rebuilder_triggers.py,test_context_rebuilder_hook.py.Verification
5422 passed, 66 skipped, 75 xfailed.uvx vulture src --min-confidence 80: no new findings.uvx typos: clean.🤖 Generated with Claude Code
Summary by Sourcery
Move the context-rebuilder injection from the PreCompact hook to SessionStart for compaction flows and make PreCompact stdout a no-op while preserving trigger-mode behavior.
Bug Fixes:
<aelfrice-rebuild>block is accepted by the host harness after compaction by emitting it via SessionStart(source="compact") instead of PreCompact.Enhancements:
sourcefield and gate rebuild emission onsource == "compact"while leaving normal startups unchanged.Documentation:
Tests:
Summary by CodeRabbit
/compactas expected.rebuildcommand behavior unchanged.