memory-file-format完了
This commit is contained in:
parent
f2e47629d0
commit
cd8b620e6a
1
TODO.md
1
TODO.md
|
|
@ -13,7 +13,6 @@
|
|||
- [ ] TUI 補完 + 型付き atom 化 → [tickets/submit-tui-completion.md](tickets/submit-tui-completion.md)
|
||||
- [ ] セッションログの Segment 保持 → [tickets/session-log-segments.md](tickets/session-log-segments.md)
|
||||
- [ ] メモリ機構
|
||||
- [ ] ファイル形式 + Linter 土台 → [tickets/memory-file-format.md](tickets/memory-file-format.md)
|
||||
- [ ] memory / Knowledge 検索ツール → [tickets/memory-search-tools.md](tickets/memory-search-tools.md)
|
||||
- [ ] `model_invokation: ON` の常駐注入 → [tickets/memory-resident-injection.md](tickets/memory-resident-injection.md)
|
||||
- [ ] Phase 1 活動抽出 → [tickets/memory-phase1-extract.md](tickets/memory-phase1-extract.md)
|
||||
|
|
|
|||
|
|
@ -126,13 +126,18 @@ impl Linter {
|
|||
}
|
||||
}
|
||||
|
||||
// Similar-slug clustering warning. Skipped for Summary (no slug).
|
||||
if let Some(slug) = &classified.slug {
|
||||
warnings::check_similar_slugs(slug, classified.kind, &existing, &mut report);
|
||||
}
|
||||
|
||||
// Frontmatter parse dispatch by kind.
|
||||
match classified.kind {
|
||||
RecordKind::Decision => {
|
||||
self.check_decision(content, &classified, &existing, &mut report);
|
||||
}
|
||||
RecordKind::Request => {
|
||||
self.check_kind::<RequestFrontmatter>(content, &classified, &mut report);
|
||||
self.check_request(content, &classified, &mut report);
|
||||
}
|
||||
RecordKind::Knowledge => {
|
||||
self.check_knowledge(content, &classified, &mut report);
|
||||
|
|
@ -163,6 +168,22 @@ impl Linter {
|
|||
let _ = parsed.frontmatter; // discarded after structural checks
|
||||
}
|
||||
|
||||
fn check_request(&self, content: &str, _cp: &ClassifiedPath, report: &mut LintReport) {
|
||||
let parsed = match parse_frontmatter::<RequestFrontmatter>(content) {
|
||||
Ok(p) => p,
|
||||
Err(e) => {
|
||||
report.push_error(e);
|
||||
return;
|
||||
}
|
||||
};
|
||||
size::check_body::<RequestFrontmatter>(parsed.body, report);
|
||||
warnings::check_warnings_with_sources(
|
||||
parsed.body,
|
||||
parsed.frontmatter.sources.len(),
|
||||
report,
|
||||
);
|
||||
}
|
||||
|
||||
fn check_decision(
|
||||
&self,
|
||||
content: &str,
|
||||
|
|
@ -229,12 +250,45 @@ impl Linter {
|
|||
}
|
||||
}
|
||||
|
||||
/// Workflow frontmatter validator exposed for human-edit paths
|
||||
/// (CLI / pre-commit). Not used by the memory tool, which rejects
|
||||
/// workflow writes outright.
|
||||
pub fn lint_workflow_frontmatter(content: &str) -> Result<WorkflowFrontmatter, LintError> {
|
||||
let parsed = parse_frontmatter::<WorkflowFrontmatter>(content)?;
|
||||
Ok(parsed.frontmatter)
|
||||
impl Linter {
|
||||
/// Workflow record validator exposed for human-edit paths
|
||||
/// (CLI / pre-commit). Not used by the memory tool, which rejects
|
||||
/// workflow writes outright.
|
||||
///
|
||||
/// Verifies frontmatter shape, body size, and that every slug in
|
||||
/// `requires` points at an existing Knowledge record under the
|
||||
/// workspace's `knowledge/` directory.
|
||||
pub fn lint_workflow(&self, content: &str) -> LintReport {
|
||||
let mut report = LintReport::default();
|
||||
let parsed = match parse_frontmatter::<WorkflowFrontmatter>(content) {
|
||||
Ok(p) => p,
|
||||
Err(e) => {
|
||||
report.push_error(e);
|
||||
return report;
|
||||
}
|
||||
};
|
||||
size::check_body::<WorkflowFrontmatter>(parsed.body, &mut report);
|
||||
|
||||
let existing = match existing::scan_existing(&self.layout) {
|
||||
Ok(e) => e,
|
||||
Err(e) => {
|
||||
report.push_error(LintError::MalformedFrontmatter(format!(
|
||||
"failed to scan existing records: {e}"
|
||||
)));
|
||||
return report;
|
||||
}
|
||||
};
|
||||
for slug in &parsed.frontmatter.requires {
|
||||
if !existing.contains(crate::workspace::RecordKind::Knowledge, slug) {
|
||||
report.push_error(LintError::UnknownReference {
|
||||
field: "requires",
|
||||
kind: "knowledge",
|
||||
slug: slug.to_string(),
|
||||
});
|
||||
}
|
||||
}
|
||||
report
|
||||
}
|
||||
}
|
||||
|
||||
struct Parsed<'a, F> {
|
||||
|
|
@ -417,6 +471,115 @@ mod tests {
|
|||
assert!(report.errors.iter().any(|e| matches!(e, LintError::SlugAlreadyExists(_))));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn workflow_lint_accepts_valid_record() {
|
||||
let (dir, linter) = workspace();
|
||||
// Place a Knowledge record that the workflow will reference.
|
||||
let kn = dir.path().join("knowledge/foo.md");
|
||||
write(
|
||||
&kn,
|
||||
&format!(
|
||||
"---\ncreated_at: {n}\nupdated_at: {n}\nkind: rule\ndescription: x\nmodel_invokation: false\nuser_invocable: true\nlast_sources: []\n---\n",
|
||||
n = iso_now()
|
||||
),
|
||||
);
|
||||
let wf = format!(
|
||||
"---\nupdated_at: {n}\ndescription: do thing\nauto_invoke: false\nuser_invocable: true\nrequires: [foo]\n---\nstep 1\n",
|
||||
n = iso_now()
|
||||
);
|
||||
let report = linter.lint_workflow(&wf);
|
||||
assert!(!report.has_errors(), "got errors: {:?}", report.errors);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn workflow_lint_flags_unknown_requires() {
|
||||
let (_dir, linter) = workspace();
|
||||
let wf = format!(
|
||||
"---\nupdated_at: {n}\ndescription: x\nauto_invoke: false\nuser_invocable: true\nrequires: [missing-knowledge]\n---\n",
|
||||
n = iso_now()
|
||||
);
|
||||
let report = linter.lint_workflow(&wf);
|
||||
assert!(report.errors.iter().any(|e| matches!(
|
||||
e,
|
||||
LintError::UnknownReference {
|
||||
field: "requires",
|
||||
kind: "knowledge",
|
||||
..
|
||||
}
|
||||
)));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn workflow_lint_collects_multiple_unknown_requires() {
|
||||
let (_dir, linter) = workspace();
|
||||
let wf = format!(
|
||||
"---\nupdated_at: {n}\ndescription: x\nauto_invoke: false\nuser_invocable: true\nrequires: [a, b, c]\n---\n",
|
||||
n = iso_now()
|
||||
);
|
||||
let report = linter.lint_workflow(&wf);
|
||||
let unknown_count = report
|
||||
.errors
|
||||
.iter()
|
||||
.filter(|e| matches!(e, LintError::UnknownReference { .. }))
|
||||
.count();
|
||||
assert_eq!(unknown_count, 3);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn similar_slugs_warns_on_cluster() {
|
||||
let (dir, linter) = workspace();
|
||||
// Two existing decisions within Levenshtein 2 of `db-pool`:
|
||||
// `db-pol` (1 deletion), `db-pools` (1 insertion).
|
||||
for slug in ["db-pol", "db-pools"] {
|
||||
write(
|
||||
&dir.path().join(format!("memory/decisions/{slug}.md")),
|
||||
&format!(
|
||||
"---\ncreated_at: {n}\nupdated_at: {n}\nsources: []\nstatus: open\n---\n",
|
||||
n = iso_now()
|
||||
),
|
||||
);
|
||||
}
|
||||
let path = dir.path().join("memory/decisions/db-pool.md");
|
||||
let content = format!(
|
||||
"---\ncreated_at: {n}\nupdated_at: {n}\nsources: []\nstatus: open\n---\nbody\n",
|
||||
n = iso_now()
|
||||
);
|
||||
let report = linter.lint(&path, &content, WriteMode::Create);
|
||||
let warned = report
|
||||
.warnings
|
||||
.iter()
|
||||
.any(|w| matches!(w, LintWarning::SimilarSlugs(slugs) if slugs.len() >= 3));
|
||||
assert!(warned, "expected SimilarSlugs warning, got {:?}", report.warnings);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn similar_slugs_silent_when_distant() {
|
||||
let (dir, linter) = workspace();
|
||||
for slug in ["alpha", "bravo"] {
|
||||
write(
|
||||
&dir.path().join(format!("memory/decisions/{slug}.md")),
|
||||
&format!(
|
||||
"---\ncreated_at: {n}\nupdated_at: {n}\nsources: []\nstatus: open\n---\n",
|
||||
n = iso_now()
|
||||
),
|
||||
);
|
||||
}
|
||||
let path = dir.path().join("memory/decisions/charlie.md");
|
||||
let content = format!(
|
||||
"---\ncreated_at: {n}\nupdated_at: {n}\nsources: []\nstatus: open\n---\n",
|
||||
n = iso_now()
|
||||
);
|
||||
let report = linter.lint(&path, &content, WriteMode::Create);
|
||||
assert!(
|
||||
!report
|
||||
.warnings
|
||||
.iter()
|
||||
.any(|w| matches!(w, LintWarning::SimilarSlugs(_))),
|
||||
"unexpected SimilarSlugs warning: {:?}",
|
||||
report.warnings
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn body_size_limit_errors() {
|
||||
let (dir, linter) = workspace();
|
||||
|
|
|
|||
|
|
@ -6,10 +6,17 @@
|
|||
|
||||
use crate::error::LintWarning;
|
||||
use crate::linter::LintReport;
|
||||
use crate::workspace::ClassifiedPath;
|
||||
use crate::linter::existing::ExistingRecords;
|
||||
use crate::slug::Slug;
|
||||
use crate::workspace::{ClassifiedPath, RecordKind};
|
||||
|
||||
const LARGE_BODY_THRESHOLD: usize = 1500;
|
||||
const SOURCES_OVERFLOW_THRESHOLD: usize = 10;
|
||||
const SIMILAR_SLUG_DISTANCE: usize = 2;
|
||||
/// Cluster size (including the new slug) at which the similar-slug
|
||||
/// warning fires. 3 follows `docs/plan/memory.md` §Linter (`類似 slug
|
||||
/// 乱立`) — two existing close neighbours plus the new write.
|
||||
const SIMILAR_SLUG_CLUSTER_MIN: usize = 3;
|
||||
|
||||
/// For kinds that don't carry a `sources` array (Summary), emit only
|
||||
/// the body-size warning.
|
||||
|
|
@ -32,3 +39,70 @@ pub fn check_warnings_with_sources(body: &str, source_count: usize, report: &mut
|
|||
});
|
||||
}
|
||||
}
|
||||
|
||||
/// Emit a `SimilarSlugs` warning when the proposed slug joins a cluster
|
||||
/// of `SIMILAR_SLUG_CLUSTER_MIN` or more slugs in the same kind that
|
||||
/// are pairwise within `SIMILAR_SLUG_DISTANCE` Levenshtein steps of the
|
||||
/// new one. The reported list includes the new slug, sorted to keep
|
||||
/// the warning text deterministic.
|
||||
pub fn check_similar_slugs(
|
||||
new_slug: &Slug,
|
||||
kind: RecordKind,
|
||||
existing: &ExistingRecords,
|
||||
report: &mut LintReport,
|
||||
) {
|
||||
let mut neighbours: Vec<String> = existing
|
||||
.slugs(kind)
|
||||
.into_iter()
|
||||
.filter(|s| *s != new_slug)
|
||||
.filter(|s| levenshtein(new_slug.as_str(), s.as_str()) <= SIMILAR_SLUG_DISTANCE)
|
||||
.map(|s| s.to_string())
|
||||
.collect();
|
||||
if neighbours.len() + 1 < SIMILAR_SLUG_CLUSTER_MIN {
|
||||
return;
|
||||
}
|
||||
neighbours.push(new_slug.to_string());
|
||||
neighbours.sort();
|
||||
report.push_warning(LintWarning::SimilarSlugs(neighbours));
|
||||
}
|
||||
|
||||
/// Iterative two-row Levenshtein distance over chars.
|
||||
fn levenshtein(a: &str, b: &str) -> usize {
|
||||
let a: Vec<char> = a.chars().collect();
|
||||
let b: Vec<char> = b.chars().collect();
|
||||
if a.is_empty() {
|
||||
return b.len();
|
||||
}
|
||||
if b.is_empty() {
|
||||
return a.len();
|
||||
}
|
||||
let mut prev: Vec<usize> = (0..=b.len()).collect();
|
||||
let mut curr: Vec<usize> = vec![0; b.len() + 1];
|
||||
for (i, ca) in a.iter().enumerate() {
|
||||
curr[0] = i + 1;
|
||||
for (j, cb) in b.iter().enumerate() {
|
||||
let cost = if ca == cb { 0 } else { 1 };
|
||||
curr[j + 1] = (curr[j] + 1)
|
||||
.min(prev[j + 1] + 1)
|
||||
.min(prev[j] + cost);
|
||||
}
|
||||
std::mem::swap(&mut prev, &mut curr);
|
||||
}
|
||||
prev[b.len()]
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn levenshtein_basics() {
|
||||
assert_eq!(levenshtein("", ""), 0);
|
||||
assert_eq!(levenshtein("a", ""), 1);
|
||||
assert_eq!(levenshtein("", "ab"), 2);
|
||||
assert_eq!(levenshtein("kitten", "sitting"), 3);
|
||||
assert_eq!(levenshtein("foo", "foo"), 0);
|
||||
assert_eq!(levenshtein("abc", "abd"), 1);
|
||||
assert_eq!(levenshtein("abcd", "acbd"), 2);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -193,7 +193,7 @@ GC Agent は **drop / merge / split を自律実行**（削除まで含む）。
|
|||
- **ファイル単位**: 丸ごと drop、複数ファイルの merge、1 ファイルの分割（split）
|
||||
- **ファイル内の部分削除**: 本文の一部節・箇条を削除 or 圧縮。frontmatter の `sources` 古いエントリの trim も含む
|
||||
|
||||
Phase 2 と同じ CRUD tool + Linter Hook を使うので、operation 粒度は自然にサポートされる（専用 API は用意しない）。
|
||||
Phase 2 と同じ memory 専用 Tool（read / write / edit、内部で pre-write Linter）を使うので、operation 粒度は自然にサポートされる（専用 API は用意しない）。
|
||||
|
||||
#### GC の評価カテゴリ
|
||||
|
||||
|
|
|
|||
|
|
@ -1,141 +0,0 @@
|
|||
# メモリ機構: ファイル形式 + Linter 土台
|
||||
|
||||
## 背景
|
||||
|
||||
`docs/plan/memory.md` で決めたメモリ機構の永続化レイヤの土台。`memory/*` と `knowledge/*` の record を保存・編集する際の静的スキーマと、書き込み時の Linter を成立させる。Phase 1/2、検索ツール、常駐注入、GC はすべてこの層に乗る。
|
||||
|
||||
Workflow（`docs/plan/workflow.md`）も同じ frontmatter / Linter 経路で扱うため、`memory/workflow/<slug>.md` の frontmatter 検証と書き込み制限も本チケットに含める。実行経路（`/<slug>` dispatch）は別。
|
||||
|
||||
## 設計方針
|
||||
|
||||
### memory クレートに集約
|
||||
|
||||
memory 関連は新規 `crates/memory/` に全部閉じ込める。`tools` クレートや `pod` 層に memory 由来のコードを漏らさない。
|
||||
|
||||
- schema（frontmatter 型）、slug 文法、Linter ルール、Tool 実装、ワークスペース解決を全部 `memory` クレート内に置く
|
||||
- `tools::write_tool` / `edit_tool` には一切手を入れない
|
||||
- LLM への違反伝達は memory tool が `ToolError::InvalidArgument` を返すことで自然に成立。Interceptor 拡張・retry message 注入・違反カウンタは持たない
|
||||
- 「N 回失敗で abort」は worker 層の max iteration に委ね、memory 固有のカウンタは設けない
|
||||
|
||||
### memory 専用 Tool（汎用 CRUD ではない）
|
||||
|
||||
`memory` クレートが `read_tool` / `write_tool` / `edit_tool` の 3 種を提供する。これらは `<workspace>/memory/`、`<workspace>/knowledge/` 配下のみを対象とし、write/edit は **fs 書き込み前** に Linter を通して違反は `ToolError::InvalidArgument` で返す。
|
||||
|
||||
`docs/plan/memory.md` の「同じ汎用 CRUD」記述は本チケットで `memory` 専用 Tool 方式に書き換える（汎用 CRUD は memory ディレクトリには触らせない）。
|
||||
|
||||
### Pod 側の責務（最小限）
|
||||
|
||||
memory を有効化する Pod は、generic tool に渡す Scope から `memory/`、`knowledge/` を deny に落とす。これにより同じ workspace 内で、generic write/edit は memory 配下を触れず、memory tool だけが触れる構造になる。Pod 側でやることはこの Scope deny と memory tool の登録だけ。
|
||||
|
||||
### 「sub-Worker / 人間」の二系統
|
||||
|
||||
- **sub-Worker**: tool 層を経由するため Linter を必ず通る
|
||||
- **人間**: エディタ / git commit は tool 層を経由しないので Linter を通らない。`memory::Linter` を import して走らせる CLI / pre-commit hook を後で用意できる構造にしておく（本チケットでは実装しない）
|
||||
|
||||
## 要件
|
||||
|
||||
### ディレクトリと record 種別
|
||||
|
||||
- `memory/summary.md` — Always-on サマリ（1 ファイル固定）
|
||||
- `memory/decisions/<slug>.md` — Decisions
|
||||
- `memory/requests/<slug>.md` — Requests
|
||||
- `memory/workflow/<slug>.md` — Workflow（frontmatter 検証のみ対象）
|
||||
- `memory/_staging/<id>.json` — Phase 1 中間（パス予約のみ。Linter 対象外）
|
||||
- `knowledge/<slug>.md` — Knowledge（`memory/` の兄弟）
|
||||
|
||||
slug 文法: `^[a-z0-9](?:[a-z0-9-]{0,62}[a-z0-9])?$`（agent-skills 準拠、1-64 chars、先頭末尾 `-` 不可、`--` 連続不可）。ファイル名がそのまま識別子で、frontmatter に `id` / `name` は持たない。
|
||||
|
||||
### frontmatter スキーマ
|
||||
|
||||
- 共通: `created_at`, `updated_at`（RFC3339）
|
||||
- Decisions: `sources`, `status: open | resolved | replaced`、置き換え時 `replaced_by: <slug>`
|
||||
- Requests: `sources`
|
||||
- Knowledge: `kind`, `description`, `model_invokation`, `user_invocable`, `last_sources`
|
||||
- Summary: `updated_at`（optional: `last_rewritten_from_range`）
|
||||
- Workflow: `description`, `auto_invoke`, `user_invocable`, `requires`
|
||||
|
||||
`sources` / `last_sources` の要素形式は `{ session_id: String, range: [u64, u64] }`。`range` は session-store の entry index ペア。
|
||||
|
||||
### Linter ルール
|
||||
|
||||
**静的 error**（memory tool が `ToolError::InvalidArgument` で返す。複数違反は集約して 1 回で返す）:
|
||||
|
||||
- frontmatter 必須 field 欠落・型違反
|
||||
- `memory/workflow/` への書き込み禁止（sub-Worker のみ。memory tool が拒否、人間編集は memory tool を通らないので素通り）
|
||||
- 同 slug での新規作成禁止（既存があれば update を要求）
|
||||
- `replaced_by: <slug>` / `requires: [..]` が実在 record を指す
|
||||
- `replaced_by` の循環は error
|
||||
- Decisions `status` の enum 違反
|
||||
- slug 文法違反
|
||||
- `model_invokation: true` な Knowledge の description 1024 chars 上限
|
||||
- 種別ごとの char 硬上限（初期既定値、設定 key は別 PR）:
|
||||
- `summary.md`: 20000 chars
|
||||
- decisions / requests / knowledge 本文: 各 8000 chars
|
||||
|
||||
**膨張抑制 Warn**（error にせず、warn として返す。memory tool は受け取って summary に追記する程度）:
|
||||
|
||||
- 低重要度 × char の天秤（暫定: 1500 chars 超のレコードに対して `sources` 1 件のみなら warn）
|
||||
- `sources` 配列長の累積（暫定: 10 件超で warn）
|
||||
- 類似 slug 乱立（暫定: Levenshtein 距離 2 以下の slug が 3 件以上で warn）
|
||||
|
||||
具体閾値の調整は別 PR（設定 key 化）。
|
||||
|
||||
### `#<slug>` 本文中検出はスコープ外
|
||||
|
||||
本文中の `#<slug>` 参照の検出 / 補完は submit-segment 系チケットの責務。本チケットの Linter は frontmatter 由来の参照（`replaced_by` / `requires`）のみを検証する。
|
||||
|
||||
### 参照整合チェックの実行方式
|
||||
|
||||
write/edit 毎に `<workspace>/memory/`、`<workspace>/knowledge/` を毎回 walk して slug 集合を構築（キャッシュなし）。ファイル数は少ない想定。
|
||||
|
||||
### 適用経路
|
||||
|
||||
- memory tool の write/edit 内で pre-write 検証
|
||||
- 人間編集 / pre-commit hook 経路は本チケットでは作らない。`memory::Linter` を pure 関数として export しておけば後で CLI 化できる
|
||||
|
||||
## 範囲外
|
||||
|
||||
- 検索ツール、常駐注入、Phase 1/2、GC の実装
|
||||
- 意味破壊（rewrite で主張が落ちる等）の検出 — 監査 LLM 層は将来検討
|
||||
- staging JSON の schema — Phase 1 チケット
|
||||
- Workflow の `/<slug>` 実行経路
|
||||
- 本文中 `#<slug>` 参照の検出 — submit-segment 系
|
||||
- 設定 key（閾値 tune）— 別 PR
|
||||
- 人間編集向け CLI / pre-commit hook — 別チケット
|
||||
- `Interceptor` / `Hook` 系統への拡張 — 不要（tool error で完結）
|
||||
|
||||
## 完了条件
|
||||
|
||||
- `crates/memory/` が新設され、workspace に登録されている
|
||||
- memory tool 3 種（read / write / edit）が登録できる
|
||||
- write/edit に違反 content を渡すと、複数違反を集約した `ToolError::InvalidArgument` が返り、fs に書き込まれない
|
||||
- 正常 content は通常通り書き込まれる
|
||||
- `memory/workflow/<slug>.md` への write/edit は error で止まる
|
||||
- 同 slug で新規作成しようとすると error になる（existing → edit に倒すサイン）
|
||||
- `replaced_by` / `requires` の参照切れと循環が error として検出される
|
||||
- Pod が memory を有効化すると、generic tool の Scope から memory/knowledge が deny される
|
||||
- 既存ビルド・テストを壊さない
|
||||
|
||||
## 実装順序
|
||||
|
||||
1. `crates/memory/` 新設、workspace 登録、依存追加
|
||||
2. `schema/`, `slug.rs`, `error.rs`（pure 関数 + 型）
|
||||
3. `linter/`（frontmatter / size / 参照存在 / 循環 / workflow 拒否）
|
||||
4. `tool/`（read / write / edit、pre-write で linter 通す）
|
||||
5. Pod 側の Scope deny 配線
|
||||
6. 単体テスト
|
||||
|
||||
各ステップ終了時点でビルド通過を維持する。
|
||||
|
||||
## 参照
|
||||
|
||||
- `docs/plan/memory.md` §ファイル形式 / §書き込み経路と Linter / §Knowledge の採択基準（本チケットで該当箇所を memory 専用 tool 方式に更新）
|
||||
- `docs/plan/workflow.md` §格納先とファイル形式 / §生成・更新ポリシー
|
||||
- `crates/tools/src/{write,edit,read}.rs` — Tool 実装の参考（依存はしない）
|
||||
- `crates/llm-worker/src/tool.rs` — `Tool` trait / `ToolError` / `ToolOutput`
|
||||
|
||||
## Review
|
||||
|
||||
- 状態: Approve with follow-up
|
||||
- レビュー詳細: [./memory-file-format.review.md](./memory-file-format.review.md)
|
||||
- 日付: 2026-04-27
|
||||
|
|
@ -1,73 +0,0 @@
|
|||
# Review: メモリ機構: ファイル形式 + Linter 土台
|
||||
|
||||
## 前提・要件の確認
|
||||
|
||||
### 完了条件マッピング
|
||||
|
||||
| # | 条件 | 結果 | 根拠 |
|
||||
| - | ---- | ---- | ---- |
|
||||
| 1 | `crates/memory/` 新設、workspace 登録 | OK | `Cargo.toml:13`, `crates/memory/Cargo.toml`, `crates/memory/src/lib.rs:9-15` |
|
||||
| 2 | memory tool 3 種（read / write / edit）が登録できる | OK | `crates/pod/src/controller.rs:243-252` |
|
||||
| 3 | 違反 content で複数違反集約の `InvalidArgument`、fs 不変 | OK | `crates/memory/src/tool/write.rs:185-202`, `:237-249`、`linter::format_report` で全 error を 1 メッセージに集約 |
|
||||
| 4 | 正常 content は通常書き込み | OK | `tool/write.rs:147-163` |
|
||||
| 5 | `memory/workflow/` 書き込み拒否 | OK | `linter/mod.rs:101-105`、`tool/write.rs:165-181`, `tool/edit.rs:276-298` |
|
||||
| 6 | 同 slug 新規作成 error | OK | `linter/mod.rs:120-127`, `tool/write.rs:204-222` |
|
||||
| 7 | `replaced_by` / `requires` の参照切れと循環が error | 部分 | `replaced_by` 不在: OK / `replaced_by` 循環: アルゴリズム実装済みだが実循環ケースの end-to-end テスト無し / **`requires` 整合: 未実装**（`schema/workflow.rs` で型パースのみ） |
|
||||
| 8 | Pod 有効化で generic tool Scope deny | OK | `pod.rs:1516-1527` (`build_scope_with_memory`)、`scope::deny_write_rules` |
|
||||
| 9 | 既存ビルド・テスト壊さない | OK | `cargo build --workspace` / `cargo test --workspace` 全 pass、memory 単体 44 テスト pass |
|
||||
|
||||
### Linter ルール（要件節）と実装の照合
|
||||
|
||||
静的 error:
|
||||
- frontmatter 必須欠落・型違反: OK (`linter/frontmatter.rs::map_serde_error`、ただし `parse_invalid_status` は `\`open\`` リテラル含有でラフに status 同定する点はやや fragile — 別フィールドが今後 enum を持つ場合は誤分類リスク)
|
||||
- workflow 書き込み禁止: OK
|
||||
- 同 slug 新規禁止: OK
|
||||
- `replaced_by` 実在: OK
|
||||
- `replaced_by` 循環: 実装あり / テスト不足
|
||||
- Decisions `status` enum: OK
|
||||
- slug 文法: OK (`slug::is_valid_slug` + 全網羅 deserialize 経路)
|
||||
- `model_invokation: true` の description 1024 上限: OK
|
||||
- 種別ごとの char 硬上限 (summary 20000 / 他 8000): OK
|
||||
|
||||
膨張抑制 Warn:
|
||||
- 大 record × 単一 sources warn: OK
|
||||
- sources 配列長 warn: OK
|
||||
- **類似 slug 乱立 (Levenshtein 距離 2 / 3 件以上) warn: 未実装**（`LintWarning::SimilarSlugs` バリアントは定義済みだが、emit 経路無し。`existing.slugs(kind)` は揃っており実装は容易）
|
||||
|
||||
## アーキテクチャ・スコープ
|
||||
|
||||
### 良い点
|
||||
- memory 関連は `crates/memory/` に明確に閉じ込められている。`tools/` 側に侵入なし、`pod` 側は `MemoryConfig` 取得 + `build_scope_with_memory` + tool 登録 3 行のみで責務が薄い
|
||||
- crate 名は `memory` で `insomnia-` プレフィックス無し（命名ルール準拠）
|
||||
- 依存追加は `cargo add` 由来とみられる版指定で workspace に整合（`crates/memory/Cargo.toml`）
|
||||
- `schema/`, `linter/`, `tool/` を feature module で分割しており、巨大 `lib.rs` を回避
|
||||
- LLM への違反伝達は `ToolError::InvalidArgument` のみで完結。Interceptor 拡張・retry message 注入・違反カウンタを持たないという設計方針通り
|
||||
- 公開 API は `lib.rs` 12 行で抑制的、leaky でない
|
||||
- `_staging/` は意図通り linter 透過 (`workspace::classify` で `Ok(None)`)
|
||||
|
||||
### 懸念
|
||||
- **plan 残存矛盾**: `docs/plan/memory.md:196` に「Phase 2 と同じ CRUD tool + Linter Hook を使う」が残っている。「Linter Hook」は本チケットで否定された旧設計の語彙。GC 節の更新漏れ（4 箇所更新と説明されたが GC 節は対象外だった可能性）
|
||||
- **deny target = `<workspace_root>/memory`**: `MemoryConfig::workspace_root` が `None` の時 pwd を採用するが、pwd ≠ workspace root の Pod を spawn したケースで deny 対象パスがズレ、generic write が実際の `memory/` を保護しない可能性。ticket 要件は文字通りには満たしているが、「workspace_root 既定値が pwd」前提が manifest 側で文書化されていない。`PodManifest` 化のドクコメントに「workspace_root 未指定 ⇒ Pod 構築時 pwd」と書く程度は欲しい
|
||||
|
||||
## 指摘事項
|
||||
|
||||
### Blocking
|
||||
なし。完了条件 7 は部分的だが、`requires` の検証は memory tool 経由で書き込む経路が（ticket の禁止により）存在しないため、実害が無い。CLI / pre-commit が別チケットに切り出されていることと整合する
|
||||
|
||||
### Non-blocking / Follow-up
|
||||
- **類似 slug 乱立 warn 未実装** — `crates/memory/src/error.rs:101` `SimilarSlugs` バリアントは定義済みだが emit されない。ticket 要件節「膨張抑制 Warn」に明記されているので、本ライフサイクル内に追加するか、後続 ticket に明示移管したい
|
||||
- **`requires` 参照整合チェック未実装** — `lint_workflow_frontmatter` (`linter/mod.rs:235`) は型パースだけで、`requires: Vec<Slug>` の各 slug が実在 Workflow か検査しない。memory tool では到達しないが、ticket 完了条件に文言が残っている。CLI ticket に明示的に移すか、本チケットで `lint_workflow_frontmatter` の引数に `&ExistingRecords` を渡す形で実装するのが筋
|
||||
- **`replaced_by` 循環の end-to-end テスト追加** — `references.rs::tests::empty_chain_terminates` は smoke test に近い。`A.replaced_by=B`, `B.replaced_by=A` を `existing` に置いた状態で 2 ノード閉路を実検出するテストを足すと完了条件 7 のカバレッジが揃う
|
||||
- **`docs/plan/memory.md:196`** — 「Linter Hook」を「Linter（memory tool 内 pre-write 検証）」相当に書き換え。GC 節は本チケットの直接対象外でも、用語整合は取りたい
|
||||
- **`MemoryConfig.workspace_root` 既定値の文書化** — `crates/manifest/src/lib.rs:46-53` のドクコメントに「`None` ⇒ Pod の pwd 採用」「pwd ≠ workspace root の Pod では明示指定が必要」を追記
|
||||
- **`lint_workflow_frontmatter` を `lib.rs` 公開** — CLI 経路の入口になる関数なので `memory::Linter` と並んで `pub use linter::lint_workflow_frontmatter;` しておくと後続 CLI ticket が触れやすい
|
||||
|
||||
### Nits
|
||||
- `linter/frontmatter.rs::parse_invalid_status` は `\`open\`` 文字列マッチで status enum を識別するため、将来別の enum (`status`-like) が増えた時に誤分類するリスク。コメント注記はあるが、`field: "status"` ハードコード分岐に何らかのテストを足したい
|
||||
- `tool/edit.rs` の `format_report` / `warning_tail` は `tool/write.rs` の同名関数とロジックがほぼ重複。`tool/mod.rs` に小さなヘルパとして括り出すと DRY に
|
||||
- `SourceRef.range: [u64; 2]` は frontmatter での扱いが配列だが、構造体名コメントが「inclusive range」とのみ。ドクコメントに「`[start_entry_index, end_entry_index]` 両端含む」を簡潔に書くと session-store との対応が明示される
|
||||
- `Slug::PartialOrd / Ord` は派生済みだが、`HashMap<Slug, _>` で十分なので `BTreeMap` 用途が現状無い。問題ないが意図メモがあると親切
|
||||
|
||||
## 判断
|
||||
|
||||
**Approve with follow-up** — 完了条件はほぼ全て充足。アーキテクチャ的に memory 専用クレートへ綺麗に閉じ込められ、`tools` / `pod` への漏出は最小（5 行程度）。LLM 違反伝達も `ToolError::InvalidArgument` 一本で完結し、ticket 設計方針に忠実。残課題（類似 slug warn / `requires` 検証 / 循環 e2e テスト / plan 残存表現 / workspace_root ドキュメント）はいずれも非 blocking で、別 PR / 追跡 ticket への計上で十分。ビルド・テストともに workspace 全 pass。
|
||||
Loading…
Reference in New Issue
Block a user