docs: propose memory belief revision rollout#197
Conversation
WalkthroughДобавлены два новых документа — детальная спецификация и поэтапный план внедрения системы пересмотра убеждений (belief revision) для модуля памяти, описывающие дополнительные поля таблицы, поведение при записи/воспроизведении и критерии поэтапного релиза. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 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)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a roadmap and specification for evolving the memory system into a belief-revision model. The proposed changes add metadata fields to track belief status, confidence, and lineage, enabling non-lossy updates where new memories supersede older ones instead of overwriting them. The review feedback highlights several areas for clarification in the specification, including the need to define the range for confidence scores, clarify the relationship between new source fields and existing ones, and ensure consistent terminology regarding lineage and non-lossy operations.
|
|
||
| Target behavior: | ||
| - retrieve top relevant existing memories for a new write, | ||
| - classify the write as `ADD`, `NOOP`, `UPDATE`, or `INVALIDATE`, |
There was a problem hiding this comment.
Using the term 'UPDATE' in a non-lossy system can be misleading as it typically implies in-place mutation. Since the goal is to create a new record while preserving history, 'REVISE' or 'SUPERSEDE' would be more accurate.
| - classify the write as `ADD`, `NOOP`, `UPDATE`, or `INVALIDATE`, | |
| - classify the write as ADD, NOOP, REVISE, or INVALIDATE, |
|
|
||
| | Column | Type intent | Purpose | | ||
| |---|---|---| | ||
| | `status` | enum/text | Current belief state of the memory | |
There was a problem hiding this comment.
The spec should define how the new status field interacts with the existing deleted_at soft-delete mechanism. It is unclear if an invalidated memory should also be soft-deleted or if they represent different layers of visibility.
| |---|---|---| | ||
| | `status` | enum/text | Current belief state of the memory | | ||
| | `status_reason` | text | Human/audit explanation for the current state | | ||
| | `confidence_score` | float | Current trust score for retrieval and re-verification | |
There was a problem hiding this comment.
The valid range for confidence_score (e.g., 0.0 to 1.0) is not specified. Defining this range is necessary to ensure consistent scoring logic across different components.
| | `confidence_score` | float | Current trust score for retrieval and re-verification | | ||
| | `valid_until` | timestamp nullable | End of the currently trusted validity window | | ||
| | `last_confirmed_at` | timestamp nullable | Last time the memory was explicitly or implicitly reconfirmed | | ||
| | `source_type` | enum/text | Provenance category used for trust initialization and later analysis | |
There was a problem hiding this comment.
The introduction of source_type and source_ref creates overlap with the existing source_agent field. The spec should clarify the intended relationship between these fields (e.g., if source_agent is being deprecated).
| | `last_confirmed_at` | timestamp nullable | Last time the memory was explicitly or implicitly reconfirmed | | ||
| | `source_type` | enum/text | Provenance category used for trust initialization and later analysis | | ||
| | `source_ref` | text nullable | Pointer to the originating source, tool call, document, issue, or session | | ||
| | `supersedes_id` | self-reference nullable | Links a new memory revision to the older memory it supersedes | |
There was a problem hiding this comment.
The type intent for supersedes_id should explicitly match the ID column type (int64) to ensure schema consistency.
| | `supersedes_id` | self-reference nullable | Links a new memory revision to the older memory it supersedes | | |
| | supersedes_id | int64 nullable | Links a new memory revision to the older memory it supersedes | |
| - These columns are **additive** and should default safely so existing reads keep working. | ||
| - `confidence_score` should be initialized deterministically from source class and rollout defaults. | ||
| - `valid_until` is nullable because not every memory has a known expiry horizon. | ||
| - `supersedes_id` should point from the newer belief to the prior one, preserving forward lineage without destroying history. |
There was a problem hiding this comment.
The description of supersedes_id pointing to a prior record describes backward lineage, not forward lineage. To avoid confusion in the data model, the terminology should be corrected.
| - `supersedes_id` should point from the newer belief to the prior one, preserving forward lineage without destroying history. | |
| - supersedes_id should point from the newer belief to the prior one, preserving historical lineage without destroying history. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
.agent/specs/memory-belief-revision.md (1)
141-153: Добавьте явные требования по constraints и индексам для новых полей.Для
supersedes_idи retrieval/write-path фильтров лучше закрепить: FK-поведение, DB defaults и индексы, иначе поведение может быть корректным логически, но нестабильным в эксплуатации.Предложение по усилению acceptance criteria
## Important schema notes @@ - `supersedes_id` should point from the newer belief to the prior one, preserving forward lineage without destroying history. - Retrieval-time decay must **not** be stored as a column. +- `supersedes_id` should be backed by a self-referential FK policy (or explicitly documented alternative) to prevent orphan lineage. +- New non-nullable belief fields must have DB-level safe defaults to protect existing writers. +- Add indexes required by Phase 1/2 access patterns (lineage traversal and belief-state filtering/ranking). ## Acceptance Criteria @@ - [ ] Migration adds the new belief-revision columns to `memories` without breaking existing clients or reads. +- [ ] DB-level defaults/constraints are defined so legacy write paths remain safe without payload changes. +- [ ] Required indexes for candidate lookup and recall ranking are present and validated via query plans.Also applies to: 165-172, 205-217
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agent/specs/memory-belief-revision.md around lines 141 - 153, Add explicit DB constraints, defaults and indexes for the new memory columns: declare enums/NOT NULL where appropriate for status and source_type, add sensible DB defaults for confidence_score and status (e.g., 0.0 and 'unknown'), mark timestamps (valid_until, last_confirmed_at) nullable but add default CURRENT_TIMESTAMP for creation/updated tracking where needed, and add length/NOT NULL constraints for status_reason and source_ref if required; for supersedes_id add a foreign key referencing the same table with ON DELETE SET NULL (or ON DELETE CASCADE per design), enforce foreign key constraint and create an index on supersedes_id; also add composite/partial indexes used in retrieval/write-path filters (e.g., indexes on (status, valid_until), (confidence_score DESC), and source_type/source_ref) to ensure stable query performance and correct DB behavior during retrieval and updates..agent/plans/memory-belief-revision-roadmap.md (1)
52-64: Зафиксируйте индексную стратегию как часть rollout-критериев.Для bounded candidate lookup и trust-aware recall стоит заранее прописать минимальные индексы/проверки плана запроса; иначе Phase 1/2 могут деградировать на росте данных.
Предложение по дополнению
Target behavior: - retrieve top relevant existing memories for a new write, @@ - initialize `confidence_score`, `source_type`, `source_ref`, and `last_confirmed_at` consistently. +- validate query performance with supporting indexes for new access paths + (e.g., status/validity/lineage filters used in write-time candidate lookup and recall ranking). ### Exit Criteria @@ - The ranking formula is inspectable enough to support debugging and tuning. +- Query plans for write-time candidate lookup and recall ranking are stable under expected dataset size.Also applies to: 94-101
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agent/plans/memory-belief-revision-roadmap.md around lines 52 - 64, Добавьте в критерии rollout явную индексную стратегию для bounded write-time candidate lookup и trust-aware recall: перечислите минимальные индексы/ключи и предикаты (например по embedding shard, updated_at, source_type) которые обязаны существовать перед включением Phase 1/2, укажите ожидаемые SLO/latency и max candidate window size, и добавьте проверку отката (health check) при росте данных; при этом сохраните текущие поля/механизмы (bounded write-time candidate lookup, классификация в ADD/NOOP/UPDATE/INVALIDATE, создание supersedes_id и переход старых записей через status/status_reason/valid_until, и инициализацию confidence_score/source_type/source_ref/last_confirmed_at) — только задокументируйте требования к индексам и пред-условиям rollout и добавьте автоматическую валидацию этих индексов как часть rollout gating.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agent/plans/memory-belief-revision-roadmap.md:
- Around line 33-39: Добавьте в Phase 0 явную задачу обновить GORM-модель и
конвертер: расширьте структуру Memory в internal/db/gorm/models.go (структура
Memory) чтобы включить новые поля, обновите методы Create в
internal/db/gorm/memory_store.go (вставки/обновления на строках, где формируется
INSERT/UPDATE) чтобы эти колонки писались при INSERT/UPDATE, и исправьте
memoryRowToModel в internal/db/gorm/memory_store.go чтобы новые колонки мапились
в API-модель; также отметьте необходимость миграции/бэфилла для существующих
строк и сделать чтение устойчивым к null/дефолтам.
In @.agent/specs/memory-belief-revision.md:
- Around line 150-152: Добавьте в спецификацию чёткие privacy-ограничения для
поля source_ref (и аналогичных мест в документе), описав допустимые форматы,
валидацию и маскирование: укажите разрешённые типов значений (например,
opaque-id, hashed-URL, or tool-call-id), ограничение длины, запрет на хранение
raw tokens/credentials, правило автодетекции и редактирования чувствительных
данных (redact on save / store only hashes), и пример валидатора/normalizer;
привяжите эти правила явно к полям source_ref (и упоминаниям рядом с source_type
и supersedes_id) чтобы редакторы/интеграции применяли маскирование и валидацию
до записи.
- Around line 29-30: Уберите действие `DELETE` из вокабуляра действий в описании
(вместо "ADD / UPDATE / DELETE / NOOP" используйте например "ADD / UPDATE /
INVALIDATE / NOOP" или "ADD / UPDATE / NOOP" с явным описанием операции
invalidation) и скорректируйте тексты, которые сейчас смешивают `DELETE` и
последующий `INVALIDATE` (включая упоминания на строках 91–95), чтобы однозначно
задать семантику non-lossy модели Engram: описать, что удаление не происходит, а
запись помечается как невалидная/инвалидирована и сохраняется; обновите все
ссылки на `DELETE` в документе на выбранную альтернативу и добавьте краткое
определение поведения `INVALIDATE`/mark-as-invalid.
---
Nitpick comments:
In @.agent/plans/memory-belief-revision-roadmap.md:
- Around line 52-64: Добавьте в критерии rollout явную индексную стратегию для
bounded write-time candidate lookup и trust-aware recall: перечислите
минимальные индексы/ключи и предикаты (например по embedding shard, updated_at,
source_type) которые обязаны существовать перед включением Phase 1/2, укажите
ожидаемые SLO/latency и max candidate window size, и добавьте проверку отката
(health check) при росте данных; при этом сохраните текущие поля/механизмы
(bounded write-time candidate lookup, классификация в
ADD/NOOP/UPDATE/INVALIDATE, создание supersedes_id и переход старых записей
через status/status_reason/valid_until, и инициализацию
confidence_score/source_type/source_ref/last_confirmed_at) — только
задокументируйте требования к индексам и пред-условиям rollout и добавьте
автоматическую валидацию этих индексов как часть rollout gating.
In @.agent/specs/memory-belief-revision.md:
- Around line 141-153: Add explicit DB constraints, defaults and indexes for the
new memory columns: declare enums/NOT NULL where appropriate for status and
source_type, add sensible DB defaults for confidence_score and status (e.g., 0.0
and 'unknown'), mark timestamps (valid_until, last_confirmed_at) nullable but
add default CURRENT_TIMESTAMP for creation/updated tracking where needed, and
add length/NOT NULL constraints for status_reason and source_ref if required;
for supersedes_id add a foreign key referencing the same table with ON DELETE
SET NULL (or ON DELETE CASCADE per design), enforce foreign key constraint and
create an index on supersedes_id; also add composite/partial indexes used in
retrieval/write-path filters (e.g., indexes on (status, valid_until),
(confidence_score DESC), and source_type/source_ref) to ensure stable query
performance and correct DB behavior during retrieval and updates.
🪄 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: dca88f1d-422b-49ef-9d78-d847cd4c77af
📒 Files selected for processing (2)
.agent/plans/memory-belief-revision-roadmap.md.agent/specs/memory-belief-revision.md
| Implementation intent for this phase: | ||
| - add migration(s) for the new columns, | ||
| - backfill safe defaults for existing rows where required, | ||
| - keep read paths tolerant of null or default values, | ||
| - define initial status vocabulary and lifecycle semantics, | ||
| - ensure retrieval-time decay remains a computed concept only. | ||
|
|
There was a problem hiding this comment.
Уточните обязательное обновление GORM-модели и конвертера в Phase 0.
Сейчас в плане нет явного шага обновить internal/db/gorm/models.go (структура Memory, строки 302-320) и internal/db/gorm/memory_store.go (Create, строки 28-60, и memoryRowToModel, строки 169-183). Без этого новые колонки могут не участвовать в INSERT/UPDATE и не попадать в API-модель, что ломает заявленную совместимость по факту.
Предложение по правке roadmap
Implementation intent for this phase:
- add migration(s) for the new columns,
- backfill safe defaults for existing rows where required,
- keep read paths tolerant of null or default values,
+- update `internal/db/gorm/models.go` `Memory` row struct to include all new columns,
+- update row↔model mapping in `internal/db/gorm/memory_store.go` (including `memoryRowToModel`) so new fields are persisted and retrievable,
- define initial status vocabulary and lifecycle semantics,
- ensure retrieval-time decay remains a computed concept only.Also applies to: 42-46
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agent/plans/memory-belief-revision-roadmap.md around lines 33 - 39,
Добавьте в Phase 0 явную задачу обновить GORM-модель и конвертер: расширьте
структуру Memory в internal/db/gorm/models.go (структура Memory) чтобы включить
новые поля, обновите методы Create в internal/db/gorm/memory_store.go
(вставки/обновления на строках, где формируется INSERT/UPDATE) чтобы эти колонки
писались при INSERT/UPDATE, и исправьте memoryRowToModel в
internal/db/gorm/memory_store.go чтобы новые колонки мапились в API-модель;
также отметьте необходимость миграции/бэфилла для существующих строк и сделать
чтение устойчивым к null/дефолтам.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.agent/specs/memory-belief-revision.md (1)
174-176: Зафиксируйте DB-ограничения дляsupersedes_idявно в спецификации.Сейчас описано назначение поля, но не зафиксированы минимальные гарантии целостности lineage. Рекомендую явно добавить: FK
supersedes_id -> memories.id,CHECK (supersedes_id IS NULL OR supersedes_id <> id)и правило запрета soft-delete родителя, пока на него ссылаются активные ревизии.Предложение правки текста
- `supersedes_id` should be nullable `int64` and point from the newer belief row to the prior belief row it supersedes. This is a self-reference on `memories.id`, making lineage direction explicit: child revision -> superseded parent. +- `supersedes_id` should enforce lineage integrity at DB level: add a foreign key to `memories(id)`, a self-link guard (`supersedes_id <> id`), and a documented policy for parent retirement (e.g., prevent retiring referenced active lineage rows).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agent/specs/memory-belief-revision.md around lines 174 - 176, Добавьте в спецификацию явные DB-ограничения для поля supersedes_id: задокументируйте внешнюю ссылку FK supersedes_id -> memories.id, добавьте проверку целостности CHECK (supersedes_id IS NULL OR supersedes_id <> id) и опишите правило/триггер, запрещающий soft-delete строки-родителя, пока на неё ссылаются активные ревизии (т.е. пока существуют записи с supersedes_id = parent id); укажите также, что эти ограничения относятся к таблице memories и к логике вставки/обновления/удаления, чтобы явно зафиксировать гарантию непорочности lineage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agent/plans/memory-belief-revision-roadmap.md:
- Around line 45-47: Replace the phrase "Existing `recall_memory` clients
continue to return results unchanged in shape." with wording that requires a
backward-compatible shape: state that existing response fields and their
semantics for `recall_memory` must remain unchanged while additive, optional
fields are allowed; likewise clarify `store_memory` behavior stays the same and
new rows get deterministic defaults for belief-revision metadata. Ensure the
text explicitly uses "backward-compatible" and mentions that extensions (extra
fields) are permitted but existing fields and semantics must not be modified.
---
Nitpick comments:
In @.agent/specs/memory-belief-revision.md:
- Around line 174-176: Добавьте в спецификацию явные DB-ограничения для поля
supersedes_id: задокументируйте внешнюю ссылку FK supersedes_id -> memories.id,
добавьте проверку целостности CHECK (supersedes_id IS NULL OR supersedes_id <>
id) и опишите правило/триггер, запрещающий soft-delete строки-родителя, пока на
неё ссылаются активные ревизии (т.е. пока существуют записи с supersedes_id =
parent id); укажите также, что эти ограничения относятся к таблице memories и к
логике вставки/обновления/удаления, чтобы явно зафиксировать гарантию
непорочности lineage.
🪄 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: 427c612a-de21-4949-8cb6-c235155fbf8e
📒 Files selected for processing (2)
.agent/plans/memory-belief-revision-roadmap.md.agent/specs/memory-belief-revision.md
| - Existing `store_memory` clients continue to succeed unchanged. | ||
| - Existing `recall_memory` clients continue to return results unchanged in shape. | ||
| - New rows receive deterministic defaults for belief-revision metadata. |
There was a problem hiding this comment.
Смягчите критерий “unchanged in shape” до “backward-compatible shape”.
Фраза “unchanged in shape” может заблокировать безопасные аддитивные поля ответа. Лучше зафиксировать, что существующие поля и их семантика не меняются, а расширения допустимы.
Предложение правки текста
-- Existing `recall_memory` clients continue to return results unchanged in shape.
+- Existing `recall_memory` clients continue to return backward-compatible results (no breaking field removals/semantic changes; additive fields allowed).🧰 Tools
🪛 LanguageTool
[style] ~46-~46: This wording could be more concise.
Context: ...ory` clients continue to return results unchanged in shape. - New rows receive deterministic defau...
(ADJECTIVE_IN_ATTRIBUTE)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agent/plans/memory-belief-revision-roadmap.md around lines 45 - 47, Replace
the phrase "Existing `recall_memory` clients continue to return results
unchanged in shape." with wording that requires a backward-compatible shape:
state that existing response fields and their semantics for `recall_memory` must
remain unchanged while additive, optional fields are allowed; likewise clarify
`store_memory` behavior stays the same and new rows get deterministic defaults
for belief-revision metadata. Ensure the text explicitly uses
"backward-compatible" and mentions that extensions (extra fields) are permitted
but existing fields and semantics must not be modified.
1 participant
Summary
store_memory/recall_memoryIssue
Artifacts
.agent/specs/memory-belief-revision.md.agent/plans/memory-belief-revision-roadmap.mdVerified basis
.agent/Summary by CodeRabbit
Примечания к выпуску