feat(cortex): add decision provenance tracking#543
feat(cortex): add decision provenance tracking#543EZotoff wants to merge 2 commits intospacedriveapp:mainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughUpdated prompt terminology from "compactors" to "compaction workers" for cross-channel memory context. Changed memory association label casing from Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
prompts/en/cortex.md.j2 (2)
107-107: Disambiguate precedence between conservative memory handling and over-capture.Rule 8 can conflict with Rule 2 unless explicitly scoped. Add “for explicit human decisions only” so behavior is deterministic.
Proposed prompt fix
-8. Human decisions are legally significant for IP attribution and compliance. When in doubt, capture a decision memory rather than letting it pass. Over-capturing is preferable to under-capturing. +8. Human decisions are legally significant for IP attribution and compliance. For explicit human decisions, when in doubt, capture a decision memory rather than letting it pass. Over-capturing is preferable to under-capturing in this decision-provenance path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@prompts/en/cortex.md.j2` at line 107, Update the Rule 8 sentence in prompts/en/cortex.md.j2 (the line that currently reads "Human decisions are legally significant for IP attribution and compliance. When in doubt, capture a decision memory rather than letting it pass. Over-capturing is preferable to under-capturing.") to explicitly scope it by appending "for explicit human decisions only" (or similar wording) so it doesn't conflict with Rule 2; ensure the revised Rule 8 clearly states that the over-capture guidance applies only to explicit human decisions to make precedence deterministic.
54-55: Assign this responsibility to compaction workers, not the compactor supervisor.“Compactor” here implies the scheduler/orchestrator is performing conversational extraction. Wording should target the worker that performs LLM analysis.
Based on learnings: “Don't make the compactor an LLM process... The LLM work happens in the compaction worker it spawns.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@prompts/en/cortex.md.j2` around lines 54 - 55, Replace the current wording that assigns Decision memory creation to the "branch or compactor" so it explicitly assigns that responsibility to the compaction worker (the LLM analysis worker), not the compactor supervisor/scheduler; update the sentence in prompts/en/cortex.md.j2 to refer to "compaction worker" (or "compactor worker/LLM worker") and add a short clarifying clause that the compactor/scheduler does not perform LLM extraction.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@prompts/en/cortex.md.j2`:
- Line 62: Update the prompt text that currently uses capitalized association
labels (`Updates` and `Contradicts`) to use the lowercase forms `updates` and
`contradicts` so it matches the memory relation parser contract; specifically
replace the inline code tokens `Updates` and `Contradicts` in the sentence "If a
new decision contradicts a prior one, create an association linking old → new
(`Updates` or `Contradicts`)." with `updates` and `contradicts`.
- Around line 54-61: The Decision memory paragraph is ambiguous about storage;
update it to instruct workers to serialize the provenance attributes into the
existing Memory object (e.g., set keys on Memory.data like decided_by, decision,
rationale, alternatives, and importance) rather than assuming dedicated fields,
and specify expected types (decided_by: string, decision: string, rationale:
string|null, alternatives: array|null, importance: number 0.0–1.0) so emitted
Memory payloads are valid for persistence and indexing.
---
Nitpick comments:
In `@prompts/en/cortex.md.j2`:
- Line 107: Update the Rule 8 sentence in prompts/en/cortex.md.j2 (the line that
currently reads "Human decisions are legally significant for IP attribution and
compliance. When in doubt, capture a decision memory rather than letting it
pass. Over-capturing is preferable to under-capturing.") to explicitly scope it
by appending "for explicit human decisions only" (or similar wording) so it
doesn't conflict with Rule 2; ensure the revised Rule 8 clearly states that the
over-capture guidance applies only to explicit human decisions to make
precedence deterministic.
- Around line 54-55: Replace the current wording that assigns Decision memory
creation to the "branch or compactor" so it explicitly assigns that
responsibility to the compaction worker (the LLM analysis worker), not the
compactor supervisor/scheduler; update the sentence in prompts/en/cortex.md.j2
to refer to "compaction worker" (or "compactor worker/LLM worker") and add a
short clarifying clause that the compactor/scheduler does not perform LLM
extraction.
🪄 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: c943ef00-9980-41ad-b086-c7ec65e334bf
📒 Files selected for processing (1)
prompts/en/cortex.md.j2
prompts/en/cortex.md.j2
Outdated
| When a human makes an explicit decision in conversation — choosing a methodology, rejecting an approach, setting a constraint — the branch or compactor should create a Decision memory with attribution: | ||
|
|
||
| - **`decided_by`**: Human's display name | ||
| - **`decision`**: What was decided | ||
| - **`rationale`**: Why (if stated or inferable) | ||
| - **`alternatives`**: What was discussed but not chosen | ||
| - **`importance`**: ≥0.8 for methodology, architecture, or constraint decisions | ||
|
|
There was a problem hiding this comment.
Clarify where provenance attributes are stored in a Decision memory.
This wording reads like dedicated fields, but persisted Memory data currently only has generic fields (not decided_by/decision/rationale/alternatives). Without explicit serialization guidance, workers may emit invalid payloads.
Proposed prompt fix
-When a human makes an explicit decision in conversation — choosing a methodology, rejecting an approach, setting a constraint — the branch or compactor should create a Decision memory with attribution:
+When a human makes an explicit decision in conversation — choosing a methodology, rejecting an approach, setting a constraint — the branch or compaction worker should create a Decision memory. Store provenance attribution in the memory `content` in a structured format:
- **`decided_by`**: Human's display name
- **`decision`**: What was decided
- **`rationale`**: Why (if stated or inferable)
- **`alternatives`**: What was discussed but not chosen
- **`importance`**: ≥0.8 for methodology, architecture, or constraint decisions📝 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.
| When a human makes an explicit decision in conversation — choosing a methodology, rejecting an approach, setting a constraint — the branch or compactor should create a Decision memory with attribution: | |
| - **`decided_by`**: Human's display name | |
| - **`decision`**: What was decided | |
| - **`rationale`**: Why (if stated or inferable) | |
| - **`alternatives`**: What was discussed but not chosen | |
| - **`importance`**: ≥0.8 for methodology, architecture, or constraint decisions | |
| When a human makes an explicit decision in conversation — choosing a methodology, rejecting an approach, setting a constraint — the branch or compaction worker should create a Decision memory. Store provenance attribution in the memory `content` in a structured format: | |
| - **`decided_by`**: Human's display name | |
| - **`decision`**: What was decided | |
| - **`rationale`**: Why (if stated or inferable) | |
| - **`alternatives`**: What was discussed but not chosen | |
| - **`importance`**: ≥0.8 for methodology, architecture, or constraint decisions |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@prompts/en/cortex.md.j2` around lines 54 - 61, The Decision memory paragraph
is ambiguous about storage; update it to instruct workers to serialize the
provenance attributes into the existing Memory object (e.g., set keys on
Memory.data like decided_by, decision, rationale, alternatives, and importance)
rather than assuming dedicated fields, and specify expected types (decided_by:
string, decision: string, rationale: string|null, alternatives: array|null,
importance: number 0.0–1.0) so emitted Memory payloads are valid for persistence
and indexing.
There was a problem hiding this comment.
Pull request overview
Adds guidance to the cortex prompt to track “decision provenance” by capturing explicit human decisions as attributed Decision memories, supporting auditing/compliance and longer-term institutional memory.
Changes:
- Add “Priority 3: Decision Provenance” instructions to
prompts/en/cortex.md.j2 - Renumber “Progression” from Priority 3 → Priority 4
- Add Rule #8 to bias toward capturing legally significant human decisions
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
prompts/en/cortex.md.j2
Outdated
| ## Priority 3: Progression | ||
| ## Priority 3: Decision Provenance | ||
|
|
||
| When a human makes an explicit decision in conversation — choosing a methodology, rejecting an approach, setting a constraint — the branch or compactor should create a Decision memory with attribution: |
There was a problem hiding this comment.
This section says the “branch or compactor should create a Decision memory”, but the compactor is not able to save memories: the compaction LLM has no tool server and the compactor prompt explicitly says it does not save memories (summarization only). This instruction will be impossible to follow as written.
Suggestion: scope this to branches / memory persistence runs (or explicitly describe an alternate mechanism the compactor can trigger, e.g., spawning a persistence branch) and avoid implying the compactor can directly create memories.
| When a human makes an explicit decision in conversation — choosing a methodology, rejecting an approach, setting a constraint — the branch or compactor should create a Decision memory with attribution: | |
| When a human makes an explicit decision in conversation — choosing a methodology, rejecting an approach, setting a constraint — the branch or another memory-persistence run should create a Decision memory with attribution. If a compactor encounters such a decision, it should surface it for a later persistence-capable run rather than trying to save the memory itself: |
prompts/en/cortex.md.j2
Outdated
| - **`alternatives`**: What was discussed but not chosen | ||
| - **`importance`**: ≥0.8 for methodology, architecture, or constraint decisions | ||
|
|
||
| If a new decision contradicts a prior one, create an association linking old → new (`Updates` or `Contradicts`). |
There was a problem hiding this comment.
The association types here are written as Updates / Contradicts, but the actual relation_type strings used by memory tools are snake_case (updates, contradicts, etc.). If an agent copies these values into tool calls, it will silently fall back to related_to, breaking the intended provenance links.
Suggestion: use the exact tool argument spellings (updates / contradicts) consistently (same as prompts/en/memory_persistence.md.j2).
prompts/en/cortex.md.j2
Outdated
| 5. Don't duplicate work. Compactors handle per-channel context management. You handle cross-channel coherence and system health. | ||
| 6. When you detect a problem you can't fix (provider down, persistent errors), log it clearly. Don't try to work around infrastructure failures. | ||
| 7. Be cheap. Most ticks should be fast programmatic checks. Save LLM reasoning for consolidation and pattern detection. | ||
| 8. Human decisions are legally significant for IP attribution and compliance. When in doubt, capture a decision memory rather than letting it pass. Over-capturing is preferable to under-capturing. |
There was a problem hiding this comment.
Rule #8 (“Over-capturing is preferable…”) conflicts with earlier guidance in this same prompt to act conservatively on memory / avoid premature merges. As written, it can be interpreted as encouraging saving decision memories even when the conversation doesn’t contain an explicit decision, which risks noisy or duplicative memories.
Suggestion: tighten the wording to keep the bias toward capture, but explicitly scope it to clear, explicit human decisions (and/or require a minimal decision statement), so it doesn’t override the general “save selectively” posture.
| 8. Human decisions are legally significant for IP attribution and compliance. When in doubt, capture a decision memory rather than letting it pass. Over-capturing is preferable to under-capturing. | |
| 8. Human decisions are legally significant for IP attribution and compliance. Capture decision memories for clear, explicit human decisions. If the conversation does not contain an explicit decision statement, do not create a decision memory just to be safe; follow the conservative memory guidance above. When a clear, explicit human decision is present and significance is uncertain, prefer capturing it rather than letting it pass. |
c9a0327 to
124be03
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/agent/channel.rs (1)
1300-1310: Consider centralizing conversation-context metadata extraction.Both batch and single-message paths now duplicate the same
SERVER_NAME/CHANNEL_NAME/CHANNEL_TOPICextraction logic. A shared helper would reduce drift risk when this context evolves.Also applies to: 1808-1818
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agent/channel.rs` around lines 1300 - 1310, Duplicate metadata extraction for SERVER_NAME/CHANNEL_NAME/CHANNEL_TOPIC is present before calling prompt_engine.render_conversation_context (e.g., where conversation_context is set); create a small helper function (e.g., extract_conversation_context_metadata) that takes the message/first item (the same type used in both single-message and batch paths) and returns the three values (Option<String> or Option<&str> as appropriate) for server_name, channel_name, and channel_topic, then replace the inline get(...).and_then(|v| v.as_str()) logic in both places (the single-message path where self.conversation_context is set and the batch path around the other duplicated block) to call this helper and pass its results into prompt_engine.render_conversation_context, keeping the existing call signature (including conversation_id.as_deref()) and error handling.src/messaging/discord.rs (1)
939-946: Trim topic text before persisting metadata.
!topic.is_empty()allows whitespace-only topics. Trim first so only meaningful values are stored.♻️ Suggested refinement
- if let Some(ref topic) = guild_channel.topic { - if !topic.is_empty() { - metadata.insert( - crate::metadata_keys::CHANNEL_TOPIC.into(), - topic.clone().into(), - ); - } - } + if let Some(topic) = guild_channel + .topic + .as_deref() + .map(str::trim) + .filter(|topic| !topic.is_empty()) + { + metadata.insert( + crate::metadata_keys::CHANNEL_TOPIC.into(), + topic.to_string().into(), + ); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/messaging/discord.rs` around lines 939 - 946, The code currently checks guild_channel.topic with !topic.is_empty() which allows whitespace-only topics; update the logic in the block handling guild_channel.topic (the variable topic and the metadata.insert call for crate::metadata_keys::CHANNEL_TOPIC) to trim the topic (e.g., let trimmed = topic.trim()) before checking emptiness and, if non-empty, insert the trimmed string into metadata instead of the original topic so only meaningful values are persisted.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/agent/channel.rs`:
- Around line 1300-1310: Duplicate metadata extraction for
SERVER_NAME/CHANNEL_NAME/CHANNEL_TOPIC is present before calling
prompt_engine.render_conversation_context (e.g., where conversation_context is
set); create a small helper function (e.g.,
extract_conversation_context_metadata) that takes the message/first item (the
same type used in both single-message and batch paths) and returns the three
values (Option<String> or Option<&str> as appropriate) for server_name,
channel_name, and channel_topic, then replace the inline get(...).and_then(|v|
v.as_str()) logic in both places (the single-message path where
self.conversation_context is set and the batch path around the other duplicated
block) to call this helper and pass its results into
prompt_engine.render_conversation_context, keeping the existing call signature
(including conversation_id.as_deref()) and error handling.
In `@src/messaging/discord.rs`:
- Around line 939-946: The code currently checks guild_channel.topic with
!topic.is_empty() which allows whitespace-only topics; update the logic in the
block handling guild_channel.topic (the variable topic and the metadata.insert
call for crate::metadata_keys::CHANNEL_TOPIC) to trim the topic (e.g., let
trimmed = topic.trim()) before checking emptiness and, if non-empty, insert the
trimmed string into metadata instead of the original topic so only meaningful
values are persisted.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c49a4313-4e47-4e15-bfed-6f01594d9df3
📒 Files selected for processing (7)
prompts/en/cortex.md.j2prompts/en/fragments/conversation_context.md.j2src/agent/channel.rssrc/api/channels.rssrc/lib.rssrc/messaging/discord.rssrc/prompts/engine.rs
✅ Files skipped from review due to trivial changes (2)
- prompts/en/fragments/conversation_context.md.j2
- src/lib.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- prompts/en/cortex.md.j2
Add Priority 3: Decision Provenance section to cortex prompt that instructs branches and compactors to create attributed Decision memories when humans make explicit decisions in conversation. Decision memories include: who decided, what was decided, rationale, alternatives considered, and elevated importance scoring. Contradicted prior decisions are linked via associations. This serves compliance, auditing, IP attribution, and institutional memory needs. Prompt-only change — no code modifications required.
124be03 to
808974e
Compare
Summary
Adds a "Decision Provenance" section to the cortex prompt that instructs branches and compactors to create attributed Decision memories when humans make explicit decisions in conversation.
Decision memories include: who decided, what was decided, rationale, alternatives considered, and elevated importance scoring. Contradicted prior decisions are linked via associations.
This serves compliance, auditing, IP attribution, and institutional memory needs.
Changes
prompts/en/cortex.md.j2Testing
Prompt-only change — no code modifications. Verified against v0.4.1.