feat(p0-1): workspace skeleton + frozen contracts #5

Merged
altair823 merged 11 commits from feat/p0-1-skeleton into main 2026-04-30 12:16:50 +00:00
Owner

요약

P0-1 task spec 구현. Cargo workspace (Rust 2024, resolver=3) + 5 crate 도입, 모든 domain type / trait / ID recipe / Citation grammar / Config schema / CLI 진입점을 frozen design doc 기준으로 1:1 동결.

기준 문서: tasks/p0/p0-1-skeleton.md, docs/superpowers/specs/2026-04-27-kb-final-form-design.md — design §3 / §3.7b / §4 / §6 / §7 / §8 / §9 / §10 / §2 wire schema 참조.

구현 내용

crate 책임 주요 산출
kb-core 동결된 도메인 타입/trait/ID/Citation/Error 16개 모듈, 9개 component trait, id_from + 6개 ID recipe, Citation 5 variant W3C Media Fragments codec
kb-parse-types parser 중간 표현 (§3.7b) ParsedBlock/ParsedPayload/Warning. 의존: kb-core + serde + thiserror only
kb-config Config schema (§6.4) + XDG Config::{defaults, load, from_file, apply_env}, 4개 XDG resolver
kb-app facade 8 함수 init_workspace / doctor 실구현, 나머지 6개 bail!("not yet wired (P<n>-<i>)")
kb-cli clap v4 derive 진입점 8 subcommand, 글로벌 --config / --verbose / --debug / --json, §10 exit code 0/1/2/3 매핑

추가 산출:

  • docs/wire-schema/v1/*.schema.json 7개 stub
  • docs/spec/*.md 7개 stub
  • migrations/V001__init.sql (schema_meta + migrations)
  • fixtures/ 11 subdir + 3 phase-0 markdown fixture
  • kb-cli/src/wire.rs--json mode schema_version wrapper convention 정착

검증

cargo check --workspace                                       # clean
RUSTFLAGS="-D warnings" cargo build --workspace               # clean
cargo test --workspace                                        # 30 passed (core 20, config 5, cli 5)
cargo clippy --workspace --all-targets -- -D warnings         # clean
cargo tree -p kb-parse-types                                  # kb-core + serde + thiserror only

스모크: XDG_*=/tmp/kbtest cargo run -p kb-cli -- doctor --json{"checks":[...],"ok":true,"schema_version":"doctor.v1"}, exit 0.

Definition of Done

  • Cargo workspace, 5 crate, resolver=3, edition 2024, rust-version 1.85
  • check / test / clippy / build 모두 통과
  • kb-parse-types tree: parser lib / 다른 kb-* 없음
  • kb --help 8 subcommand
  • kb init XDG dir + idempotent
  • kb doctor --json doctor.v1
  • wire-schema v1 7 stub
  • spec doc 7 stub
  • fixtures 11 subdir + 3 markdown
  • Allowed deps 외 import 없음

Review

  • spec compliance: PASS (must-fix 없음).
  • code quality: APPROVED-WITH-MINOR — important 3건 (IngestItemKind re-export / wire schema_version convention / clippy single_char_add_str) 모두 반영 (commit 5af07c1).
  • 후속 phase 자연 해소 nice-to-fix: block/chunk/embedding/index pinned hex 추가, apply_env 일반 매핑, Citation parse 에러 메시지 입력 인용.

다음

머지 후 feat/p1-1-source-fs 분기 → P1-1 (kb-source-fs) 진행.

## 요약 P0-1 task spec 구현. Cargo workspace (Rust 2024, resolver=3) + 5 crate 도입, 모든 domain type / trait / ID recipe / Citation grammar / Config schema / CLI 진입점을 frozen design doc 기준으로 1:1 동결. 기준 문서: tasks/p0/p0-1-skeleton.md, docs/superpowers/specs/2026-04-27-kb-final-form-design.md — design §3 / §3.7b / §4 / §6 / §7 / §8 / §9 / §10 / §2 wire schema 참조. ## 구현 내용 | crate | 책임 | 주요 산출 | |-------|------|----------| | `kb-core` | 동결된 도메인 타입/trait/ID/Citation/Error | 16개 모듈, 9개 component trait, `id_from` + 6개 ID recipe, Citation 5 variant W3C Media Fragments codec | | `kb-parse-types` | parser 중간 표현 (§3.7b) | `ParsedBlock`/`ParsedPayload`/`Warning`. 의존: `kb-core` + serde + thiserror only | | `kb-config` | Config schema (§6.4) + XDG | `Config::{defaults, load, from_file, apply_env}`, 4개 XDG resolver | | `kb-app` | facade 8 함수 | `init_workspace` / `doctor` 실구현, 나머지 6개 `bail!("not yet wired (P<n>-<i>)")` | | `kb-cli` | clap v4 derive 진입점 | 8 subcommand, 글로벌 `--config / --verbose / --debug / --json`, §10 exit code 0/1/2/3 매핑 | 추가 산출: - `docs/wire-schema/v1/*.schema.json` 7개 stub - `docs/spec/*.md` 7개 stub - `migrations/V001__init.sql` (`schema_meta` + `migrations`) - `fixtures/` 11 subdir + 3 phase-0 markdown fixture - `kb-cli/src/wire.rs` — `--json` mode `schema_version` wrapper convention 정착 ## 검증 ``` cargo check --workspace # clean RUSTFLAGS="-D warnings" cargo build --workspace # clean cargo test --workspace # 30 passed (core 20, config 5, cli 5) cargo clippy --workspace --all-targets -- -D warnings # clean cargo tree -p kb-parse-types # kb-core + serde + thiserror only ``` 스모크: `XDG_*=/tmp/kbtest cargo run -p kb-cli -- doctor --json` → `{"checks":[...],"ok":true,"schema_version":"doctor.v1"}`, exit 0. ## Definition of Done - [x] Cargo workspace, 5 crate, resolver=3, edition 2024, rust-version 1.85 - [x] check / test / clippy / build 모두 통과 - [x] kb-parse-types tree: parser lib / 다른 kb-* 없음 - [x] `kb --help` 8 subcommand - [x] `kb init` XDG dir + idempotent - [x] `kb doctor --json` doctor.v1 - [x] wire-schema v1 7 stub - [x] spec doc 7 stub - [x] fixtures 11 subdir + 3 markdown - [x] Allowed deps 외 import 없음 ## Review - spec compliance: PASS (must-fix 없음). - code quality: APPROVED-WITH-MINOR — important 3건 (IngestItemKind re-export / wire schema_version convention / clippy `single_char_add_str`) 모두 반영 (commit 5af07c1). - 후속 phase 자연 해소 nice-to-fix: block/chunk/embedding/index pinned hex 추가, apply_env 일반 매핑, Citation parse 에러 메시지 입력 인용. ## 다음 머지 후 feat/p1-1-source-fs 분기 → P1-1 (kb-source-fs) 진행.
altair823 added 7 commits 2026-04-30 07:30:17 +00:00
Stand up the Cargo workspace (Rust 2024 / resolver=3) with the kb-core
crate per the frozen design (§3, §4, §7, §10). kb-core has zero
deps on other kb-* crates and exposes:

- Newtype IDs (AssetId / DocumentId / BlockId / ChunkId / EmbeddingId /
  IndexId) with Display + FromStr that reject anything but 32 lower-hex.
- id_from + id_for_{asset,doc,block,chunk,embedding,index} per §4.2;
  pinned hex test values computed via an independent JCS+blake3 tool.
- CanonicalDocument, Block (8 variants), SourceSpan, Inline (§3.4).
- Citation (5 variants) with W3C Media Fragments to_uri / parse;
  round-trip property holds for every variant.
- Metadata + Provenance (§3.6); SearchQuery / SearchHit / RetrievalDetail
  (§3.7); DocFilter / DocSummary mirrors of wire §2.5.
- Answer / AnswerCitation / RefusalReason / ModelRef (§3.8).
- IngestReport, JobRepo support types, VectorRecord / VectorHit.
- Component traits (SourceConnector / Extractor / Chunker / Embedder /
  Retriever / LanguageModel / DocumentStore / VectorStore / JobRepo)
  plus their input helpers (SourceScope / ExtractContext / ChunkPolicy
  / EmbeddingInput / GenerateRequest / TokenChunk / FinishReason).
- CoreError (§10).
- nfc + to_posix helpers (§4.1, §6.6).

20 unit tests cover ID determinism (1000-run regression), key-order
invariance, two pinned hex values, newtype rejection of bad input,
Citation round-trip for all 5 variants, and to_posix collapsing +
Korean NFC.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds the kb-parse-types crate per design §3.7b. Depends only on kb-core
+ serde/thiserror — never on parser libraries. Defines:

- ParsedBlock + ParsedBlockKind + ParsedPayload (8 variants matching
  Block variants in kb-core).
- Warning + WarningKind for parser diagnostics.
- Forward-declared ParsedImageRegion / ParsedPdfPage / ParsedAudioSegment
  shells for P6/P7/P8.

`cargo tree -p kb-parse-types` shows only kb-core, serde, and thiserror.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds the kb-config crate per design §6. Provides the frozen Config
schema (§6.4) with serde + toml round-trip, defaults() that exactly
match the reference values (e.g. score_gate=0.30, target_tokens=500,
embedding.dimensions=384, rrf_k=60), and XDG path resolvers that honor
XDG_CONFIG_HOME / XDG_DATA_HOME / XDG_CACHE_HOME / XDG_STATE_HOME.

Layer order in load(): defaults → file → env (KB_<SECTION>_<KEY>);
CLI overrides apply later in kb-cli. Env mapping covers the keys
needed by P0 smoke tests; the rest land as their config sections wire
up.

5 unit tests cover serde round-trip, defaults pinned to design,
KB_RAG_SCORE_GATE / KB_SEARCH_DEFAULT_K env override, and
XDG_CONFIG_HOME handling.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds the kb-app crate (§7) as the single facade between UI crates
(kb-cli / kb-tui / kb-desktop) and the rest of the workspace. Public
surface mirrors the task spec exactly:

- init_workspace(force) — XDG dir creation + config.toml seed; idempotent
  unless force=true. Honors XDG envs and tilde-expands the workspace
  root to $HOME/KnowledgeBase.
- doctor() — emits a doctor.v1 report with config_loaded +
  data_dir_writable checks; downstream checks land in later phases.
- ingest / list_docs / inspect_doc / inspect_chunk / search / ask —
  bail!("not yet wired (P<n>-<i>)") so kb-cli surfaces exit code 2
  cleanly per §10.
- AskOpts + DoctorReport + DoctorCheck.
- doctor_signal::{DoctorUnhealthy, RefusalSignal, NoHitSignal} —
  signal types the CLI downcasts on for §10 exit-code mapping.
- logging::init() — daily-rolling file appender at
  $XDG_STATE_HOME/kb/logs/kb.log, plus stderr-fallback EnvFilter.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds the kb binary with clap v4 derive subcommands mapping 1:1 to
kb-app facade functions:

  init | ingest | list docs | inspect (doc|chunk) | search | ask
  | doctor | eval run

Global flags: --config, --verbose, --debug, --json. On --json, output
conforms to wire schema v1 (e.g. doctor.v1 emitted by kb-app::doctor).

Exit-code mapping per design §10:
  0 success
  1 RefusalSignal / NoHitSignal (kb ask refusal, kb search no-hit)
  2 any other anyhow::Error
  3 DoctorUnhealthy

Tracing initialized at startup with the file appender from kb-app.

Verified via:
  XDG_*=… cargo run -p kb-cli -- init    → idempotent
  XDG_*=… cargo run -p kb-cli -- doctor --json
    → {"schema_version":"doctor.v1","ok":true,…} exit 0
  XDG_*=… cargo run -p kb-cli -- doctor   (human form, ✓ marks)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- docs/wire-schema/v1/ ships 7 schema stubs (citation, search_hit,
  answer, ingest_report, doc_summary, chunk_inspection, doctor) that
  pin schema_version + required fields per design §2. Full property
  validation lands in later phases.
- docs/spec/ ships 7 markdown stubs each linking to the canonical
  frozen design (domain-model, ids, canonical-document, chunk-policy,
  citation-policy, module-boundaries, ai-generation-guidelines).
- migrations/V001__init.sql contains only schema_meta + migrations
  tables per design §5.1; data tables ship in P1-6/P2-1/P3-3.
- fixtures/ has the 11 subdirectories every downstream task references
  (markdown, source-fs, search/{lexical,hybrid}, embed, vector, rag,
  eval, image, pdf, audio). Empty subdirs use .gitkeep so they track.
  fixtures/markdown/ ships the 3 phase-0 fixtures: simple-note.md,
  nested-headings.md, code-and-table.md.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three follow-ups from the code-quality review pass on P0-1:

- Re-export `IngestItemKind` from `kb-core` so downstream tasks
  constructing `IngestItem` don't need `kb_core::ingest::IngestItemKind`.
- Document the `--json` wire-schema convention by introducing
  `kb-cli/src/wire.rs` with `wire_*` helpers paralleling the existing
  inline `wire_ingest`. Each Ok-path `--json` branch now routes through
  these helpers so future P1-5/P3/P4/P5 implementations slot the
  `schema_version` envelope in automatically. `DoctorReport` keeps its
  struct-field `schema_version` (the documented exception), and the
  helper round-trips it idempotently. Records the convention in
  `kb-app/src/lib.rs`'s top docstring.
- Fix clippy `single_char_add_str` in `kb_core::normalize` (replace
  `out.push_str(".")` with `out.push('.')`).

Verified: `cargo check`, `cargo test` (5 new wire-helper tests),
`cargo clippy -D warnings`, and `RUSTFLAGS=-D warnings cargo build` all
clean. Smoke-tested `kb doctor --json` still emits
`{"schema_version":"doctor.v1",...}`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
claude-reviewer-01 reviewed 2026-04-30 08:36:56 +00:00
claude-reviewer-01 left a comment
Member

코드 리뷰 (COMMENT only)

내부 spec compliance + code quality review 통과 후 important 3건 fix commit (5af07c1) 반영 확인. 추가로 PR 차원의 inline 코멘트 9개 (problems 6 + strengths 3) 첨부.

핵심 판정: must-fix 없음. nice-to-fix 만 — 후속 phase 자연 해소 가능.

검증 통과

  • cargo check / test (30 pass) / clippy / build (warnings-as-errors) 모두 clean
  • cargo tree -p kb-parse-types → kb-core + serde + thiserror only
  • design §3 / §4 / §6 / §7 / §10 / §2 wire schema_version contract 1:1 부합
  • kb doctor --json exit 0 + doctor.v1 shape

추천 후속 작업 (P1-* 진행 중 자연 해소 가능)

  1. id_for_block/chunk/embedding/index 4개 pinned hex test 추가
  2. Citation::parse 경로 내 # 동작 결정 (test pin 또는 reject)
  3. Citation::parse 에러 메시지에 입력 인용 (8군데, mechanical)
  4. kb-config/src/lib.rs::apply_env 일반 KB_<SECTION>_<KEY> 매핑
  5. unused dep 정리 (kb-config, kb-parse-types, kb-appthiserror)

approve는 작성자(=리뷰어 본인)가 별도 처리.

## 코드 리뷰 (COMMENT only) 내부 spec compliance + code quality review 통과 후 important 3건 fix commit (`5af07c1`) 반영 확인. 추가로 PR 차원의 inline 코멘트 9개 (problems 6 + strengths 3) 첨부. **핵심 판정:** must-fix 없음. nice-to-fix 만 — 후속 phase 자연 해소 가능. ### 검증 통과 - `cargo check / test (30 pass) / clippy / build (warnings-as-errors)` 모두 clean - `cargo tree -p kb-parse-types` → kb-core + serde + thiserror only - design §3 / §4 / §6 / §7 / §10 / §2 wire `schema_version` contract 1:1 부합 - `kb doctor --json` exit 0 + `doctor.v1` shape ### 추천 후속 작업 (P1-* 진행 중 자연 해소 가능) 1. `id_for_block/chunk/embedding/index` 4개 pinned hex test 추가 2. `Citation::parse` 경로 내 `#` 동작 결정 (test pin 또는 reject) 3. `Citation::parse` 에러 메시지에 입력 인용 (8군데, mechanical) 4. `kb-config/src/lib.rs::apply_env` 일반 `KB_<SECTION>_<KEY>` 매핑 5. unused dep 정리 (`kb-config`, `kb-parse-types`, `kb-app`의 `thiserror`) approve는 작성자(=리뷰어 본인)가 별도 처리.
@@ -0,0 +17,4 @@
/// Initialize tracing. Returns a guard to keep alive until exit. Idempotent
/// — a second call is a no-op.
pub fn init(level: LogLevel) -> Result<Option<WorkerGuard>> {

🟡 minor — Result<Option<WorkerGuard>>이지만 항상 Ok(Some(_))

현재 함수는 ErrOk(None)도 반환하지 않음 (fs::create_dir_all 실패 시만 Err). Option은 죽은 surface. caller (kb-cli/src/main.rs:159)가 .ok().flatten()으로 두 layer를 한꺼번에 떠는 형태도 같은 noise.

옵션:

  • Result<WorkerGuard>로 단순화 (caller도 .ok() 한 번만).
  • 또는 "파일 logging 비활성화" variant가 미래에 있을 거면 doc-string에 명시.

현 phase 작동에는 영향 없으므로 후속 cleanup으로 OK.

## 🟡 minor — `Result<Option<WorkerGuard>>`이지만 항상 `Ok(Some(_))` 현재 함수는 `Err`도 `Ok(None)`도 반환하지 않음 (`fs::create_dir_all` 실패 시만 `Err`). `Option`은 죽은 surface. caller (`kb-cli/src/main.rs:159`)가 `.ok().flatten()`으로 두 layer를 한꺼번에 떠는 형태도 같은 noise. 옵션: - `Result<WorkerGuard>`로 단순화 (caller도 `.ok()` 한 번만). - 또는 "파일 logging 비활성화" variant가 미래에 있을 거면 doc-string에 명시. 현 phase 작동에는 영향 없으므로 후속 cleanup으로 OK.
@@ -0,0 +10,4 @@
//! Future tasks (P1-5, P3, P4, P5) replacing stub `bail!` paths must call
//! these helpers from the relevant CLI subcommand handler before
//! `serde_json::to_string`.
//!

👍 strength — wire convention 정착

quality review 후 kb-cli/src/wire.rs 도입한 게 정확한 결정. 도메인 타입 (kb-core)에 schema_version 필드를 박지 않고 CLI emit 시점에 wrap 하는 방향이 design §2 의도와 부합 (도메인 ↔ wire 분리).

특히 wire_search_hitretrieval.fusion_score → 최상위 score promote 하는 로직 (§2.2 wire schema와 부합)을 한 곳에 모아둔 것 — P1-5/P3/P4/P5 어느 task가 이 함수를 호출하든 spec drift 없이 동일한 wire shape 보장.

나중에 wire schema 갱신할 때 단일 진입점이라 영향 범위 명확.

## 👍 strength — wire convention 정착 quality review 후 `kb-cli/src/wire.rs` 도입한 게 정확한 결정. 도메인 타입 (`kb-core`)에 `schema_version` 필드를 박지 않고 CLI emit 시점에 wrap 하는 방향이 design §2 의도와 부합 (도메인 ↔ wire 분리). 특히 `wire_search_hit`이 `retrieval.fusion_score` → 최상위 `score` promote 하는 로직 (§2.2 wire schema와 부합)을 한 곳에 모아둔 것 — P1-5/P3/P4/P5 어느 task가 이 함수를 호출하든 spec drift 없이 동일한 wire shape 보장. 나중에 wire schema 갱신할 때 단일 진입점이라 영향 범위 명확.
@@ -0,0 +8,4 @@
description = "Config schema + XDG path resolution"
[dependencies]
kb-core = { path = "../kb-core" }

🟡 minor — 사용 안 하는 dep 선언

kb-core (line 11) 와 thiserror (line 13) 가 declare 됐지만 kb-config/src/lib.rs에서 import되지 않음. 동일 패턴이 kb-parse-types/Cargo.toml (thiserror) 와 kb-app/Cargo.toml (thiserror) 에도 있음.

빌드 시간 증가 + 미래 grep으로 "왜 의존성 있지?" 혼란 유발. 두 옵션:

  1. 제거 (가장 깔끔).
  2. # reserved for P1-*: <reason> 주석 — 의도가 분명할 때만.

kb-config의 경우 kb-core::CoreError를 결국 사용할 가능성 높으므로 옵션 2 + 주석 권장.

## 🟡 minor — 사용 안 하는 dep 선언 `kb-core` (line 11) 와 `thiserror` (line 13) 가 declare 됐지만 `kb-config/src/lib.rs`에서 import되지 않음. 동일 패턴이 `kb-parse-types/Cargo.toml` (`thiserror`) 와 `kb-app/Cargo.toml` (`thiserror`) 에도 있음. 빌드 시간 증가 + 미래 grep으로 "왜 의존성 있지?" 혼란 유발. 두 옵션: 1. 제거 (가장 깔끔). 2. `# reserved for P1-*: <reason>` 주석 — 의도가 분명할 때만. `kb-config`의 경우 `kb-core::CoreError`를 결국 사용할 가능성 높으므로 옵션 2 + 주석 권장.
@@ -0,0 +196,4 @@
}
// Match a small, intentional whitelist for P0 — full env→config
// mapping lands when the rest of the schema is wired up.
match k.as_str() {

🟡 nice-to-fix — apply_env 4-key whitelist

P0 unit test가 의존하는 4개 키만 처리. design §6.4 contract는 generic KB_<SECTION>_<KEY> 패턴이라 P1+에서 키 추가될 때마다 여기 손대야 함. 주석에 의도가 명시돼 있고 P0 단계 동작에는 문제 없으니 이번 PR에서 막진 않음.

P1-* component task가 새 config 키를 도입할 때 자연스럽게 확장하거나, 후속 cleanup PR에서 reflection-style 일반 매핑 (예: serde + nested key path)으로 전환 권장.

또한 negative test 하나 (env_unknown_key_is_ignored)가 있으면 화이트리스트 동작이 명시적으로 pin 됨.

## 🟡 nice-to-fix — `apply_env` 4-key whitelist P0 unit test가 의존하는 4개 키만 처리. design §6.4 contract는 generic `KB_<SECTION>_<KEY>` 패턴이라 P1+에서 키 추가될 때마다 여기 손대야 함. 주석에 의도가 명시돼 있고 P0 단계 동작에는 문제 없으니 이번 PR에서 막진 않음. P1-* component task가 새 config 키를 도입할 때 자연스럽게 확장하거나, 후속 cleanup PR에서 reflection-style 일반 매핑 (예: serde + nested key path)으로 전환 권장. 또한 negative test 하나 (`env_unknown_key_is_ignored`)가 있으면 화이트리스트 동작이 명시적으로 pin 됨.
@@ -0,0 +88,4 @@
/// `section = None` and `model = ""` for the relevant variants.
/// Round-trip property holds for citations whose non-URI fields are at
/// their default values (see test).
pub fn parse(s: &str) -> Result<Self> {

🟡 minor — parse error 메시지에 입력 인용 부족

citation.rsbail!("bad line start") / bail!("unknown fragment") 등 (8군데) 모두 입력 문자열을 인용하지 않음. 사용자/디버거 입장에서 "bad line start"만 받으면 s 값을 추측해야 함.

bail!("bad line start in {a:?}")

같은 패턴으로 다 바꾸면 디버깅 비용이 한 번에 줄어듦. 회귀 위험 0.

## 🟡 minor — parse error 메시지에 입력 인용 부족 `citation.rs`의 `bail!("bad line start")` / `bail!("unknown fragment")` 등 (8군데) 모두 입력 문자열을 인용하지 않음. 사용자/디버거 입장에서 "bad line start"만 받으면 `s` 값을 추측해야 함. ```rust bail!("bad line start in {a:?}") ``` 같은 패턴으로 다 바꾸면 디버깅 비용이 한 번에 줄어듦. 회귀 위험 0.
@@ -0,0 +89,4 @@
/// Round-trip property holds for citations whose non-URI fields are at
/// their default values (see test).
pub fn parse(s: &str) -> Result<Self> {
let (path_str, frag) = match s.rsplit_once('#') {

🟡 nice-to-fix — 경로에 # 포함될 때 동작 미정

rsplit_once('#')은 마지막 #를 fragment delimiter로 선택. POSIX/NFC에서 파일명에 #은 합법이라 (드물지만) notes/foo#bar.md#L1-L5 같은 입력은 path를 notes/foo#bar.md로 올바르게 parse하지만, notes/foo#bar.md (fragment 없음) + 잘못된 위치의 #는 silently mis-parse 가능.

둘 중 하나 권장:

  1. 동작 pin test 추가a#b#L5-L10 → path=a#b, fragment=L5-L10 동작을 명시적으로 박아둠.
  2. to_posix/WorkspacePath 생성에서 # reject — 더 안전, design §0 Q3의 W3C Media Fragments 문법과도 부합.

결정 자체보다 미정 상태가 문제. P1-1 source-fs 들어가기 전에 정하는 게 좋음.

## 🟡 nice-to-fix — 경로에 `#` 포함될 때 동작 미정 `rsplit_once('#')`은 마지막 `#`를 fragment delimiter로 선택. POSIX/NFC에서 파일명에 `#`은 합법이라 (드물지만) `notes/foo#bar.md#L1-L5` 같은 입력은 path를 `notes/foo#bar.md`로 올바르게 parse하지만, `notes/foo#bar.md` (fragment 없음) + 잘못된 위치의 `#`는 silently mis-parse 가능. 둘 중 하나 권장: 1. **동작 pin test 추가** — `a#b#L5-L10` → path=`a#b`, fragment=`L5-L10` 동작을 명시적으로 박아둠. 2. **`to_posix`/`WorkspacePath` 생성에서 `#` reject** — 더 안전, design §0 Q3의 W3C Media Fragments 문법과도 부합. 결정 자체보다 미정 상태가 문제. P1-1 source-fs 들어가기 전에 정하는 게 좋음.
@@ -0,0 +51,4 @@
s.len()
)));
}
if !s.bytes().all(|b| matches!(b, b'0'..=b'9' | b'a'..=b'f')) {

🟡 nice-to-fix — FromStr이 lowercase-hex만 수용

blake3::Hash::to_hex()가 lowercase로 출력하므로 내부 round-trip은 안전. 다만 외부 시스템(다른 프로젝트, 사람이 손으로 친 ID)에서 uppercase hex로 들어오면 silently InvalidId. wire schema input 검증 단계에서 normalize하거나, 여기서 case-insensitive 비교 + lowercase로 정규화해 저장하는 쪽이 robustness 측면에서 안전.

현재 phase에서는 모든 ID가 id_from을 거치므로 문제 없음 — P1+에서 외부 입력 받기 시작할 때 결정 필요.

## 🟡 nice-to-fix — `FromStr`이 lowercase-hex만 수용 `blake3::Hash::to_hex()`가 lowercase로 출력하므로 내부 round-trip은 안전. 다만 외부 시스템(다른 프로젝트, 사람이 손으로 친 ID)에서 uppercase hex로 들어오면 silently `InvalidId`. wire schema input 검증 단계에서 normalize하거나, 여기서 case-insensitive 비교 + lowercase로 정규화해 저장하는 쪽이 robustness 측면에서 안전. 현재 phase에서는 모든 ID가 `id_from`을 거치므로 문제 없음 — P1+에서 외부 입력 받기 시작할 때 결정 필요.
@@ -0,0 +283,4 @@
// → cec9353553efb238a7919d38d3e148f1...
let id = id_for_asset("deadbeef");
assert_eq!(id.0, "cec9353553efb238a7919d38d3e148f1");
}

👍 strength — pinned hex test 구조가 정확

외부 도구(b3sum)로 계산한 hex를 literal로 박았고, 주석에 canonical-JSON 입력 + b3sum 명령까지 명시. self-referential 아니므로 JCS 또는 blake3 파이프라인 회귀가 진짜로 잡힘. 이게 ID recipe test의 올바른 형태.

같은 패턴을 id_for_block / id_for_chunk / id_for_embedding / id_for_index에도 4개 더 추가하면 모든 ID 함수가 외부 검증 안전망에 들어감 — P0-1 spec test plan의 "each id_for_* recipe matches design §4.2 byte-for-byte" 항목을 더 강하게 만족.

## 👍 strength — pinned hex test 구조가 정확 외부 도구(`b3sum`)로 계산한 hex를 literal로 박았고, 주석에 canonical-JSON 입력 + `b3sum` 명령까지 명시. self-referential 아니므로 JCS 또는 blake3 파이프라인 회귀가 진짜로 잡힘. 이게 ID recipe test의 올바른 형태. 같은 패턴을 `id_for_block` / `id_for_chunk` / `id_for_embedding` / `id_for_index`에도 4개 더 추가하면 모든 ID 함수가 외부 검증 안전망에 들어감 — P0-1 spec test plan의 "each `id_for_*` recipe matches design §4.2 byte-for-byte" 항목을 더 강하게 만족.
@@ -0,0 +1,70 @@
//! `kb-core` — frozen domain types, traits, and ID recipe.

👍 strength — module 분할 깔끔

kb-core 16 모듈이 design §3 sub-section 경계를 그대로 따라감 (ids / citation / document / chunk / metadata / search / answer / ingest / jobs / vector / errors / traits / media / asset / versions / normalize). 거대 dump 파일 없음. 가장 큰 citation.rs도 316 lines로 hold-in-context 가능.

lib.rs 자체가 70 lines, pub mod + 큐레이션된 re-export만 있고 로직 없음. P1-* component task가 kb_core::*로 단일 facade를 통해 import할 수 있어 downstream 의존성 그래프가 안 흔들림.

## 👍 strength — module 분할 깔끔 `kb-core` 16 모듈이 design §3 sub-section 경계를 그대로 따라감 (`ids` / `citation` / `document` / `chunk` / `metadata` / `search` / `answer` / `ingest` / `jobs` / `vector` / `errors` / `traits` / `media` / `asset` / `versions` / `normalize`). 거대 dump 파일 없음. 가장 큰 `citation.rs`도 316 lines로 hold-in-context 가능. `lib.rs` 자체가 70 lines, `pub mod` + 큐레이션된 re-export만 있고 로직 없음. P1-* component task가 `kb_core::*`로 단일 facade를 통해 import할 수 있어 downstream 의존성 그래프가 안 흔들림.
altair823 added 4 commits 2026-04-30 08:56:41 +00:00
- ids.rs: validate_hex32 now accepts upper+lower hex; FromStr lowercases
  the stored representation so equality and hashing stay canonical.
- Renamed test newtype_rejects_uppercase →
  newtype_accepts_uppercase_normalizes_to_lowercase and added
  newtype_rejects_invalid_chars_after_uppercase_pass.
- Added pinned-hex independence tests for id_for_block / id_for_chunk /
  id_for_embedding / id_for_index. Each test also asserts that the bytes
  serde-json-canonicalizer emits for the tuple match the literal JSON we
  hashed externally with b3sum, so a future field-rename can't silently
  drift the hash without flagging the JSON layer first.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- WorkspacePath: add `WorkspacePath::new(s)` validating constructor that
  rejects any string containing `#` (collides with the W3C-Media-Fragments
  separator that Citation URIs depend on). Doc-comment on the type now
  explains the invariant.
- normalize::to_posix changes signature to `Result<WorkspacePath, CoreError>`
  and now flows through `WorkspacePath::new`, so a path with `#` is rejected
  at construction rather than at every reader. Only one caller existed
  outside tests, so the signature change is contained.
- Citation::parse uses `WorkspacePath::new` on the path side. With multiple
  `#` separators in the input, `rsplit_once` would otherwise leave a `#` on
  the path; the new constructor closes the hole.
- Citation::parse + parse_hms_ms: every `bail!` / `anyhow!` site now quotes
  the offending substring (and the full input where useful) so error
  messages identify what went wrong without re-deriving from context.
- New tests: `to_posix_rejects_hash_in_path`,
  `parse_path_with_hash_rejected_at_to_posix_layer`. Existing
  `to_posix(...).0` call sites updated for the Result signature.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- kb-config::apply_env now covers every leaf key in `Config` via an
  explicit grep-friendly match block (one arm per leaf), keyed
  `KB_<SECTION>_<KEY>`. Booleans flow through a shared `parse_bool` helper.
  Numeric leaves silently keep their prior value on parse failure so a
  malformed env entry can't crash startup.
- New tests: env_unknown_key_is_ignored,
  env_overrides_chunking_target_tokens,
  env_overrides_models_llm_endpoint_and_temperature,
  env_overrides_indexing_watch_filesystem_bool.
- kb-app::logging::init now returns `Result<WorkerGuard>` instead of
  `Result<Option<WorkerGuard>>` — the inner `Option` was always `Some` so
  the wrapper was dead. kb-cli/main.rs collapses the call from
  `.ok().flatten()` to `.ok()`, preserving fail-soft semantics on logging
  init.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Cargo.toml: remove `thiserror` from kb-config, kb-parse-types, kb-app
  (unused — none of those crates' src trees reference thiserror; CoreError
  in kb-core is the only consumer).
- kb-config keeps the `kb-core` dep with a one-line comment marking
  CoreError reserved for P1-* config-error wiring per the review thread.
- ids.rs: switch `validate_hex32` from a hand-rolled `matches!` byte range
  to `is_ascii_hexdigit()` so the hex check is the canonical idiom (and
  satisfies `clippy::manual_is_ascii_check` under `-D warnings`).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
claude-reviewer-01 reviewed 2026-04-30 09:01:01 +00:00
claude-reviewer-01 left a comment
Member

재리뷰: 모든 항목 해소 — APPROVE 권고 (gate 우회 정책상 COMMENT로 등록)

이전 리뷰 9건 (problems 6 + minor 3) 모두 commit 286e627 / ca0eb2f / d91b603 / d2c8728 에 반영됨. fresh subagent로 항목별 코드 다시 읽고 검증 완료.

검증 결과

# 항목 상태 위치
1 FromStr lowercase-only hex → upper+lower 수용 + lowercase 정규화 ids.rs:34-37,49-62
2 block/chunk/embedding/index pinned hex test 4개 추가 (b3sum 외부 재검증) ids.rs:331-476
3 WorkspacePath::new 에서 # reject + to_posix Result 시그니처 + 호출자 모두 갱신 asset.rs:33-40, normalize.rs:24-61, citation.rs:99
4 apply_env 27 arm 전체 schema 매핑 + negative test + positive test 3종 config/lib.rs:192-329,431-469
5 logging::initResult<WorkerGuard> (dead Option 제거) + 호출자 정리 logging.rs:21, cli/main.rs:163
6 Citation::parse 모든 bail!/anyhow! 19개 site에 입력 인용 citation.rs 전체
7 thiserror 미사용 dep 제거 (3 Cargo.toml) + kb-core reservation 주석 kb-config/parse-types/app Cargo.toml

외부 b3sum 재검증

테스트 박힌 hex b3sum 재계산 매치
id_for_asset_pinned cec9353553efb238a7919d38d3e148f1 동일
id_for_doc_pinned 8547fe58cb42d593fd761d77242401db 동일
id_for_block_pinned 8a7bf22de7ec3293a792028c829b3812 동일
id_for_chunk_pinned 8809f627777fe7ca5c4433b97dd88ce9 동일
id_for_embedding_pinned 71992c457a5da39880a6d17d646ed0fd 동일
id_for_index_pinned e733ee2f9936f0e1ac5143cdbf0f2b54 동일

각 테스트에 serde_json_canonicalizer::to_vec(&tuple) == expected_json_bytes 별도 assert 존재 → JSON canonicalization 회귀와 hash 회귀가 독립적으로 잡힘.

Gate 통과

  • cargo check --workspace clean
  • RUSTFLAGS="-D warnings" cargo build --workspace clean
  • cargo test --workspace41 passed / 0 failed (kb-core 27, kb-config 9, kb-cli 5; +11 vs 직전)
  • cargo clippy --workspace --all-targets -- -D warnings clean
  • kb doctor --json exit 0 + doctor.v1

새로 발견된 이슈

없음.

결론

모든 항목 papered over 아닌 root-cause fix. APPROVE 권고. self-review gate 정책상 본 코멘트는 COMMENT로 등록 — author 측���서 수동 APPROVE + merge 부탁드림.

## 재리뷰: 모든 항목 해소 — APPROVE 권고 (gate 우회 정책상 COMMENT로 등록) 이전 리뷰 9건 (problems 6 + minor 3) 모두 commit `286e627` / `ca0eb2f` / `d91b603` / `d2c8728` 에 반영됨. fresh subagent로 항목별 코드 다시 읽고 검증 완료. ### 검증 결과 | # | 항목 | 상태 | 위치 | |---|------|------|------| | 1 | `FromStr` lowercase-only hex → upper+lower 수용 + lowercase 정규화 | ✅ | [`ids.rs:34-37,49-62`](crates/kb-core/src/ids.rs#L34-L62) | | 2 | block/chunk/embedding/index pinned hex test 4개 추가 (b3sum 외부 재검증) | ✅ | [`ids.rs:331-476`](crates/kb-core/src/ids.rs#L331-L476) | | 3 | `WorkspacePath::new` 에서 `#` reject + `to_posix` Result 시그니처 + 호출자 모두 갱신 | ✅ | [`asset.rs:33-40`](crates/kb-core/src/asset.rs#L33-L40), [`normalize.rs:24-61`](crates/kb-core/src/normalize.rs#L24-L61), [`citation.rs:99`](crates/kb-core/src/citation.rs#L99) | | 4 | `apply_env` 27 arm 전체 schema 매핑 + negative test + positive test 3종 | ✅ | [`config/lib.rs:192-329,431-469`](crates/kb-config/src/lib.rs#L192-L469) | | 5 | `logging::init` → `Result<WorkerGuard>` (dead Option 제거) + 호출자 정리 | ✅ | [`logging.rs:21`](crates/kb-app/src/logging.rs#L21), [`cli/main.rs:163`](crates/kb-cli/src/main.rs#L163) | | 6 | `Citation::parse` 모든 `bail!`/`anyhow!` 19개 site에 입력 인용 | ✅ | citation.rs 전체 | | 7 | `thiserror` 미사용 dep 제거 (3 Cargo.toml) + `kb-core` reservation 주석 | ✅ | kb-config/parse-types/app Cargo.toml | ### 외부 b3sum 재검증 | 테스트 | 박힌 hex | b3sum 재계산 | 매치 | |--------|---------|------------|-----| | `id_for_asset_pinned` | `cec9353553efb238a7919d38d3e148f1` | 동일 | ✓ | | `id_for_doc_pinned` | `8547fe58cb42d593fd761d77242401db` | 동일 | ✓ | | `id_for_block_pinned` | `8a7bf22de7ec3293a792028c829b3812` | 동일 | ✓ | | `id_for_chunk_pinned` | `8809f627777fe7ca5c4433b97dd88ce9` | 동일 | ✓ | | `id_for_embedding_pinned` | `71992c457a5da39880a6d17d646ed0fd` | 동일 | ✓ | | `id_for_index_pinned` | `e733ee2f9936f0e1ac5143cdbf0f2b54` | 동일 | ✓ | 각 테스트에 `serde_json_canonicalizer::to_vec(&tuple) == expected_json_bytes` 별도 assert 존재 → JSON canonicalization 회귀와 hash 회귀가 독립적으로 잡힘. ### Gate 통과 - `cargo check --workspace` clean - `RUSTFLAGS="-D warnings" cargo build --workspace` clean - `cargo test --workspace` → **41 passed** / 0 failed (kb-core 27, kb-config 9, kb-cli 5; +11 vs 직전) - `cargo clippy --workspace --all-targets -- -D warnings` clean - `kb doctor --json` exit 0 + `doctor.v1` ### 새로 발견된 이슈 없음. ### 결론 모든 항목 papered over 아닌 root-cause fix. APPROVE 권고. self-review gate 정책상 본 코멘트는 COMMENT로 등록 — author 측���서 수동 APPROVE + merge 부탁드림.
altair823 merged commit 98280c6249 into main 2026-04-30 12:16:50 +00:00
Sign in to join this conversation.
No Reviewers
No Label
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: altair823-org/kebab#5