Conversation
Phase 6, deferred from #636. A level-0 checkpoint covers ~40 messages, so a channel running for months accumulates hundreds of them while the prompt view carries at most `max_recent` + `max_older`. Past that the oldest entries fell off the end of the view entirely — still in the table and still reachable through the tool, but no longer visible to the agent unless it went looking. Rollups fold the oldest run of un-rolled checkpoints into one entry covering the whole stretch, and they nest: once enough level-1 rollups accumulate they roll into level-2 by the same path, so a session of any length keeps a bounded number of entries at the top of the view. ## Nothing is destroyed The obvious implementation — replace the covered rows with one — is the untraceable summary-of-summaries the design set out to avoid. Instead children keep their rows and are stamped with `rolled_up_into`, the rollup records `rolls_up_from_seq`/`rolls_up_to_seq`, and its message coverage is exactly the union of theirs. Both directions are queryable, so a rollup expands back into its checkpoints and each of those back into raw transcript. The insert and the stamping are one `BEGIN IMMEDIATE` transaction, and the children are re-read inside it. A rollup whose children were left unmarked would be rolled forever; children marked without a surviving parent would vanish from the view with nothing covering them. There is one summary-of-summaries here — a rollup reads its children's summaries, not raw messages. It is bounded to a single level of recursion per generation and reversible, because the level-0 rows are still there. ## Surfaces - View selection asks for the most compact covering set rather than level-0 only, so an absorbed checkpoint is represented by its rollup. - `chronicle list` shows every level with a `[rollup]` marker; `open` on a rollup lists the checkpoints it covers so it is never an opaque blob. - `rollup_threshold` (12) and `rollup_batch` (8) are exposed through TOML, GET/PATCH config, OpenAPI, generated client, CLI inspection and self-inspection, and clamped at load — a batch below two buys nothing and a batch above the threshold would roll entries that have not accumulated. ## Tests Provenance in both directions with child text unchanged, coverage equal to the union, double-rollup refused, view selection preferring the rollup, and nesting to level 2 with the chain back down intact.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughChronicle checkpoints now support configurable hierarchical rollups. Rollups use transactional child stamping, prompt-generated summaries, nested levels, uncovered-history retrieval, and rollup-aware listing and expansion. ChangesChronicle hierarchical rollups
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 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 |
jamiepine
marked this pull request as ready for review
August 10, 2026 22:59
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/tools/chronicle.rs (1)
179-226: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThe list now shows the same span twice.
list_all_levelsreturns rollups and the children they cover. The test atsrc/conversation/chronicle.rs:1440-1465asserts four rows for three checkpoints and one rollup. The output therefore contains a[rollup]entry plus every child it absorbed, each marked· rolled up. This contradicts the intent stated in the comment at lines 179-181 and it spends thelimitbudget on duplicated coverage.If you want the rollup to stand for its children in the index, filter out entries where
rolled_up_intois set. Keep the children reachable throughopen.♻️ Proposed change
for checkpoint in &checkpoints { + // A rolled-up checkpoint is already represented by its rollup. + if checkpoint.rolled_up_into.is_some() { + continue; + } summary.push_str(&format!(Reporting
checkpoints.len()in the header then needs the filtered count.🤖 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/tools/chronicle.rs` around lines 179 - 226, Update the checkpoint list flow around list_all_levels and the rendering loop to filter out entries whose rolled_up_into is set, so rollups stand in for their absorbed children while those children remain available through open. Use the filtered collection for both row rendering and the header’s checkpoint count, preserving ordering and the existing empty-list behavior.
🧹 Nitpick comments (2)
src/agent/chronicle.rs (2)
795-863: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
summarize_rollupduplicates the model and hook setup insummarize.Lines 817-837 repeat the routing resolution,
SpacebotModel::make,AgentBuilder, andSpacebotHookconstruction fromsummarizeat lines 886-906. Only the context label differs. Extract one helper that takes the preamble and the context label and returns the agent and hook.🤖 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/agent/chronicle.rs` around lines 795 - 863, Refactor the duplicated model, AgentBuilder, and SpacebotHook setup from summarize_rollup and summarize into one shared helper that accepts the rendered preamble and context label, returning the configured agent and hook. Reuse this helper in both methods while preserving their existing routing, model override, process type, channel, and differing context labels.
682-701: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueHigher levels only advance after a lower level commits.
roll_up_if_duereturns as soon asroll_up_levelreportsOk(false). Leveln+1is therefore evaluated only in the same pass where levelncommitted a rollup. If level 0 stops producing rollups, for example after a session goes quiet or afterrollup_thresholdis lowered, accumulated level-1 entries never roll up.Consider continuing to the next level instead of returning, and stop only when no level committed.
♻️ Proposed change
let mut level = 0i64; while level < MAX_ROLLUP_LEVELS { match self.roll_up_level(level).await { - Ok(true) => level += 1, - Ok(false) => return, + Ok(_) => level += 1, Err(error) => {🤖 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/agent/chronicle.rs` around lines 682 - 701, Update roll_up_if_due so Ok(false) from roll_up_level does not immediately return; continue evaluating higher levels, and track whether any level committed a rollup. Stop only after a full pass makes no progress, while preserving the existing error logging and return behavior.
🤖 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 `@docs/design-docs/session-chronicles.md`:
- Line 47: Resolve the conflict between invariant 2 and the rollup behavior
described around child checkpoint summaries: explicitly define rollups as an
exception to the no-checkpoint-text rule, or revise invariant 2 to apply only to
level-0 interval checkpoints. Ensure the wording consistently permits rollups to
use child checkpoint summaries.
In `@src/agent/chronicle.rs`:
- Around line 736-764: Update summarize_rollup to return Option<(String, String,
Option<String>)>, returning None for both prompt-rendering and model-call
failures. In roll_up_level, handle None by skipping commit_rollup and returning
without marking children as rolled up; only construct and commit NewCheckpoint
when summarization succeeds, allowing the next cut to retry.
- Around line 1187-1194: The older-history selection in the flow around
uncovered_before_seq must use the coverage boundary rather than the first
entry’s seq: pass recent.first().covers_from_seq as the cutoff and ensure
candidates are restricted to covers_to_seq <= that cutoff, preserving the
existing limit. Add a regression test covering an older rollup whose children
are hidden by rolled_up_into while checkpoint ordering would otherwise exclude
the rollup.
In `@src/conversation/chronicle.rs`:
- Around line 1348-1363: The commit_chain function uses abbreviated variable
names; rename out to checkpoints and cp to checkpoint, updating their push and
dereference usages while preserving behavior.
- Around line 631-633: Update the rollback handling in the match arm around the
transaction outcome so a failed sqlx::query("ROLLBACK").execute call is not
discarded: either log the rollback error or propagate it without replacing the
original transaction outcome. Preserve the existing behavior for successful
rollbacks and keep the original error/result as the returned transaction
outcome.
- Around line 497-501: The chronicle query in render_chronicle_view must select
compact entries by coverage boundaries rather than creation seq, so rollups
covering history before before_seq remain eligible with their stamped children.
Update the filtering and ordering logic around covers_from_seq/covers_to_seq
while preserving the recent/older uncovered hierarchy, and add a regression test
covering a rollup that overlaps the level-0 recent-window cutoff.
- Around line 645-681: Update the transaction flow around the claimed-child
query and commit_in_transaction to load every requested child row, then validate
the complete set before inserting the rollup: exact ID equality, matching
new.channel_id, level equal to new.level - 1, contiguous coverage, and
boundaries matching the new rollup’s coverage. Reject malformed, missing,
foreign, already-rolled-up, or partial child sets without stamping or
committing; only execute the existing stamp update after validation and
successful commit.
---
Outside diff comments:
In `@src/tools/chronicle.rs`:
- Around line 179-226: Update the checkpoint list flow around list_all_levels
and the rendering loop to filter out entries whose rolled_up_into is set, so
rollups stand in for their absorbed children while those children remain
available through open. Use the filtered collection for both row rendering and
the header’s checkpoint count, preserving ordering and the existing empty-list
behavior.
---
Nitpick comments:
In `@src/agent/chronicle.rs`:
- Around line 795-863: Refactor the duplicated model, AgentBuilder, and
SpacebotHook setup from summarize_rollup and summarize into one shared helper
that accepts the rendered preamble and context label, returning the configured
agent and hook. Reuse this helper in both methods while preserving their
existing routing, model override, process type, channel, and differing context
labels.
- Around line 682-701: Update roll_up_if_due so Ok(false) from roll_up_level
does not immediately return; continue evaluating higher levels, and track
whether any level committed a rollup. Stop only after a full pass makes no
progress, while preserving the existing error logging and return behavior.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fda6684f-065e-446e-afb0-da8ecf01aaf1
📒 Files selected for processing (15)
docs/content/docs/(configuration)/config.mdxdocs/design-docs/session-chronicles.mdinterface/src/api/schema.d.tsprompts/en/chronicle_rollup.md.j2src/agent/chronicle.rssrc/api/config.rssrc/cli/agent.rssrc/config/load.rssrc/config/toml_schema.rssrc/config/types.rssrc/conversation/chronicle.rssrc/prompts/engine.rssrc/prompts/text.rssrc/self_awareness.rssrc/tools/chronicle.rs
| 1. **Coverage is total, contiguous, and non-overlapping.** Checkpoint N's start boundary *is* checkpoint N-1's end boundary, read in the same transaction that writes N. There is no span of the durable log that is covered twice, and none that is covered zero times once chronicling has started. | ||
| 2. **A checkpoint summarizes raw messages only.** Prior summaries may be supplied to the summarizer as narrative context, but the output describes only the new interval. No checkpoint is ever regenerated from another checkpoint's text. | ||
| 3. **Checkpoints are append-only and immutable.** Rollups add rows; they never delete or rewrite the rows they cover. A level-0 checkpoint's summary text, boundaries, and sequence are fixed at commit. | ||
| 3. **Checkpoints are append-only and immutable.** Rollups add rows and stamp `rolled_up_into` on what they cover; they never delete or rewrite it. A checkpoint's summary text, boundaries, and sequence are fixed at commit, so a rollup can always be expanded back into the checkpoints it summarizes, and each of those back into raw transcript. |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Resolve the rollup invariant conflict.
Invariant 2 states that no checkpoint is generated from checkpoint text. Lines 96-98 state that rollups use child checkpoint summaries. Define rollups as an explicit exception, or limit invariant 2 to level-0 interval checkpoints.
🤖 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/design-docs/session-chronicles.md` at line 47, Resolve the conflict
between invariant 2 and the rollup behavior described around child checkpoint
summaries: explicitly define rollups as an exception to the no-checkpoint-text
rule, or revise invariant 2 to apply only to level-0 interval checkpoints.
Ensure the wording consistently permits rollups to use child checkpoint
summaries.
Comment on lines
+645
to
+681
| // Re-read the children inside the transaction: another rollup may have | ||
| // claimed them between selection and commit. | ||
| let placeholders = vec!["?"; child_ids.len()].join(","); | ||
| let claimed_sql = format!( | ||
| "SELECT COUNT(*) AS claimed FROM channel_chronicle_checkpoints \ | ||
| WHERE id IN ({placeholders}) AND rolled_up_into IS NOT NULL" | ||
| ); | ||
| let mut claimed_query = sqlx::query(&claimed_sql); | ||
| for id in child_ids { | ||
| claimed_query = claimed_query.bind(id); | ||
| } | ||
| let claimed: i64 = claimed_query | ||
| .fetch_one(&mut *connection) | ||
| .await? | ||
| .try_get("claimed") | ||
| .unwrap_or(0); | ||
| if claimed > 0 { | ||
| return Ok(CommitOutcome::Superseded { | ||
| expected: 0, | ||
| found: claimed, | ||
| }); | ||
| } | ||
|
|
||
| let outcome = self.commit_in_transaction(new, &mut *connection).await?; | ||
| let CommitOutcome::Committed(rollup) = &outcome else { | ||
| return Ok(outcome); | ||
| }; | ||
|
|
||
| let stamp_sql = format!( | ||
| "UPDATE channel_chronicle_checkpoints SET rolled_up_into = ? \ | ||
| WHERE id IN ({placeholders})" | ||
| ); | ||
| let mut stamp_query = sqlx::query(&stamp_sql).bind(&rollup.id); | ||
| for id in child_ids { | ||
| stamp_query = stamp_query.bind(id); | ||
| } | ||
| stamp_query.execute(&mut *connection).await?; |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Validate the complete child set inside the transaction.
The transaction only checks whether any supplied ID is already stamped. It does not verify that every ID exists, belongs to new.channel_id, has new.level - 1, is contiguous, or matches the new rollup coverage. A stale or malformed child list can stamp foreign or partial rows and create a parent whose coverage does not equal the union of its children.
Load and validate the exact child rows before insertion. Reject the commit unless the validated rows exactly match the requested IDs and rollup boundaries.
🤖 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/conversation/chronicle.rs` around lines 645 - 681, Update the transaction
flow around the claimed-child query and commit_in_transaction to load every
requested child row, then validate the complete set before inserting the rollup:
exact ID equality, matching new.channel_id, level equal to new.level - 1,
contiguous coverage, and boundaries matching the new rollup’s coverage. Reject
malformed, missing, foreign, already-rolled-up, or partial child sets without
stamping or committing; only execute the existing stamp update after validation
and successful commit.
- filter rolled-up children from chronicle list output (avoid duplicate coverage) - summarize_rollup returns Option — skip commit on failure instead of writing a fallback placeholder - use coverage boundaries (covers_from_seq) for older history cutoff so rollups are included even when their commit seq is higher - rename abbreviated variables in commit_chain (out→checkpoints, cp→checkpoint) - log rollback errors instead of silently discarding with .ok() - roll_up_if_due continues to higher levels instead of returning early on Ok(false)
coruhoorhan
pushed a commit
to coruhoorhan/spacebot
that referenced
this pull request
Sep 14, 2026
The stabilization plan made the instance reliable; this doc plans what it becomes: an assistant that remembers, finishes what it starts and answers when called. It lays out four phases — J0 bring-up on the stronger machine, J1 reliability (ACP result path, streaming fallback chain, cortex budget), J2 memory (active recall, chronicle summarization, merge bloat, export/import CLI) and J3 autonomy/access (resident channels, Telegram, voice) — each with measurable exit criteria, and maps them to the upstream PRs worth porting (spacedriveapp#593, spacedriveapp#652, spacedriveapp#637, spacedriveapp#641, spacedriveapp#578, spacedriveapp#604, spacedriveapp#650/spacedriveapp#651) since upstream has merged nothing since Aug 17 and the fork is the live line of development. Also records the clean-install vs memory-transplant decision (§4) left open on 2026-09-11, with the measured trade-off: 503 memories, 2715 edges, 1500 messages and the 159 MB vector store against a fresh schema. 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
bruno320x
added a commit
to bruno320x/spacebot
that referenced
this pull request
Sep 15, 2026
bruno320x
added a commit
to bruno320x/spacebot
that referenced
this pull request
Sep 15, 2026
github-actions Bot
added a commit
to bruno320x/spacebot
that referenced
this pull request
Sep 15, 2026
bruno320x
added a commit
to bruno320x/spacebot
that referenced
this pull request
Sep 15, 2026
This branch has not been deployed
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.
Phase 6 of session chronicles, deferred out of #636.
A level-0 checkpoint covers about 40 messages, so a channel running for months accumulates hundreds while the prompt view carries at most
max_recent+max_olderentries. Past that the oldest simply fell off the end — still in the table, still reachable through thechronicletool, but no longer visible to the agent unless it went looking for them.Rollups fold the oldest run of un-rolled checkpoints into one entry covering the whole stretch, and they nest: once enough level-1 rollups accumulate they roll into level-2 by the same path. A session of any length keeps a bounded number of entries at the top of the view.
Nothing is destroyed
The obvious implementation — replace the covered rows with one — is exactly the untraceable summary-of-summaries the original design set out to avoid. So children keep their rows and are stamped with
rolled_up_into, the rollup recordsrolls_up_from_seq/rolls_up_to_seq, and its message coverage is exactly the union of theirs. Both directions are queryable: a rollup expands back into its checkpoints, and each of those back into raw transcript.The insert and the stamping are one
BEGIN IMMEDIATEtransaction, and the children are re-read inside it before anything is written:A rollup whose children were left unmarked would be rolled forever; children marked without a surviving parent would drop out of the view with nothing covering them.
There is one summary-of-summaries here — a rollup reads its children's summaries, not raw messages. It is bounded to a single level of recursion per generation and reversible, because the level-0 rows are still sitting there.
Contiguity is checked rather than assumed: if the selected run has a gap the rollup declines to commit instead of claiming a span it did not summarize.
Surfaces
chronicle listshows every level with a[rollup]marker;openon a rollup lists the checkpoints it covers, so it is never an opaque blob.rollup_threshold(12) androllup_batch(8) go through TOML, GET/PATCH config, OpenAPI, the generated client, CLI inspection and self-inspection, and are clamped at load — a batch below two buys nothing, and a batch above the threshold would try to roll entries that have not accumulated.No migration.
level,rolled_up_intoandrolls_up_from_seq/rolls_up_to_seqwere already in the schema from #636, sitting inert; this is what writes them.Testing
Five rollup tests, alongside the 44 already covering chronicles:
Known limits
roll_up_if_dueruns after a successful cut rather than on its own schedule, so a channel that stops cutting stops rolling up. That is the intended coupling — no new checkpoints means nothing new to roll — but a channel parked just under the interval keeps an un-rolled tail.Channel.