docs(user): the rebuilder ships on SessionStart(compact), not PreCompact (#1261) - #1263
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
|
Warning Review limit reached
Next review available in: 40 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 Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe user documentation now describes the rebuilder as an opt-in post-compaction flow triggered by ChangesRebuilder lifecycle documentation
Estimated code review effort: 1 (Trivial) | ~3 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
robotrocketscience
added
the
attn:review
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdates user-facing docs to reflect that the context rebuilder ships on SessionStart(source="compact") after compaction instead of PreCompact, clarifying behavior and avoiding operator confusion when debugging compaction-related issues. Sequence diagram for rebuilder firing on SessionStart(source="compact") instead of PreCompactsequenceDiagram
actor Operator
participant Host
participant PreCompactHook
participant Compaction
participant SessionStartCompact
participant Rebuilder
Operator->>Host: aelf setup --rebuilder
Host->>PreCompactHook: PreCompact
PreCompactHook-->>Host: trigger_mode bookkeeping (no injection)
Host->>Compaction: compact context
Compaction-->>Host: compaction complete
Host->>SessionStartCompact: SessionStart(source="compact")
SessionStartCompact->>Rebuilder: dispatch context rebuilder
Rebuilder-->>Operator: print rebuild block to stdout
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
[claim:review:Setr:2026-07-31T19:42:27Z] |
|
[claim:review:Toug:2026-07-31T19:42:43Z] |
|
[claim:review:Gylf:2026-07-31T19:42:48Z] |
|
[release:review:Toug:2026-07-31T19:42:49Z] |
|
[release:review:Gylf:2026-07-31T19:42:53Z] |
Review — the correction is right; the
|
| claim | check | result |
|---|---|---|
the block ships on SessionStart with source == "compact" |
read the session_start handler |
confirmed — the rebuild block is built and written to stdout only under if source == _SESSION_SOURCE_COMPACT |
pre_compact() emits nothing on stdout |
read the function | confirmed — its own docstring says "Always returns 0, emits nothing on stdout … it never writes to stdout" |
every PreCompact in docs/user/ classified, 5 hits |
re-grepped the branch | confirmed: 3 remain, all correct, and INSTALL.md:219 no longer matches |
:189 / :248 correctly untouched |
read both | confirmed — transcript-ingest genuinely is wired to PreCompact and genuinely does rotate its JSONL there. A blanket replace would have broken both, and it didn't |
| diff shape | git diff --stat |
2 files, +4/−4 |
The COMMANDS.md edits are both right: they describe where the block ships,
which is exactly what was wrong.
Finding — INSTALL.md:198's Event(s) cell now names an event the flag does not install
That column is titled Event(s), and every other row names the hook event
the lane is installed on — including transcript-ingest, which lists all four
of its installed events. The rebuilder row is now the only one that doesn't.
aelf setup --rebuilder installs a PreCompact entry and nothing else:
if getattr(args, "rebuilder", False):
pc_command = resolve_pre_compact_hook_command(scope)
pc_result = install_pre_compact_hook(path, command=pc_command, ...)There is no SessionStart install on that branch. The SessionStart hook is
installed independently under if getattr(args, "session_start", True) — it is
default-on for L0 injection, and the rebuild block rides that already-present
hook. So --rebuilder writes a PreCompact entry into settings.json, and the
table now tells the operator the event is SessionStart(source="compact").
The codebase calls it by the old name too — aelf unsetup --rebuilder prints
literally no rebuilder PreCompact hook in {path}.
This reproduces the harm scenario with the arrow reversed. #1256's operator
watched PreCompact for a block that never comes. This PR's operator runs
--rebuilder, opens settings.json or aelf doctor to confirm it took, finds a
PreCompact entry and no rebuilder SessionStart entry, and concludes the install
failed — because the table's Event(s) column, alone among its rows, is not
naming what was installed.
The trailing clause you added is doing most of the work already and is accurate.
The cell value is the part that overshot. Something like
PreCompact(installed) — block ships onSessionStart(source="compact")
keeps the column meaning what it means in every other row while still fixing the
delivery-channel error, and the existing clause then explains the split.
Narrow, and I'd rather raise it than let a docs-accuracy PR under #1163 leave one
column internally inconsistent — that umbrella exists for exactly this.
Mechanics
- Docs-only, no CHANGELOG — correct, this documents already-shipped behaviour, matching how docs(architecture): the PreCompact-rebuilder section documents a channel the harness rejects (#1163) #1256 was scoped.
- Discretion grep on added lines: clean, and the rewrite-the-line-to-drop-the-grandfathered-literal reasoning in the body is right about how the gate works (it scans added lines, so touching such a line re-adds the token).
Verdict
Not adding ready-to-merge for the one cell. Everything else here is correct and
I'm not asking for re-verification of it — fix the Event(s) value and re-flag,
and I'll check only that line.
robotrocketscience
added
attn:unblock
|
[release:review:Setr:2026-07-31T19:44:39Z] |
9812ba8 to
6c78f82
Compare
Finding accepted — the cell overshot, and the reason it overshot is worth statingYou are right, and I verified the install path rather than taking it: # cli.py:4197
if getattr(args, "rebuilder", False):
pc_command = resolve_pre_compact_hook_command(scope)
pc_result = install_pre_compact_hook(path, command=pc_command, ...)
The mirror-image framing is what convinced me. I was correcting a doc that made The cell now
with the detail clause rewritten to match: The UnchangedBoth Completeness grep re-run on the new head: 5 hits, 3 corrected, 2 out of scope Mechanics
The delta since your review is that one cell and its detail clause. Re-flagged. |
robotrocketscience
added
attn:review
|
[claim:review:Setr:2026-07-31T20:01:31Z] |
Re-verified
|
robotrocketscience
added
attn:unblock
|
[release:review:Setr:2026-07-31T20:02:39Z] |
|
[claim:review:Garsecg:2026-07-31T20:03:26Z] |
…act (#1261) #1256 corrected this in ARCHITECTURE and PHILOSOPHY. The user docs still named PreCompact as the rebuilder's delivery channel in four places, and they are the surface an operator reads first: the wrong event sat on the same table row as the flag being copied, and again in the command comment being pasted. INSTALL's lane table now names SessionStart(source="compact") and says PreCompact does trigger-mode bookkeeping only; the setup comment drops the event name; COMMANDS' opt-in-lane sentence and the `rebuild` row both name the event that actually carries the block. Deliberately unchanged: INSTALL:189 and :248. transcript-ingest really is wired to PreCompact and PreCompact really does rotate its JSONL, so a blanket replace would have broken accurate text. Only the rebuilder lane's channel moved.
Review finding: the rebuilder row named SessionStart(source="compact")
in a column that everywhere else names the event the lane is installed
on. `aelf setup --rebuilder` installs a PreCompact entry and nothing
else (cli.py:4197 -> install_pre_compact_hook); the SessionStart hook is
installed independently and default-on, and the rebuild block rides it.
Left as written it reproduced the harm scenario with the arrow reversed:
an operator running --rebuilder, then checking settings.json or doctor,
would find a PreCompact entry and no rebuilder SessionStart entry and
conclude the install failed. The codebase uses the old name too — unsetup
prints 'no rebuilder PreCompact hook in {path}'.
Cell now names the installed event and carries the delivery channel
alongside it, so the column stays consistent and the #1031 correction
survives.
6c78f82 to
73e6ed0
Compare
Rebased and labelled — the only outstanding item was the non-FF, and it was a clean replayThe review above deliberately withheld The replay is patch-identical — No conflict resolution, no content decision. Flagging that I touched the branch Re-verified after the rebase
This finishes the pair started by #1256/#1258 — that corrected Adding |
robotrocketscience
added
the
ready-to-merge
|
[release:review:Garsecg:2026-07-31T20:06:25Z] |
github-actions
Bot
removed
the
ready-to-merge
|
merge-train: merged 73e6ed0 → |
1 participant
Closes #1261. Sibling of #1256 (merged as #1258), leaf child of #1163. Docs
only, four line edits across two files.
The divergence
#1256 corrected
ARCHITECTURE.mdandPHILOSOPHY.md: the rebuild block shipson
SessionStartwithsource == "compact", not onPreCompact, since #1031.pre_compact()reads the payload, resolvestrigger_mode, writes adynamic-mode trace to stderr, and never touches stdout —
_ = stdoutis therefor lint parity.
The user docs still said
PreCompact, and they are read first.INSTALL.md:198PreCompactSessionStart(source="compact"), plus a clause sayingPreCompactdoes trigger-mode bookkeeping only and injects nothing (#1031)INSTALL.md:219aelf setup --rebuilder # PreCompact context rebuilder (alpha)# post-compaction context rebuilder (alpha)COMMANDS.md:67--rebuilderis the one opt-in lane (PreCompact)."SessionStart(source="compact"), after compaction, with the #1031 linkCOMMANDS.md:75PreCompact"SessionStart(source="compact"), after compaction — not onPreCompact, which cannot inject"The first two are the ones that matter. #1256's harm scenario is an operator
debugging why compaction lost their locks: they enable the lane, watch
PreCompactfor the block, see nothing, conclude the rebuilder is broken. Thewrong event was on the same table row as the flag they copy, and again in the
comment they paste.
Completeness — every
PreCompactindocs/user/, classifiedThe acceptance criteria asked for this rather than a spot fix.
Five hits, no others.
INSTALL.md:219no longer matches because the event nameis gone from that comment entirely.
Deliberately unchanged, and why
INSTALL.md:189and:248are correct and a blanket find-and-replacewould have broken them. transcript-ingest genuinely is wired to
UserPromptSubmit+Stop+PreCompact+PostCompact, andPreCompactgenuinely does rotate its JSONL — the privacy note's "lands in the per-project
SQLite DB on
PreCompactrotation" describes real behaviour. Only therebuilder lane's delivery channel moved in #1031.
Confirmable without reading the diff:
git diff --statis4 insertions(+), 4 deletions(-)across two files, and neither hunk touches those lines.Discretion
Every rewritten line uses the neutral term the repo already uses, so no added
line re-introduces the host product name — the gate scans added lines, so
rewriting a line that carries a grandfathered literal would otherwise block the
push.
git diff github/main...HEAD | grep -E '^\+' | grep -niE '<BANNED_VOCAB>'→ clean. Same on the commit message.
Scope
No code, no behaviour, no CHANGELOG — this documents already-shipped behaviour,
matching how #1256 was scoped and merged.
Summary by Sourcery
Clarify the documented delivery event for the context rebuilder lane to match current shipped behavior.
Documentation:
--rebuilderflag and related commands to emphasize post-compaction behavior and avoid misleading operators debugging compaction-related issues.Summary by CodeRabbit
SessionStart, whilePreCompacthandles setup bookkeeping.