docs(memory): align memory context docs#576
Open
vsumner wants to merge 3 commits into
Open
Conversation
Contributor
WalkthroughReplaces timer-driven "memory bulletin" framing with event-driven "knowledge synthesis" across docs and comments; documents new config keys for synthesis sizing/debounce; clarifies compactor is summary-only (no durable writes/toolserver); updates prompt/context assembly and warmup/readiness semantics. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
docs/content/docs/(features)/tasks.mdx (1)
163-178:⚠️ Potential issue | 🟡 MinorSection heading still says "Bulletin Integration" while the body has moved to "knowledge context".
Line 165:
## Bulletin IntegrationThe body (Line 167: "Active tasks (non-done) are included in the cortex knowledge context…") was rewritten to the new terminology, and the
cortex.rsmodule-layout note on Line 326 now readsknowledge context integration. The heading is the last lingering "bulletin" usage in this section and directly contradicts the renamed prose.-## Bulletin Integration +## Knowledge Context Integration🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/content/docs/`(features)/tasks.mdx around lines 163 - 178, Update the section heading "## Bulletin Integration" to match the rewritten terminology used in the body and module notes by renaming it to "## Knowledge Context Integration" (replace the unique heading string "## Bulletin Integration" in docs/content/docs/(features)/tasks.mdx), so the heading and the paragraph about "cortex knowledge context" and the "knowledge context integration" module note are consistent.docs/content/docs/(configuration)/config.mdx (1)
531-560:⚠️ Potential issue | 🟡 MinorAdd missing
bulletin_*keys to the[defaults.cortex]table or clarify their status.The
bulletin_interval_secs,bulletin_max_words, andbulletin_max_turnsfields are actively maintained inCortexConfig(defaults: 3600 seconds, 1500 words, 15 turns) and are fully exposed in the API, parsed from TOML, and used to control cortex bulletin generation timing and synthesis prompts. They are not deprecation-only compatibility surfaces.The current table at lines 532–543 omits these keys entirely, leaving no discoverable way for users to configure them. Either:
- Add the three keys to the table with their defaults and descriptions, or
- Clarify in the narrative at line 553 that these keys are deprecated and will be removed in a future release (if that is the actual intent).
The evidence shows they remain active production config, not compatibility-only surfaces, so they should be documented.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/content/docs/`(configuration)/config.mdx around lines 531 - 560, The docs omit active config keys present in CortexConfig: add `bulletin_interval_secs` (default 3600), `bulletin_max_words` (default 1500), and `bulletin_max_turns` (default 15) to the `[defaults.cortex]` table with short descriptions explaining they control bulletin generation timing and synthesis prompts; alternatively, if they truly are deprecated, update the narrative near the warmup section to state they are deprecated and will be removed, but do not leave them undocumented while they remain parsed and used by CortexConfig.docs/design-docs/working-memory-implementation-plan.md (1)
898-916:⚠️ Potential issue | 🟡 MinorMark compactor memory-extraction removal as complete, not future work.
Lines 80-84 and the provided
src/agent/compactor.rssnippets already establish the compactor as summary-only. Keeping Phase 5b as “Remove Memory Save from Compactor” makes the plan read like the current runtime still has a compactor durable-write path.📝 Proposed documentation adjustment
-### Phase 5b: Sunset Compactor Memory Extraction +### Phase 5b: Verify Compactor Memory Extraction Remains Sunset -**Dependencies:** Phase 5a deployed and validated (new persistence triggers confirmed adequate) -**Risk:** Medium — removes a memory creation path. Only deploy after Phase 5a monitoring confirms coverage. +**Dependencies:** Phase 5a deployed and validated (new persistence triggers confirmed adequate) +**Risk:** Medium — validates that the removed memory creation path stays covered by persistence. -#### 5b.1 Remove Memory Save from Compactor +#### 5b.1 Keep Memory Save Out of Compactor Update `src/agent/compactor.rs`: -- Remove durable memory-write access from the compaction worker +- Verify the compaction worker has no durable memory-write access - The compactor's only output is the compaction summary injected into history Update `prompts/en/compactor.md.j2`: -- Remove all `memory_save` instructions +- Keep all `memory_save` instructions removed - The compactor's sole responsibility is producing a compaction summary #### 5b.2 Verification - Integration test: run compaction, verify no durable memory-write calls in tool output🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/design-docs/working-memory-implementation-plan.md` around lines 898 - 916, Update the plan to mark Phase 5b as completed instead of future work: change the Phase 5b heading/description to indicate "Completed — compactor no longer performs durable memory writes" and remove or strike the “Remove Memory Save from Compactor” action items; update references in the doc text around the current summary-only claim (lines referencing src/agent/compactor.rs and prompts/en/compactor.md.j2) to state that compactor.rs no longer calls durable-write and prompts/en/compactor.md.j2 no longer contains memory_save instructions, and adjust the Verification section to document the verification already done (integration/regression checks) rather than prescribing them as upcoming steps.docs/content/docs/(core)/prompts.mdx (1)
41-48:⚠️ Potential issue | 🟡 MinorSync the prompt example with the actual fallback behavior.
The provided
prompts/en/channel.md.j2snippet guards legacy## Memory Contextwithnot working_memory and not knowledge_synthesis, whilesrc/prompts/engine.rsalso promotesmemory_bulletinintoknowledge_synthesisas a fallback. This doc sample currently suggests## Memory Contextcan render whenever knowledge synthesis is absent, even when working memory is present.📝 Proposed documentation fix
let channel_prompt = engine.render_channel_prompt( Some(identity_context), // Option<String> Some(memory_bulletin), // Option<String> legacy fallback + Some(knowledge_synthesis), // Option<String>, falls back to memory_bulletin when absent Some(skills_prompt), // Option<String> worker_capabilities, // String conversation_context, // Option<String> status_text, // Option<String> + working_memory, // Option<String> + channel_activity_map, // Option<String> + participant_context, // Option<String> )?;-{%- if memory_bulletin and not knowledge_synthesis %} +{%- if memory_bulletin and not working_memory and not knowledge_synthesis %} ## Memory Context {{ memory_bulletin }} {%- endif %}Also applies to: 123-145
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/content/docs/`(core)/prompts.mdx around lines 41 - 48, The docs example for render_channel_prompt is out of sync with the actual fallback: src/prompts/engine.rs promotes memory_bulletin into knowledge_synthesis when knowledge_synthesis is missing, and prompts/en/channel.md.j2 only renders the "## Memory Context" block when not working_memory and not knowledge_synthesis; update the prompt example (the callsite showing engine.render_channel_prompt and/or the channel.md.j2 snippet) to reflect that memory_bulletin is only shown as "Memory Context" when both working_memory and knowledge_synthesis are absent (i.e., keep the guard condition matching prompts/en/channel.md.j2 and mention that memory_bulletin will be promoted to knowledge_synthesis by src/prompts/engine.rs rather than displayed separately).docs/content/docs/(core)/memory.mdx (2)
21-21:⚠️ Potential issue | 🟡 MinorReplace the remaining “recall worker” wording with branch recall.
Line 96 says recall is branch-mediated, but Lines 21 and 65 still describe a “recall worker.” That keeps the old model alive in the same page.
Proposed doc cleanup
-The two are joined on memory ID. A recall worker queries LanceDB for semantic/keyword matches, then hits SQLite for graph traversal and metadata. No server processes -- both are embedded, everything is files in a data directory. +The two are joined on memory ID. During branch recall, the `memory_recall` tool queries LanceDB for semantic/keyword matches, then hits SQLite for graph traversal and metadata. No server processes -- both are embedded, everything is files in a data directory. ... -The graph enables traversal during recall. When a recall worker finds a relevant memory, it can walk the graph to find connected context -- related facts, the history of how a decision evolved, contradictions that need resolution. +The graph enables traversal during recall. When branch recall finds a relevant memory, it can walk the graph to find connected context -- related facts, the history of how a decision evolved, contradictions that need resolution.Based on learnings, memory recall must go through a branch, which curates before returning clean conclusions.
Also applies to: 65-65, 96-96
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/content/docs/`(core)/memory.mdx at line 21, The page still uses the phrase "recall worker" in multiple places; replace all occurrences of the phrase "recall worker" with "branch recall" and update the surrounding sentence content to reflect branch-mediated recall semantics (e.g., change "A recall worker queries LanceDB for semantic/keyword matches, then hits SQLite for graph traversal and metadata." to describe a branch recall querying LanceDB and then performing SQLite graph traversal/metadata via the branch), and ensure the page's later sentence that already says recall is branch-mediated remains consistent with these edits.
69-90:⚠️ Potential issue | 🟡 MinorDocument ingestion as a durable-memory creation path.
The page now says “Three paths,” but the PR objective also calls out ingestion as a durable memory source. Add an ingestion path or fold it explicitly into this list so the docs match the stated invariant.
Proposed doc update
-Three paths: +Four paths: ... ### 3. Cortex-initiated (system-level) The cortex observes patterns across channels and can create memories at the system level. It consolidates related memories, creates observations ("James has been asking about authentication a lot this week"), and manages the graph. + +### 4. Ingestion + +Ingestion workflows can create durable memories from imported or external source material. They use the same storage, embedding, and association pipeline as conversation-created memories.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/content/docs/`(core)/memory.mdx around lines 69 - 90, The docs list "Three paths" for durable-memory creation but omit document ingestion; add a new path (or fold into the list) titled "Document ingestion (ingest-initiated)" after the existing items, describing that ingestion pipelines/processors can create durable memories from uploaded documents, external datasets, or connectors, including how they create memory entries, association/metadata, persistence semantics, and how they differ from branch- or cortex-initiated flows; update the heading around "Three paths:" and the surrounding text (the sections for "Branch-initiated", "Persistence branches", and "Cortex-initiated") so the invariant that ingestion is a durable-memory source is explicitly stated and the list/count matches the prose.
🧹 Nitpick comments (3)
docs/design-docs/task-tracking.md (1)
128-181: Stale "bulletin" references inside the renamed "Knowledge Context Integration" section.The section heading (Line 169) and struct example (Line 174) were renamed, but two prose sentences in this file still reference "bulletin":
- Line 146: "evaluates whether it's actionable using an LLM call with the current bulletin as context"
- Line 181: "a small extension to the bulletin gathering … formatted into the bulletin sections alongside memory data"
Given this PR's intent (closes
#569) is to align all docs on the new terminology, these read as misses from the rename pass.✏️ Suggested wording updates
-2. For each unprocessed todo, evaluates whether it's actionable using an LLM call with the current bulletin as context +2. For each unprocessed todo, evaluates whether it's actionable using an LLM call with the current knowledge synthesis as context-This requires a small extension to the bulletin gathering — a direct SQLite query on the `tasks` table for non-done tasks, formatted into the bulletin sections alongside memory data. +This requires a small extension to knowledge-synthesis gathering — a direct SQLite query on the `tasks` table for non-done tasks, formatted into the knowledge-context sections alongside memory data.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/design-docs/task-tracking.md` around lines 128 - 181, Update the two leftover prose references that still use "bulletin" to the new terminology used in this section: replace "current bulletin" in the Todo Promotion Loop sentence with "current knowledge context" (or "current knowledge synthesis context") and change "bulletin gathering" / "bulletin sections" in the Knowledge Context Integration paragraph to "knowledge context gathering" / "knowledge context sections" (the surrounding identifiers to locate the lines are the "Todo Promotion Loop" heading and the KnowledgeSynthesisSection struct example).docs/design-docs/wiki.md (1)
364-384: Phase-1 bullet still references the "bulletin template".Line 374:
- Wiki index block in bulletin templateThe rest of the doc moved to "knowledge context" / "knowledge synthesis"; this Phase 1 bullet should follow. Also consider updating Line 13 ("Not the bulletin.") to either point at knowledge synthesis or explicitly mark the bulletin as legacy, to avoid reintroducing the old term as a primary concept.
-- Wiki index block in bulletin template +- Wiki index block in knowledge-synthesis template🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/design-docs/wiki.md` around lines 364 - 384, Update the Phase 1 Core bullet list: remove or relocate the item "- Wiki index block in bulletin template" so it lives with the knowledge context / knowledge synthesis section (Phase 2 or the new section), and update the introductory note near the top ("Not the bulletin.") to either reference the knowledge synthesis/knowledge context docs or explicitly mark "bulletin" as legacy; edit the "Phase 1 — Core" block and the header text containing "Not the bulletin." to keep terminology consistent and prevent reintroducing "bulletin" as the primary concept.docs/content/docs/(core)/architecture.mdx (1)
310-332: Update two tables in architecture.mdx to use "knowledge synthesis" terminology.The rest of the file was updated to reflect the knowledge synthesis terminology, but two table entries still reference "Bulletin":
- Line 319 (Max Turns table):
Bulletin generation. Single-pass with tool calls.- Line 332 (Control API endpoint groups):
Bulletin, profile, cortex chatUpdate both to keep documentation consistent with the terminology used elsewhere (cortex.mdx, configuring-channels.mdx, config.mdx).
✏️ Suggested diff
-| Cortex | 10 | Bulletin generation. Single-pass with tool calls. | +| Cortex | 10 | Knowledge synthesis. Single-pass with tool calls. |-| Cortex | `/api/cortex` | Bulletin, profile, cortex chat | +| Cortex | `/api/cortex` | Knowledge synthesis, profile, cortex chat |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/content/docs/`(core)/architecture.mdx around lines 310 - 332, Replace the two remaining uses of "Bulletin" with the standardized "knowledge synthesis" terminology: update the Cortex max-turns table entry string 'Bulletin generation. Single-pass with tool calls.' to 'Knowledge synthesis generation. Single-pass with tool calls.' and update the Control API endpoint group description 'Bulletin, profile, cortex chat' to 'Knowledge synthesis, profile, cortex chat' so the wording matches cortex.mdx/configuring-channels.mdx/config.mdx.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/content/docs/`(core)/cortex.mdx:
- Around line 18-24: The phrase "Memory mutations" is too broad—update the docs
text in cortex.mdx (the list item describing step 1) to say that only
content-changing mutations bump knowledge_synthesis_version (e.g.,
create/update/delete/merge), matching the behavior documented in
src/config/runtime.rs; keep the rest of the list intact and ensure you reference
RuntimeConfig::knowledge_synthesis / knowledge_synthesis_version so readers see
this is the specific version bump for content changes only.
In `@docs/content/docs/`(core)/memory.mdx:
- Around line 177-185: The opening sentence uses the term "memory-derived
context" too broadly; change it to make clear that layered context is injected
into the system prompt but not all layers come from memory (e.g., use wording
like "layered context (not all from memory)" or "layered context composed of
multiple sources"); explicitly mark which bullets are memory-derived (Working
memory and Knowledge context) and which are separate prompt layers (Identity
files, Channel activity, Participant context) so the list distinguishes
memory-derived layers from non-memory layers while keeping "This is not a raw
dump."
---
Outside diff comments:
In `@docs/content/docs/`(configuration)/config.mdx:
- Around line 531-560: The docs omit active config keys present in CortexConfig:
add `bulletin_interval_secs` (default 3600), `bulletin_max_words` (default
1500), and `bulletin_max_turns` (default 15) to the `[defaults.cortex]` table
with short descriptions explaining they control bulletin generation timing and
synthesis prompts; alternatively, if they truly are deprecated, update the
narrative near the warmup section to state they are deprecated and will be
removed, but do not leave them undocumented while they remain parsed and used by
CortexConfig.
In `@docs/content/docs/`(core)/memory.mdx:
- Line 21: The page still uses the phrase "recall worker" in multiple places;
replace all occurrences of the phrase "recall worker" with "branch recall" and
update the surrounding sentence content to reflect branch-mediated recall
semantics (e.g., change "A recall worker queries LanceDB for semantic/keyword
matches, then hits SQLite for graph traversal and metadata." to describe a
branch recall querying LanceDB and then performing SQLite graph
traversal/metadata via the branch), and ensure the page's later sentence that
already says recall is branch-mediated remains consistent with these edits.
- Around line 69-90: The docs list "Three paths" for durable-memory creation but
omit document ingestion; add a new path (or fold into the list) titled "Document
ingestion (ingest-initiated)" after the existing items, describing that
ingestion pipelines/processors can create durable memories from uploaded
documents, external datasets, or connectors, including how they create memory
entries, association/metadata, persistence semantics, and how they differ from
branch- or cortex-initiated flows; update the heading around "Three paths:" and
the surrounding text (the sections for "Branch-initiated", "Persistence
branches", and "Cortex-initiated") so the invariant that ingestion is a
durable-memory source is explicitly stated and the list/count matches the prose.
In `@docs/content/docs/`(core)/prompts.mdx:
- Around line 41-48: The docs example for render_channel_prompt is out of sync
with the actual fallback: src/prompts/engine.rs promotes memory_bulletin into
knowledge_synthesis when knowledge_synthesis is missing, and
prompts/en/channel.md.j2 only renders the "## Memory Context" block when not
working_memory and not knowledge_synthesis; update the prompt example (the
callsite showing engine.render_channel_prompt and/or the channel.md.j2 snippet)
to reflect that memory_bulletin is only shown as "Memory Context" when both
working_memory and knowledge_synthesis are absent (i.e., keep the guard
condition matching prompts/en/channel.md.j2 and mention that memory_bulletin
will be promoted to knowledge_synthesis by src/prompts/engine.rs rather than
displayed separately).
In `@docs/content/docs/`(features)/tasks.mdx:
- Around line 163-178: Update the section heading "## Bulletin Integration" to
match the rewritten terminology used in the body and module notes by renaming it
to "## Knowledge Context Integration" (replace the unique heading string "##
Bulletin Integration" in docs/content/docs/(features)/tasks.mdx), so the heading
and the paragraph about "cortex knowledge context" and the "knowledge context
integration" module note are consistent.
In `@docs/design-docs/working-memory-implementation-plan.md`:
- Around line 898-916: Update the plan to mark Phase 5b as completed instead of
future work: change the Phase 5b heading/description to indicate "Completed —
compactor no longer performs durable memory writes" and remove or strike the
“Remove Memory Save from Compactor” action items; update references in the doc
text around the current summary-only claim (lines referencing
src/agent/compactor.rs and prompts/en/compactor.md.j2) to state that
compactor.rs no longer calls durable-write and prompts/en/compactor.md.j2 no
longer contains memory_save instructions, and adjust the Verification section to
document the verification already done (integration/regression checks) rather
than prescribing them as upcoming steps.
---
Nitpick comments:
In `@docs/content/docs/`(core)/architecture.mdx:
- Around line 310-332: Replace the two remaining uses of "Bulletin" with the
standardized "knowledge synthesis" terminology: update the Cortex max-turns
table entry string 'Bulletin generation. Single-pass with tool calls.' to
'Knowledge synthesis generation. Single-pass with tool calls.' and update the
Control API endpoint group description 'Bulletin, profile, cortex chat' to
'Knowledge synthesis, profile, cortex chat' so the wording matches
cortex.mdx/configuring-channels.mdx/config.mdx.
In `@docs/design-docs/task-tracking.md`:
- Around line 128-181: Update the two leftover prose references that still use
"bulletin" to the new terminology used in this section: replace "current
bulletin" in the Todo Promotion Loop sentence with "current knowledge context"
(or "current knowledge synthesis context") and change "bulletin gathering" /
"bulletin sections" in the Knowledge Context Integration paragraph to "knowledge
context gathering" / "knowledge context sections" (the surrounding identifiers
to locate the lines are the "Todo Promotion Loop" heading and the
KnowledgeSynthesisSection struct example).
In `@docs/design-docs/wiki.md`:
- Around line 364-384: Update the Phase 1 Core bullet list: remove or relocate
the item "- Wiki index block in bulletin template" so it lives with the
knowledge context / knowledge synthesis section (Phase 2 or the new section),
and update the introductory note near the top ("Not the bulletin.") to either
reference the knowledge synthesis/knowledge context docs or explicitly mark
"bulletin" as legacy; edit the "Phase 1 — Core" block and the header text
containing "Not the bulletin." to keep terminology consistent and prevent
reintroducing "bulletin" as the primary concept.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9ccab8a8-1d7e-45c1-b49f-6ddb9a837dc7
📒 Files selected for processing (26)
docs/content/docs/(configuration)/config.mdxdocs/content/docs/(core)/architecture.mdxdocs/content/docs/(core)/channels.mdxdocs/content/docs/(core)/compaction.mdxdocs/content/docs/(core)/cortex.mdxdocs/content/docs/(core)/memory.mdxdocs/content/docs/(core)/prompts.mdxdocs/content/docs/(deployment)/roadmap.mdxdocs/content/docs/(features)/cron.mdxdocs/content/docs/(features)/tasks.mdxdocs/content/docs/(features)/tools.mdxdocs/content/docs/(features)/wiki.mdxdocs/design-docs/cortex-chat.mddocs/design-docs/cortex-history.mddocs/design-docs/cortex-implementation.mddocs/design-docs/cron-timezone-and-reliability.mddocs/design-docs/participant-awareness.mddocs/design-docs/prompt-routing.mddocs/design-docs/task-tracking.mddocs/design-docs/user-scoped-memories.mddocs/design-docs/wiki.mddocs/design-docs/working-memory-implementation-plan.mddocs/design-docs/working-memory-problem-analysis.mddocs/design-docs/working-memory.mdsrc/agent/compactor.rssrc/tools.rs
![tembo[bot]](https://avatars.githubusercontent.com/in/1223475?s=60&v=4){kind=small}
| @@ -164,7 +164,7 @@ They cannot change the task's status, priority, title, description, or worker bi | |||
|
|
|||
| ## Bulletin Integration | |||
Contributor
There was a problem hiding this comment.
This section was updated to talk about knowledge context, but the heading still says "Bulletin". Renaming the heading keeps terminology consistent.
Suggested change
| ## Bulletin Integration | |
| ## Knowledge Context Integration |
| → Initialize MessagingManager (start all platform adapters) | ||
| → Initialize CronScheduler | ||
| → Start Cortex loops (warmup, bulletin fallback, association, ready-task) | ||
| → Start Cortex loops (warmup, knowledge synthesis, association, ready-task) |
Contributor
There was a problem hiding this comment.
The startup-sequence diagram line for starting cortex loops looks accidentally indented under Initialize CronScheduler. Aligning it with the other steps makes the hierarchy clearer.
Suggested change
| → Start Cortex loops (warmup, knowledge synthesis, association, ready-task) | |
| → Start Cortex loops (warmup, knowledge synthesis, association, ready-task) |
Member
|
Hey — looked through this to unblock for 0.5. Two things to handle: Clippy failures aren't yours. They're in 4 🟡 doc nits from CodeRabbit, all valid:
Once those land plus the rebase, happy to stamp and merge this for 0.5. |
vsumner
force-pushed
the
codex/docs-memory-alignment
branch
from
1c8d77d to
929815c
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/content/docs/(core)/compaction.mdx (1)
111-117:⚠️ Potential issue | 🟡 MinorBroaden the table entry to match the documented persistence paths.
Line 51 correctly lists persistence branches, direct memory tools, ingestion, and cortex workflows. The table narrows Spacebot memory extraction to only persistence branches, which makes this section inconsistent.
Proposed wording
-| Memory extraction | Separate pass | Separate persistence branches | +| Memory extraction | Separate pass | Separate persistence paths (branches, tools, ingestion, cortex workflows) |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/content/docs/`(core)/compaction.mdx around lines 111 - 117, The Spacebot "Memory extraction" cell in the table is too narrow and should be expanded to match the documented persistence paths described earlier (persistence branches, direct memory tools, ingestion, and cortex workflows); update the Spacebot row in the table so its "Memory extraction" column lists all four paths (e.g., "Persistence branches, direct memory tools, ingestion, cortex workflows") to restore consistency with the prose and the earlier list.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/content/docs/`(core)/cortex.mdx:
- Around line 176-180: Update the docs example so the legacy bulletin defaults
match the runtime defaults defined in src/config/types.rs: change the
bulletin_max_words example from 500 to the runtime default 1500 (and keep
bulletin_interval_secs as shown), or explicitly note that the example uses a
legacy value distinct from knowledge_synthesis_max_words; locate the
bulletin_max_words/bulletin_interval_secs entries in the cortex.mdx snippet and
make the value or explanatory text consistent with the bulletin_max_words
default in types.rs.
- Around line 127-131: The documentation example uses a ToolCompleted { tool:
"memory_save" } snippet but the code actually triggers knowledge synthesis on
ProcessEvent::MemorySaved; update the example to show the MemorySaved event
(matching ProcessEvent::MemorySaved) instead of a ToolCompleted/memory_save
entry so the docs reflect the real signal name and shape processed by the cortex
(see ProcessEvent::MemorySaved and related handlers in cortex.rs).
---
Outside diff comments:
In `@docs/content/docs/`(core)/compaction.mdx:
- Around line 111-117: The Spacebot "Memory extraction" cell in the table is too
narrow and should be expanded to match the documented persistence paths
described earlier (persistence branches, direct memory tools, ingestion, and
cortex workflows); update the Spacebot row in the table so its "Memory
extraction" column lists all four paths (e.g., "Persistence branches, direct
memory tools, ingestion, cortex workflows") to restore consistency with the
prose and the earlier list.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 41d0b3d3-08dd-4e06-a776-eed3bdcbb439
📒 Files selected for processing (26)
docs/content/docs/(configuration)/config.mdxdocs/content/docs/(core)/architecture.mdxdocs/content/docs/(core)/channels.mdxdocs/content/docs/(core)/compaction.mdxdocs/content/docs/(core)/cortex.mdxdocs/content/docs/(core)/memory.mdxdocs/content/docs/(core)/prompts.mdxdocs/content/docs/(deployment)/roadmap.mdxdocs/content/docs/(features)/cron.mdxdocs/content/docs/(features)/tasks.mdxdocs/content/docs/(features)/tools.mdxdocs/content/docs/(features)/wiki.mdxdocs/design-docs/cortex-chat.mddocs/design-docs/cortex-history.mddocs/design-docs/cortex-implementation.mddocs/design-docs/cron-timezone-and-reliability.mddocs/design-docs/participant-awareness.mddocs/design-docs/prompt-routing.mddocs/design-docs/task-tracking.mddocs/design-docs/user-scoped-memories.mddocs/design-docs/wiki.mddocs/design-docs/working-memory-implementation-plan.mddocs/design-docs/working-memory-problem-analysis.mddocs/design-docs/working-memory.mdsrc/agent/compactor.rssrc/tools.rs
✅ Files skipped from review due to trivial changes (12)
- docs/design-docs/cron-timezone-and-reliability.md
- docs/design-docs/cortex-history.md
- src/tools.rs
- docs/content/docs/(features)/wiki.mdx
- src/agent/compactor.rs
- docs/content/docs/(features)/tasks.mdx
- docs/design-docs/user-scoped-memories.md
- docs/design-docs/task-tracking.md
- docs/design-docs/wiki.md
- docs/content/docs/(core)/architecture.mdx
- docs/content/docs/(core)/channels.mdx
- docs/design-docs/participant-awareness.md
🚧 Files skipped from review as they are similar to previous changes (12)
- docs/design-docs/cortex-chat.md
- docs/design-docs/cortex-implementation.md
- docs/content/docs/(deployment)/roadmap.mdx
- docs/content/docs/(features)/cron.mdx
- docs/content/docs/(features)/tools.mdx
- docs/content/docs/(core)/prompts.mdx
- docs/content/docs/(configuration)/config.mdx
- docs/design-docs/prompt-routing.md
- docs/content/docs/(core)/memory.mdx
- docs/design-docs/working-memory-problem-analysis.md
- docs/design-docs/working-memory.md
- docs/design-docs/working-memory-implementation-plan.md
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/content/docs/(configuration)/config.mdx (1)
104-110:⚠️ Potential issue | 🟡 MinorMirror the new Cortex keys in the Full Reference TOML.
Lines 541-545 document new user-facing Cortex keys, but the
[defaults.cortex]example in the “Full Reference” block still omits them. Users copying the “complete” TOML reference won’t see these knobs.📝 Proposed docs fix
[defaults.cortex] tick_interval_secs = 30 worker_timeout_secs = 600 branch_timeout_secs = 60 circuit_breaker_threshold = 3 # consecutive failures before auto-disable +bulletin_interval_secs = 3600 # legacy compatibility interval for bulletin-named synthesis surfaces +bulletin_max_words = 1500 # legacy compatibility word budget for bulletin-named synthesis surfaces +bulletin_max_turns = 15 # legacy compatibility max-turn budget for bulletin generation passes +knowledge_synthesis_max_words = 500 +knowledge_synthesis_debounce_secs = 60Also applies to: 541-545
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/content/docs/`(configuration)/config.mdx around lines 104 - 110, The Full Reference TOML example is missing the new Cortex keys; update the [defaults.cortex] example to include tick_interval_secs, worker_timeout_secs, branch_timeout_secs, and circuit_breaker_threshold with the same default values/comments shown earlier (tick_interval_secs = 30, worker_timeout_secs = 600, branch_timeout_secs = 60, circuit_breaker_threshold = 3) so the “complete” TOML mirrors the user-facing keys documented at the other location; edit the [defaults.cortex] block to add these entries and keep the explanatory comment for circuit_breaker_threshold.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/content/docs/`(configuration)/config.mdx:
- Around line 556-564: The copy currently implies warmup blocks traffic; update
the paragraph mentioning "ready before traffic" to instead state that warmup
attempts to ensure embeddings and initial knowledge synthesis are prepared
before normal traffic but does not block dispatch: keep references to the legacy
bulletin_* names and the user-facing "## Knowledge Context" prompt, retain the
readiness conditions (warmup state == 'warm', embedding ready, knowledge
synthesis generated at least once), and explicitly note that if readiness isn't
met Spacebot still dispatches, increments cold-dispatch metrics, and queues a
forced warmup in the background (preserve the term "cold-dispatch" and the
behavior described later).
In `@docs/content/docs/`(core)/compaction.mdx:
- Line 49: Update the "Inject summary" step in
docs/content/docs/(core)/compaction.mdx to explicitly state that summaries are
prepended newest-first (inserting at position 0 means the most recent summary
appears before older ones); revise any phrases like "stack chronologically" and
the examples referenced around the other occurrences (the blocks noted at ~lines
65-75 and 116-118) to reflect newest-to-oldest ordering so readers won't expect
oldest-first ordering.
In `@docs/design-docs/working-memory-implementation-plan.md`:
- Around line 907-927: Update the Phase 5b section to preserve the "compaction
is summary-only" invariant: change the status from "Completed" to a
pending/conditional state that reflects dependency on Phase 5a validation,
remove any language implying the compactor has a tool server, and delete the
rollback sentence that documents restoring compactor memory writes; verify the
text in the Phase 5b header and the subsections referencing
src/agent/compactor.rs and prompts/en/compactor.md.j2 now consistently state
that the compactor produces only a compaction summary and has no durable
memory-write path.
---
Outside diff comments:
In `@docs/content/docs/`(configuration)/config.mdx:
- Around line 104-110: The Full Reference TOML example is missing the new Cortex
keys; update the [defaults.cortex] example to include tick_interval_secs,
worker_timeout_secs, branch_timeout_secs, and circuit_breaker_threshold with the
same default values/comments shown earlier (tick_interval_secs = 30,
worker_timeout_secs = 600, branch_timeout_secs = 60, circuit_breaker_threshold =
3) so the “complete” TOML mirrors the user-facing keys documented at the other
location; edit the [defaults.cortex] block to add these entries and keep the
explanatory comment for circuit_breaker_threshold.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 78e935c6-7922-46d5-a4e3-1edec82ca119
📒 Files selected for processing (9)
docs/content/docs/(configuration)/config.mdxdocs/content/docs/(core)/architecture.mdxdocs/content/docs/(core)/compaction.mdxdocs/content/docs/(core)/cortex.mdxdocs/content/docs/(core)/memory.mdxdocs/content/docs/(core)/prompts.mdxdocs/design-docs/task-tracking.mddocs/design-docs/wiki.mddocs/design-docs/working-memory-implementation-plan.md
✅ Files skipped from review due to trivial changes (2)
- docs/design-docs/task-tracking.md
- docs/content/docs/(core)/cortex.mdx
🚧 Files skipped from review as they are similar to previous changes (4)
- docs/design-docs/wiki.md
- docs/content/docs/(core)/prompts.mdx
- docs/content/docs/(core)/architecture.mdx
- docs/content/docs/(core)/memory.mdx
Comment on lines
+556
to
564
| When warmup is enabled, it makes sure embeddings and the initial knowledge synthesis are ready before traffic. The older `bulletin_*` config and status names still exist as compatibility surfaces, but the user-facing prompt prefers `## Knowledge Context` plus the working-memory layers. | ||
|
|
||
| Dispatch readiness is derived from warmup runtime state: | ||
|
|
||
| - warmup state must be `warm` | ||
| - embedding must be ready | ||
| - bulletin age must be fresh (`<= max(60s, refresh_secs * 2)`) | ||
| - knowledge synthesis must have been generated at least once | ||
|
|
||
| When branch/worker/cron dispatch happens before readiness is satisfied, Spacebot still dispatches, increments cold-dispatch metrics, and queues a forced warmup pass in the background. |
Contributor
There was a problem hiding this comment.
Avoid implying warmup blocks all traffic.
“Ready before traffic” conflicts with Line 564, which says Spacebot still dispatches when readiness is not satisfied. The generated-at-least-once readiness condition is good; just soften the warmup guarantee.
📝 Proposed wording
-When warmup is enabled, it makes sure embeddings and the initial knowledge synthesis are ready before traffic. The older `bulletin_*` config and status names still exist as compatibility surfaces, but the user-facing prompt prefers `## Knowledge Context` plus the working-memory layers.
+When warmup is enabled, it prepares embeddings and triggers initial knowledge synthesis for dispatch readiness. The older `bulletin_*` config and status names still exist as compatibility surfaces, but the user-facing prompt prefers `## Knowledge Context` plus the working-memory layers.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| When warmup is enabled, it makes sure embeddings and the initial knowledge synthesis are ready before traffic. The older `bulletin_*` config and status names still exist as compatibility surfaces, but the user-facing prompt prefers `## Knowledge Context` plus the working-memory layers. | |
| Dispatch readiness is derived from warmup runtime state: | |
| - warmup state must be `warm` | |
| - embedding must be ready | |
| - bulletin age must be fresh (`<= max(60s, refresh_secs * 2)`) | |
| - knowledge synthesis must have been generated at least once | |
| When branch/worker/cron dispatch happens before readiness is satisfied, Spacebot still dispatches, increments cold-dispatch metrics, and queues a forced warmup pass in the background. | |
| When warmup is enabled, it prepares embeddings and triggers initial knowledge synthesis for dispatch readiness. The older `bulletin_*` config and status names still exist as compatibility surfaces, but the user-facing prompt prefers `## Knowledge Context` plus the working-memory layers. | |
| Dispatch readiness is derived from warmup runtime state: | |
| - warmup state must be `warm` | |
| - embedding must be ready | |
| - knowledge synthesis must have been generated at least once | |
| When branch/worker/cron dispatch happens before readiness is satisfied, Spacebot still dispatches, increments cold-dispatch metrics, and queues a forced warmup pass in the background. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/content/docs/`(configuration)/config.mdx around lines 556 - 564, The
copy currently implies warmup blocks traffic; update the paragraph mentioning
"ready before traffic" to instead state that warmup attempts to ensure
embeddings and initial knowledge synthesis are prepared before normal traffic
but does not block dispatch: keep references to the legacy bulletin_* names and
the user-facing "## Knowledge Context" prompt, retain the readiness conditions
(warmup state == 'warm', embedding ready, knowledge synthesis generated at least
once), and explicitly note that if readiness isn't met Spacebot still
dispatches, increments cold-dispatch metrics, and queues a forced warmup in the
background (preserve the term "cold-dispatch" and the behavior described later).
| 2. **Summarize** -- Build a transcript from the removed messages and run a one-turn Rig agent with `prompts/en/compactor.md.j2` as the system prompt. The agent produces a condensed summary preserving key decisions, active topics, commitments, emotional context, and active tasks. It discards greetings, tool call mechanics, and intermediate reasoning. | ||
|
|
||
| 3. **Extract memories** — The compaction agent has access to the `memory_save` tool. While summarizing, it identifies facts, preferences, decisions, and observations worth keeping long-term and saves them directly to the memory store. These persist independently of the conversation. | ||
| 3. **Inject summary** -- Write-lock the history again, insert the summary at position 0 as `[Compaction Summary]: ...`. Release the lock. The channel sees this summary on its next turn. |
Contributor
There was a problem hiding this comment.
Clarify that prepended summaries are newest-first.
Line 49 says the summary is inserted at position 0, so if multiple summaries remain, the newest summary appears before older summaries. The later “stack chronologically” wording/example should be adjusted or readers may expect oldest-to-newest ordering.
📝 Proposed doc wording
-| Multiple summaries | One summary replaces all | Summaries stack chronologically |
+| Multiple summaries | One summary replaces all | Summaries are prepended as rolling summaries |Also applies to: 65-75, 116-118
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/content/docs/`(core)/compaction.mdx at line 49, Update the "Inject
summary" step in docs/content/docs/(core)/compaction.mdx to explicitly state
that summaries are prepended newest-first (inserting at position 0 means the
most recent summary appears before older ones); revise any phrases like "stack
chronologically" and the examples referenced around the other occurrences (the
blocks noted at ~lines 65-75 and 116-118) to reflect newest-to-oldest ordering
so readers won't expect oldest-first ordering.
Comment on lines
+907
to
927
| ### Phase 5b: Completed — Compactor Memory Extraction Remains Sunset | ||
|
|
||
| **Dependencies:** Phase 5a deployed and validated (new persistence triggers confirmed adequate) | ||
| **Risk:** Medium — removes a memory creation path. Only deploy after Phase 5a monitoring confirms coverage. | ||
| **Risk:** Medium — validate that persistence coverage remains adequate now that the compactor is summary-only. | ||
|
|
||
| #### 5b.1 Remove Memory Save from Compactor | ||
| #### 5b.1 Keep Memory Save Out of Compactor | ||
|
|
||
| Update `src/agent/compactor.rs`: | ||
| - Remove `memory_save` from the compactor's tool server | ||
| - Verify the compaction worker has no durable memory-write access | ||
| - The compactor's only output is the compaction summary injected into history | ||
|
|
||
| Update `prompts/en/compactor.md.j2`: | ||
| - Remove all `memory_save` instructions | ||
| - Keep all `memory_save` instructions removed | ||
| - The compactor's sole responsibility is producing a compaction summary | ||
|
|
||
| #### 5b.2 Verification | ||
|
|
||
| - Integration test: run compactor, verify no `memory_save` calls in tool output | ||
| - Regression test: verify memory quality doesn't degrade — compare memories saved before/after | ||
| - Monitor: memory count growth rate should remain stable compared to Phase 5a baseline | ||
| - Verified by inspection: `src/agent/compactor.rs` no longer exposes durable memory-write access | ||
| - Verified by inspection: `prompts/en/compactor.md.j2` contains no `memory_save` instructions | ||
| - Regression coverage continues through compaction and persistence branch verification rather than a compactor durable-write path | ||
|
|
Contributor
There was a problem hiding this comment.
Keep Phase 5b and rollback wording consistent with the summary-only invariant.
This section says Phase 5b is “Completed” while still depending on future Phase 5a validation, later implies the compactor has a tool server, and the rollback line still documents restoring compactor memory writes. That weakens the PR’s invariant that compaction is summary-only.
📝 Proposed cleanup
-### Phase 5b: Completed — Compactor Memory Extraction Remains Sunset
+### Phase 5b: Compactor Memory Extraction Remains Sunset-1. Remove `memory_save` from its tool server (Phase 5)
-2. Optionally emit a `System` working memory event when compaction runs (Phase 1)
+1. Keep the compactor toolless with no durable memory-write access
+2. Optionally emit a `System` working memory event when compaction runs (Phase 1)-- **Phase 5:** Restore the prior persistence trigger only if signal-based persistence regresses. Do not restore compactor memory writes unless accepting the old context-pressure coupling as an explicit emergency rollback.
+- **Phase 5:** Restore the prior persistence trigger only if signal-based persistence regresses. Keep the compactor summary-only; durable memory writes remain in persistence paths.Also applies to: 1020-1025, 1148-1148
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/design-docs/working-memory-implementation-plan.md` around lines 907 -
927, Update the Phase 5b section to preserve the "compaction is summary-only"
invariant: change the status from "Completed" to a pending/conditional state
that reflects dependency on Phase 5a validation, remove any language implying
the compactor has a tool server, and delete the rollback sentence that documents
restoring compactor memory writes; verify the text in the Phase 5b header and
the subsections referencing src/agent/compactor.rs and
prompts/en/compactor.md.j2 now consistently state that the compactor produces
only a compaction summary and has no durable memory-write path.
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
2 participants
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.
Summary
Closes #569
Verification
rg -n "compactor.*memory_save|memory_save.*compactor|Extract memories|compaction agent has access" docs src promptsrg -n "What happens today|compactor currently|Compactor-initiated|Creation.*compaction|bulletin generation|bulletin refresh|via bulletin|Memory Context via the bulletin|maintains the memory bulletin" docs/content/docs docs/design-docsrg -n "max_turns\\(10\\)" docs src/agent/compactor.rs src/tools.rs promptsgit diff --checkjust preflightjust gate-prNote
Documentation-only changes across 26 files updating memory system architecture, compaction design, cortex behavior, and memory workflow guidance. Changes reflect the shift from bulletin-based memory context to a layered model with explicit boundaries between compaction (summarization) and memory persistence (branches, cortex, ingestion). Includes historical markers for deprecated patterns and prompt-cache alignment guidance from security review. Written by Tembo for commit 1c8d77d.