feat(fb-32): per-hit + per-citation freshness indicators #122

Merged
altair823 merged 18 commits from feat/fb-32-stale-doc-indicator into main 2026-05-09 03:26:06 +00:00
Owner

Summary

  • adds indexed_at: RFC3339 + stale: bool to SearchHit and AnswerCitation
  • server-computes stale = now - documents.updated_at > threshold_days * 86400s via kebab_app::staleness::compute_stale
  • config search.stale_threshold_days default 30; 0 disables; env KEBAB_SEARCH_STALE_THRESHOLD_DAYS
  • no DB migration — reuses documents.updated_at (V001). fb-23 incremental ingest's skip path leaves it untouched, making it the natural source of truth
  • wire schema additive minor (search_hit.v1 / citation.v1 keep their version slot)
  • CLI plain [stale] tag (yellow on TTY), TUI [STALE] Warning-styled badge on Search/Inspect/Ask, agent JSON inherits via serde
  • pipeline-level stamping in RagPipeline::ask closes a discovered gap — App::ask doesn't go through App::search, so the App-level post-process never ran on the ask path; refusal + grounded paths now share one source of truth
  • cache hits re-stamp on every return so config threshold + clock changes take effect without invalidation

Test plan

  • cargo test --workspace --no-fail-fast -j 1 — all green
  • cargo clippy --workspace --all-targets -- -D warnings — clean
  • new tests: compute_stale boundaries (5), App::search integration (3), lexical retriever (1), vector retriever (1), pipeline grounded citations (2), CLI search wire+plain (3), CLI ask wire+plain (3 Ollama-gated + unit), TUI Search/Inspect/Ask staleness (5)
  • manual smoke per docs/SMOKE.md "Stale doc indicator" walkthrough

Architectural notes for reviewers

  • compute_stale is intentionally duplicated in kebab-app and kebab-rag (dep boundary rule prevents kebab-rag → kebab-app). Both copies cross-reference. Future drift mitigation tracked under follow-up.
  • All OffsetDateTime::UNIX_EPOCH references are in synthetic test fixtures only — no production path stamps placeholder values.
  • kebab-core still has zero kebab-* deps.

Files of interest

  • spec: docs/superpowers/specs/2026-05-08-p9-fb-32-stale-doc-indicator-design.md
  • plan: docs/superpowers/plans/2026-05-09-p9-fb-32-stale-doc-indicator.md
  • core: crates/kebab-core/src/{search,answer}.rs
  • staleness: crates/kebab-app/src/staleness.rs
  • pipeline stamp: crates/kebab-rag/src/pipeline.rs:188-202
  • wire: docs/wire-schema/v1/{search_hit,citation}.schema.json
## Summary - adds `indexed_at: RFC3339` + `stale: bool` to `SearchHit` and `AnswerCitation` - server-computes `stale = now - documents.updated_at > threshold_days * 86400s` via `kebab_app::staleness::compute_stale` - config `search.stale_threshold_days` default 30; `0` disables; env `KEBAB_SEARCH_STALE_THRESHOLD_DAYS` - no DB migration — reuses `documents.updated_at` (V001). fb-23 incremental ingest's skip path leaves it untouched, making it the natural source of truth - wire schema additive minor (`search_hit.v1` / `citation.v1` keep their version slot) - CLI plain `[stale]` tag (yellow on TTY), TUI `[STALE]` Warning-styled badge on Search/Inspect/Ask, agent JSON inherits via serde - pipeline-level stamping in `RagPipeline::ask` closes a discovered gap — `App::ask` doesn't go through `App::search`, so the App-level post-process never ran on the ask path; refusal + grounded paths now share one source of truth - cache hits re-stamp on every return so config threshold + clock changes take effect without invalidation ## Test plan - [x] `cargo test --workspace --no-fail-fast -j 1` — all green - [x] `cargo clippy --workspace --all-targets -- -D warnings` — clean - [x] new tests: `compute_stale` boundaries (5), `App::search` integration (3), lexical retriever (1), vector retriever (1), pipeline grounded citations (2), CLI search wire+plain (3), CLI ask wire+plain (3 Ollama-gated + unit), TUI Search/Inspect/Ask staleness (5) - [x] manual smoke per `docs/SMOKE.md` "Stale doc indicator" walkthrough ## Architectural notes for reviewers - `compute_stale` is intentionally duplicated in `kebab-app` and `kebab-rag` (dep boundary rule prevents `kebab-rag → kebab-app`). Both copies cross-reference. Future drift mitigation tracked under follow-up. - All `OffsetDateTime::UNIX_EPOCH` references are in synthetic test fixtures only — no production path stamps placeholder values. - `kebab-core` still has zero `kebab-*` deps. ## Files of interest - spec: `docs/superpowers/specs/2026-05-08-p9-fb-32-stale-doc-indicator-design.md` - plan: `docs/superpowers/plans/2026-05-09-p9-fb-32-stale-doc-indicator.md` - core: `crates/kebab-core/src/{search,answer}.rs` - staleness: `crates/kebab-app/src/staleness.rs` - pipeline stamp: `crates/kebab-rag/src/pipeline.rs:188-202` - wire: `docs/wire-schema/v1/{search_hit,citation}.schema.json`
altair823 added 16 commits 2026-05-08 18:10:11 +00:00
검색 hit / RAG citation 에 indexed_at + stale 두 wire 필드 추가.
documents.updated_at 재활용 (V006 incremental ingest 가 자연 source-of-truth).
config [search] stale_threshold_days = 30 default. additive minor wire.
TUI Warning role / CLI plain [stale] tag / agent --json 동시 surface.
자동 재 ingest 는 out of scope.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
15 tasks covering domain (kebab-core SearchHit + AnswerCitation),
config (SearchCfg.stale_threshold_days), retrievers (lexical + vector
JOIN documents.updated_at), App facade (staleness module + cache
re-stamp), wire schema, CLI plain [stale] tag, TUI [STALE] Warning
badge, snapshot fan-out, docs.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Domain field additions for p9-fb-32. Wire serialization is
automatic via serde rfc3339. Other crates fail to compile until
they populate the new fields — fixed in subsequent tasks.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
default 30 days. env override KEBAB_SEARCH_STALE_THRESHOLD_DAYS.
Malformed env values are silently ignored, matching the existing
apply_env pattern.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
JOIN documents.updated_at. stale defaults to false; App facade
post-processes against config threshold.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
hydrate_chunks now JOINs d.updated_at. Hybrid fusion path is
unchanged (passes SearchHit through, fields preserved).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
compute_stale: strict > boundary, threshold=0 disables, future
timestamps treated as fresh (clock skew safety). App::search
re-stamps on cache hit so config threshold changes take effect
without flushing the cache.

Also unblocks the workspace build by plugging placeholder
indexed_at/stale into the two AnswerCitation construction
sites in kebab-rag/pipeline.rs (the score-gate refusal path
forwards from SearchHit; the LLM-citation path uses
UNIX_EPOCH/false until Task 7 wires the real values through
pack_context).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Test helper missed the SearchHit field expansion from fb-32 Task 1.
UNIX_EPOCH + false placeholders consistent with the cross-crate
synthetic-mock pattern (hybrid.rs, vector.rs build_hit Task 4 stub).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
pack_context widened to carry indexed_at + stale alongside marker
and Citation. LLM-citation construction site now plumbs real values
from upstream SearchHit instead of the Task 6 UNIX_EPOCH placeholder.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- kebab-app::staleness::compute_stale gains note pointing at the
  kebab-rag mirror so future modifiers know to update both copies.
- kebab-rag::pipeline: doc comments adjacent to compute_stale and
  embedding_ref_for were positioned such that rustdoc would
  misattribute them. Reorder/separate so each comment hugs its
  own function.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Additive minor — schema_version unchanged. Existing v1 consumers
that ignore unknown fields stay compatible; consumers that validate
strictly will reject pre-fb-32 payloads, which matches the wire
contract escape hatch (recipient version >= producer required).

Cross-task placeholders: kebab-eval / kebab-tui synthetic test
fixtures pin UNIX_EPOCH + stale=false (same pattern as
hybrid.rs / vector.rs). These don't exercise staleness — Task 11
adds dedicated TUI staleness rendering tests.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Yellow when TTY, plain when not. JSON path inherits via serde
on the domain type; no CLI-side wire change needed there.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mirror of Task 9's search-output rendering: yellow [stale] on TTY,
plain text otherwise. JSON path inherits via serde on AnswerCitation.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
insta filter pattern '[indexed_at]' applied where snapshots
otherwise capture time-dependent RFC3339 strings.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
claude-reviewer-01 requested changes 2026-05-09 02:51:36 +00:00
claude-reviewer-01 left a comment
Member

회차 1 — fb-32 의 두 신호 (indexed_at + stale) 를 retriever → app facade → wire / TUI / CLI 까지 깔끔하게 흘려보내는 구현. boundary semantics (strict >, threshold=0 short-circuit, future timestamp = fresh) 가 5개 단위 테스트로 정확히 pin 되어 있고, App 캐시 hit 시 re-stamp 결정도 cache invalidation 없이 threshold 변경을 반영하는 좋은 trade-off 입니다. 다만 (1) 음수 threshold 가 spec 상 error.v1 config_invalid 로 떨어져야 한다고 명시되어 있는데 현재 테스트는 env path 의 silent-ignore 만 검증하면서 이름은 ..._rejected_at_validation 으로 잘못 붙어 있고, (2) kebab-rag::pipeline::compute_stale 미러는 의도된 중복이지만 단독 boundary 테스트가 없어 drift 가능성이 있고, (3) wire schema 의 required 추가가 strict validator 기준으로는 breaking 인데 이게 의식적 결정인지 확인이 필요합니다. CLI wire test 두 파일의 helper 중복, 코드 주석의 "Task 6" 같은 plan-step 참조 등 nit 도 몇 가지 같이 정리하면 좋겠습니다.

회차 1 — fb-32 의 두 신호 (`indexed_at` + `stale`) 를 retriever → app facade → wire / TUI / CLI 까지 깔끔하게 흘려보내는 구현. boundary semantics (strict `>`, threshold=0 short-circuit, future timestamp = fresh) 가 5개 단위 테스트로 정확히 pin 되어 있고, App 캐시 hit 시 re-stamp 결정도 cache invalidation 없이 threshold 변경을 반영하는 좋은 trade-off 입니다. 다만 (1) 음수 threshold 가 spec 상 `error.v1` config_invalid 로 떨어져야 한다고 명시되어 있는데 현재 테스트는 env path 의 silent-ignore 만 검증하면서 이름은 `..._rejected_at_validation` 으로 잘못 붙어 있고, (2) `kebab-rag::pipeline::compute_stale` 미러는 의도된 중복이지만 단독 boundary 테스트가 없어 drift 가능성이 있고, (3) wire schema 의 `required` 추가가 strict validator 기준으로는 breaking 인데 이게 의식적 결정인지 확인이 필요합니다. CLI wire test 두 파일의 helper 중복, 코드 주석의 "Task 6" 같은 plan-step 참조 등 nit 도 몇 가지 같이 정리하면 좋겠습니다.
@@ -194,0 +195,4 @@
// and an older threshold; if either has shifted (config
// reload, time passing) the cached `stale: false` may now
// be wrong. Re-stamping is cheap (per-hit comparison) and
// avoids invalidating the cache on threshold changes.

Strength: 캐시 히트 시 re-stamp 결정 — config threshold 가 바뀌어도 LRU 를 invalidate 할 필요 없게 만들어 cache hit 의 의미를 살리면서 stale 신호의 정확성도 유지하는 좋은 trade-off 입니다. 주석에서 "cached stale: false may now be wrong" 라는 양방향 (true→false, false→true 양쪽) reasoning 도 명확합니다. 다만 한 가지 — re-stamp 가 매번 Vec<SearchHit>::clone 을 강제합니다 (이전 fb-19 도 clone 이긴 했지만 그건 락 때문이었고 이번엔 mutate 때문). 캐시 hit 의 핫 경로 비용이 살짝 늘어났지만, per-hit 비교가 cheap 하다는 본 PR 의 reasoning 자체는 옳습니다. 그냥 인지 차원에서 코멘트.

Strength: 캐시 히트 시 re-stamp 결정 — config threshold 가 바뀌어도 LRU 를 invalidate 할 필요 없게 만들어 cache hit 의 의미를 살리면서 stale 신호의 정확성도 유지하는 좋은 trade-off 입니다. 주석에서 "cached `stale: false` may now be wrong" 라는 양방향 (true→false, false→true 양쪽) reasoning 도 명확합니다. 다만 한 가지 — re-stamp 가 매번 `Vec<SearchHit>::clone` 을 강제합니다 (이전 fb-19 도 clone 이긴 했지만 그건 락 때문이었고 이번엔 mutate 때문). 캐시 hit 의 핫 경로 비용이 살짝 늘어났지만, per-hit 비교가 cheap 하다는 본 PR 의 reasoning 자체는 옳습니다. 그냥 인지 차원에서 코멘트.
@@ -0,0 +10,4 @@
///
/// p9-fb-32: mirrored in `kebab_rag::pipeline::compute_stale` (dep-boundary
/// rule prevents `kebab-rag → kebab-app`). Update both together.
pub fn compute_stale(

Strengths: compute_stale 의 boundary 처리가 매우 깔끔합니다 — strict > 채택, threshold=0 short-circuit, future timestamp = fresh (clock skew 안전성), 그리고 5개 테스트가 정확히 이 4개 boundary 를 모두 pin 합니다 (exactly_threshold_is_fresh + one_minute_past_threshold_is_stale 가 strict > 를 양쪽으로 가둠). pure function + 도메인 의존성 최소화로 mirror 가능한 모양으로 잘 빠져 있습니다. 이 design 이 후속 fb-33/34/35 와 같은 query-path post-process 들의 reference pattern 이 될 수 있겠습니다.

Strengths: `compute_stale` 의 boundary 처리가 매우 깔끔합니다 — strict `>` 채택, threshold=0 short-circuit, future timestamp = fresh (clock skew 안전성), 그리고 5개 테스트가 정확히 이 4개 boundary 를 모두 pin 합니다 (`exactly_threshold_is_fresh` + `one_minute_past_threshold_is_stale` 가 strict `>` 를 양쪽으로 가둠). pure function + 도메인 의존성 최소화로 mirror 가능한 모양으로 잘 빠져 있습니다. 이 design 이 후속 fb-33/34/35 와 같은 query-path post-process 들의 reference pattern 이 될 수 있겠습니다.
@@ -0,0 +67,4 @@
temperature = 0.0
seed = 0
[search]

write_config, ingest, backdate_updated_atwire_ask_stale.rs 와 거의 글자 단위로 동일하게 중복되어 있습니다 (config 본문 100여 줄, helper 함수 3개 × 2). crates/kebab-cli/tests/common/mod.rs 가 이미 다른 wire test 들에서 쓰일 만한 위치이고, crates/kebab-app/tests/common/mod.rs::backdate_document_updated_at 와도 거의 동일한 SQL UPDATE 헬퍼라 — 한 군데로 합치면 후속 wire test 들이 같은 dependency 를 또 추가하는 패턴을 끊을 수 있습니다. 이번 PR scope 밖이라면 follow-up 으로라도 남겨두는 것을 권장합니다.

`write_config`, `ingest`, `backdate_updated_at` 가 `wire_ask_stale.rs` 와 거의 글자 단위로 동일하게 중복되어 있습니다 (config 본문 100여 줄, helper 함수 3개 × 2). `crates/kebab-cli/tests/common/mod.rs` 가 이미 다른 wire test 들에서 쓰일 만한 위치이고, `crates/kebab-app/tests/common/mod.rs::backdate_document_updated_at` 와도 거의 동일한 SQL UPDATE 헬퍼라 — 한 군데로 합치면 후속 wire test 들이 같은 dependency 를 또 추가하는 패턴을 끊을 수 있습니다. 이번 PR scope 밖이라면 follow-up 으로라도 남겨두는 것을 권장합니다.
@@ -984,0 +1016,4 @@
}
#[test]
fn negative_stale_threshold_rejected_at_validation() {

테스트 이름이 본문과 일치하지 않아 보입니다. negative_stale_threshold_rejected_at_validation 라는 이름은 "validation 단계에서 음수가 거부된다" 는 인상을 주지만, 실제 코드는 apply_env 의 silent-ignore 동작 ("garbage env value must not corrupt the default") 을 검증합니다. 더불어 spec §Config 는 "음수 threshold → Config::load error (error.v1.code = config_invalid)" 를 명시하는데 — TOML 리터럴 stale_threshold_days = -5 가 실제로 ConfigInvalid 로 떨어지는 경로를 검증하는 테스트가 없습니다. u32 직렬화 실패 시 toml 단계에서 자연스럽게 에러가 나는 것은 사실이지만 spec 의 contract 를 명시적으로 pin 하려면 from_file 으로 음수 TOML 을 읽어 ConfigInvalid 를 downcast 검증하는 테스트가 필요해 보입니다 (인접 fb27_tests::config_invalid_on_malformed_toml 패턴 그대로 응용 가능). 최소한 테스트 이름은 env_negative_threshold_silently_ignored 처럼 동작과 일치시키는 것을 권장합니다.

테스트 이름이 본문과 일치하지 않아 보입니다. `negative_stale_threshold_rejected_at_validation` 라는 이름은 "validation 단계에서 음수가 거부된다" 는 인상을 주지만, 실제 코드는 `apply_env` 의 silent-ignore 동작 ("garbage env value must not corrupt the default") 을 검증합니다. 더불어 spec §Config 는 "음수 threshold → `Config::load` error (`error.v1.code` = config_invalid)" 를 명시하는데 — TOML 리터럴 `stale_threshold_days = -5` 가 실제로 `ConfigInvalid` 로 떨어지는 경로를 검증하는 테스트가 없습니다. u32 직렬화 실패 시 toml 단계에서 자연스럽게 에러가 나는 것은 사실이지만 spec 의 contract 를 명시적으로 pin 하려면 `from_file` 으로 음수 TOML 을 읽어 `ConfigInvalid` 를 downcast 검증하는 테스트가 필요해 보입니다 (인접 `fb27_tests::config_invalid_on_malformed_toml` 패턴 그대로 응용 가능). 최소한 테스트 이름은 `env_negative_threshold_silently_ignored` 처럼 동작과 일치시키는 것을 권장합니다.
@@ -47,0 +50,4 @@
/// LLM-citation construction site can build a complete
/// [`kebab_core::AnswerCitation`] (p9-fb-32).
#[derive(Clone, Debug)]
struct PackedCitation {

Nit: PackedCitation 의 4개 필드 중 staleRagPipeline::ask 진입부에서 stamp 된 후 pack_context 가 단순히 forward 합니다. 이 "stale 은 caller 가 미리 채워야 한다" 는 invariant 가 struct doc 에 없습니다. 현재 사용처는 한 곳뿐이라 큰 문제는 아니지만, future maintainer 가 이 struct 를 다른 경로에서 만들면 stale 이 의미를 잃을 수 있습니다. doc 에 "NOTE: stale is expected to already be stamped against the configured threshold by the caller of pack_context; this struct just forwards." 정도의 한 줄 추가를 권장.

Nit: `PackedCitation` 의 4개 필드 중 `stale` 은 `RagPipeline::ask` 진입부에서 stamp 된 후 `pack_context` 가 단순히 forward 합니다. 이 "stale 은 caller 가 미리 채워야 한다" 는 invariant 가 struct doc 에 없습니다. 현재 사용처는 한 곳뿐이라 큰 문제는 아니지만, future maintainer 가 이 struct 를 다른 경로에서 만들면 stale 이 의미를 잃을 수 있습니다. doc 에 "NOTE: `stale` is expected to already be stamped against the configured threshold by the caller of `pack_context`; this struct just forwards." 정도의 한 줄 추가를 권장.
@@ -628,0 +687,4 @@
}
let threshold = time::Duration::days(i64::from(threshold_days));
(now - indexed_at) > threshold
}

compute_stale 의 의도된 미러링은 dep-boundary 상 합당한 결정이고 doc 에 그 이유까지 명시되어 좋습니다. 다만 — 이 미러 함수에 대한 단독 unit test 가 본 crate 에 없습니다. 현재는 통합 테스트 (grounded_citations_inherit_indexed_at_and_stale_from_hit 등) 에서만 간접 검증되는데, app 측 compute_stale 의 boundary tests (exactly_threshold_is_fresh, one_minute_past_threshold_is_stale, future_indexed_at_is_fresh) 와 동치임을 회귀 단위에서 보장하지 못합니다. 미래에 한쪽이 drift 했을 때 통합 테스트만으로 잡힐 가능성은 낮습니다. 동일한 5개 boundary 테스트를 본 모듈 #[cfg(test)] 에 복사해 미러의 의미적 동치성을 명시적으로 pin 하는 게 안전해 보입니다.

`compute_stale` 의 의도된 미러링은 dep-boundary 상 합당한 결정이고 doc 에 그 이유까지 명시되어 좋습니다. 다만 — 이 미러 함수에 대한 단독 unit test 가 본 crate 에 없습니다. 현재는 통합 테스트 (`grounded_citations_inherit_indexed_at_and_stale_from_hit` 등) 에서만 간접 검증되는데, app 측 `compute_stale` 의 boundary tests (`exactly_threshold_is_fresh`, `one_minute_past_threshold_is_stale`, `future_indexed_at_is_fresh`) 와 동치임을 회귀 단위에서 보장하지 못합니다. 미래에 한쪽이 drift 했을 때 통합 테스트만으로 잡힐 가능성은 낮습니다. 동일한 5개 boundary 테스트를 본 모듈 `#[cfg(test)]` 에 복사해 미러의 의미적 동치성을 명시적으로 pin 하는 게 안전해 보입니다.
@@ -404,1 +418,4 @@
chunker_version: ChunkerVersion(raw.chunker_version),
indexed_at,
// Placeholder — App layer overwrites against config threshold (Task 6).
stale: false,

Nit: 주석 // Placeholder — App layer overwrites against config threshold (Task 6). 의 "Task 6" 은 plan 문서의 step 번호입니다. plan 은 PR 머지 후에는 historical artifact 가 되고 코드 리더는 "Task 6" 이 무엇을 가리키는지 추적이 어렵습니다. App::search / RagPipeline::ask 의 post-process 가 덮어쓴다고 직접 풀어 쓰거나 (App::search / ask 진입부에서 mark_stale_in_place / compute_stale 으로 덮어씀), kebab_app::staleness::mark_stale_in_place 함수명을 참조하는 형태가 후속 독자에게 더 친절합니다. vector.rs 의 동일 주석에도 동일 적용 권장.

Nit: 주석 `// Placeholder — App layer overwrites against config threshold (Task 6).` 의 "Task 6" 은 plan 문서의 step 번호입니다. plan 은 PR 머지 후에는 historical artifact 가 되고 코드 리더는 "Task 6" 이 무엇을 가리키는지 추적이 어렵습니다. `App::search` / `RagPipeline::ask` 의 post-process 가 덮어쓴다고 직접 풀어 쓰거나 (`App::search` / `ask` 진입부에서 `mark_stale_in_place` / `compute_stale` 으로 덮어씀), `kebab_app::staleness::mark_stale_in_place` 함수명을 참조하는 형태가 후속 독자에게 더 친절합니다. `vector.rs` 의 동일 주석에도 동일 적용 권장.
@@ -105,2 +123,4 @@
let mut lines: Vec<Line> = Vec::new();
// Header
let now = time::OffsetDateTime::now_utc();
let stale = kebab_app::compute_stale(doc.metadata.updated_at, now, threshold_days);

Inspect 패널의 stale 계산이 App::search / RagPipeline::ask 와 코드 경로가 다릅니다 — 여기는 doc.metadata.updated_at 를 직접 읽고 kebab_app::compute_stale 을 부르는 query-time compute, 반면 search/ask 는 retriever 단에서 indexed_at 추출 후 post-process. 결과는 같아야 하지만 "같은 doc 의 stale 판정 source 가 두 군데" 라는 점은 가독성 측면에서 약간 헛갈립니다. 현재 구조는 InspectSearchHit 를 거치지 않으니 어쩔 수 없는 선택이긴 합니다 — 다만 SearchHit::indexed_at 의 source-of-truth (documents.updated_at) 와 동일하다는 사실은 주석으로 한 줄 묶어두면 미래 독자가 "왜 두 군데서 같은 계산을?" 에 빠지지 않습니다.

Inspect 패널의 stale 계산이 `App::search` / `RagPipeline::ask` 와 코드 경로가 다릅니다 — 여기는 `doc.metadata.updated_at` 를 직접 읽고 `kebab_app::compute_stale` 을 부르는 query-time compute, 반면 search/ask 는 retriever 단에서 `indexed_at` 추출 후 post-process. 결과는 같아야 하지만 "같은 doc 의 stale 판정 source 가 두 군데" 라는 점은 가독성 측면에서 약간 헛갈립니다. 현재 구조는 `Inspect` 가 `SearchHit` 를 거치지 않으니 어쩔 수 없는 선택이긴 합니다 — 다만 SearchHit::indexed_at 의 source-of-truth (documents.updated_at) 와 동일하다는 사실은 주석으로 한 줄 묶어두면 미래 독자가 "왜 두 군데서 같은 계산을?" 에 빠지지 않습니다.
@@ -17,3 +17,3 @@
"retrieval",
"index_version",
"chunker_version"
"chunker_version",

indexed_atstalerequired 에 추가한 것 — additive 처럼 보이지만 strict JSON Schema validator 입장에서는 pre-fb-32 payload 가 invalid 가 됩니다. plan task 8 의 commit message 자체도 "Existing v1 consumers that ignore unknown fields stay compatible; consumers that validate strictly will reject pre-fb-32 payloads" 라고 인지하고 있습니다. CLAUDE.md 의 wire-schema 정책 ("breaking it requires a v2 major bump and parallel-running both for one phase") 과 충돌 여지가 있습니다. additive minor 로 다루려면 required 가 아니라 properties 만 추가하고 producer 측에서만 항상 채우는 형태가 더 안전한 옵션입니다. 본 PR 의 단일 사용자 / 단일 producer 환경에서는 실용적으로 문제가 없겠지만, 정책 정합성 차원에서 의식적 결정이었는지 확인 필요합니다 (의식적이라면 HOTFIXES.md 같은 곳에 "v1 가 strict validation 측면에서 한 번 깨졌다" 는 결정 로그가 어울립니다).

`indexed_at` 와 `stale` 을 `required` 에 추가한 것 — additive 처럼 보이지만 strict JSON Schema validator 입장에서는 pre-fb-32 payload 가 invalid 가 됩니다. plan task 8 의 commit message 자체도 "Existing v1 consumers that ignore unknown fields stay compatible; consumers that validate strictly will reject pre-fb-32 payloads" 라고 인지하고 있습니다. CLAUDE.md 의 wire-schema 정책 ("breaking it requires a v2 major bump and parallel-running both for one phase") 과 충돌 여지가 있습니다. additive minor 로 다루려면 `required` 가 아니라 `properties` 만 추가하고 producer 측에서만 항상 채우는 형태가 더 안전한 옵션입니다. 본 PR 의 단일 사용자 / 단일 producer 환경에서는 실용적으로 문제가 없겠지만, 정책 정합성 차원에서 의식적 결정이었는지 확인 필요합니다 (의식적이라면 HOTFIXES.md 같은 곳에 "v1 가 strict validation 측면에서 한 번 깨졌다" 는 결정 로그가 어울립니다).
@@ -102,6 +102,7 @@ Claude Code spawns `kebab mcp` at session start; the process stays alive across
- `search` output can be large for broad queries. Project relevant fields when summarizing — for CLI: `jq '.[] | {rank, doc_path, heading: .heading_path[-1], snippet}'`.
- `ask`'s `citations[]` mirrors `search_hit.v1` minus retrieval internals — same `doc_path` / `citation` shape.
- Schema reference lives in the kebab repo at `docs/wire-schema/v1/*.schema.json` if a field is unclear.
- `search_hit.v1` and `answer.v1.citations[]` carry `indexed_at` (RFC3339) + `stale` (bool). When `stale == true`, the source doc hasn't been re-processed since `config.search.stale_threshold_days`. Surface this caveat to the user when summarizing — the cited snapshot may not reflect current reality.

Strength: skill 의 parsing tip 에 "Surface this caveat to the user when summarizing — the cited snapshot may not reflect current reality" 까지 적어준 점이 좋습니다. agent 가 단순히 필드를 인지하는 수준이 아니라 사용자 응답에 반영해야 하는 행동 지침까지 명시되어 있어, fb-32 의 본래 목적 ("근거 최신성 약점을 즉시 인지") 이 LLM 측 surface 까지 잘 이어집니다.

Strength: skill 의 parsing tip 에 "Surface this caveat to the user when summarizing — the cited snapshot may not reflect current reality" 까지 적어준 점이 좋습니다. agent 가 단순히 필드를 인지하는 수준이 아니라 사용자 응답에 반영해야 하는 행동 지침까지 명시되어 있어, fb-32 의 본래 목적 ("근거 최신성 약점을 즉시 인지") 이 LLM 측 surface 까지 잘 이어집니다.
altair823 added 1 commit 2026-05-09 03:05:22 +00:00
- config: rename env-silent-ignore test + add file-load negative test
  asserting ConfigInvalid for negative TOML stale_threshold_days
- rag: add 5 boundary unit tests pinning compute_stale mirror equivalence
- search: rewrite "Task 6" plan refs in lexical/vector to point at
  actual function names (mark_stale_in_place / RagPipeline::ask)
- cli: dedupe write_config / ingest / backdate_updated_at helpers
  from wire_search_stale + wire_ask_stale into tests/common/mod.rs
- tui: clarify inspect.rs uses same source-of-truth as SearchHit
- rag: PackedCitation.stale invariant doc comment
- HOTFIXES: log conscious decision on wire-schema required-field
  expansion (strict-validator concern)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
claude-reviewer-01 requested changes 2026-05-09 03:11:02 +00:00
claude-reviewer-01 left a comment
Member

회차 2 — 라운드 1 의 7개 fix 는 대부분 깔끔하게 들어왔다. 새 negative TOML test (file_negative_stale_threshold_returns_config_invalid) 는 ConfigInvalid downcast + parse_failed cause 까지 정확히 검증하고, 5개 boundary mirror tests 는 kebab-app::staleness 의 test set 과 1:1 대응, CLI helper dedupe 는 write_config / write_config_with_llm_model split 로 두 consumer 의 차이를 깔끔히 흡수했다. 다만 round 1 변경 자체에서 한 가지 spec-contract 위반을 발견 — HOTFIXES 에 새 entry 를 추가했지만 tasks/p9/p9-fb-32-stale-doc-indicator.md 에 그 entry 로의 cross-link 한 줄이 빠져 있다 (CLAUDE.md Spec contract 절의 명시 요구). 추가로 HOTFIXES heading 의 fb-32 표기가 다른 entry 의 p9-fb-NN 컨벤션과 어긋나는 문제, 그리고 negative TOML test 가 default round-trip + string-replace 패턴이라 default 값이 미래에 바뀌면 진단 메시지가 한 단계 멀어지는 robustness nit 까지 총 3건. spec-contract 위반은 phase 5+ reader 의 live-truth 추적을 끊는 항목이라 blocker 로 분류.

회차 2 — 라운드 1 의 7개 fix 는 대부분 깔끔하게 들어왔다. 새 negative TOML test (`file_negative_stale_threshold_returns_config_invalid`) 는 ConfigInvalid downcast + `parse_failed` cause 까지 정확히 검증하고, 5개 boundary mirror tests 는 `kebab-app::staleness` 의 test set 과 1:1 대응, CLI helper dedupe 는 `write_config` / `write_config_with_llm_model` split 로 두 consumer 의 차이를 깔끔히 흡수했다. 다만 round 1 변경 자체에서 한 가지 spec-contract 위반을 발견 — HOTFIXES 에 새 entry 를 추가했지만 `tasks/p9/p9-fb-32-stale-doc-indicator.md` 에 그 entry 로의 cross-link 한 줄이 빠져 있다 (CLAUDE.md `Spec contract` 절의 명시 요구). 추가로 HOTFIXES heading 의 `fb-32` 표기가 다른 entry 의 `p9-fb-NN` 컨벤션과 어긋나는 문제, 그리고 negative TOML test 가 default round-trip + string-replace 패턴이라 default 값이 미래에 바뀌면 진단 메시지가 한 단계 멀어지는 robustness nit 까지 총 3건. spec-contract 위반은 phase 5+ reader 의 live-truth 추적을 끊는 항목이라 blocker 로 분류.
@@ -1030,0 +1095,4 @@
// under test — this isolates the failure to the negative
// value rather than missing required sections.
let cfg = Config::defaults();
let mut toml_text = toml::to_string(&cfg).expect("default round-trips");

Defensive nit — 본 test 는 Config::defaults() round-trip TOML 에 대해 replace("stale_threshold_days = 30", "stale_threshold_days = -5") 로 negative 주입한다. 만약 default_stale_threshold_days 가 미래에 30 → (예) 14 로 바뀌면 replace 가 no-op 이 되고, 결과 TOML 은 valid → Config::from_fileOk 반환 → unwrap_err() 가 panic 한다. 소리는 큼 (loud failure) 이지만 실패 메시지가 'replace 가 no-op 됐다' 가 아니라 'called Result::unwrap_err() on an Ok value' 라 원인 진단이 한 단계 멀다.

방어책 — replace 직전에 assert!(toml_text.contains("stale_threshold_days = 30"), "default value drifted; update test fixture"); 한 줄, 또는 default round-trip 대신 toml-edit 으로 [search] 만 inline 구성하는 쪽이 미래의 default 변경에 더 견고. 본 round 의 blocker 는 아니다.

Defensive nit — 본 test 는 `Config::defaults()` round-trip TOML 에 대해 `replace("stale_threshold_days = 30", "stale_threshold_days = -5")` 로 negative 주입한다. 만약 `default_stale_threshold_days` 가 미래에 30 → (예) 14 로 바뀌면 `replace` 가 no-op 이 되고, 결과 TOML 은 valid → `Config::from_file` 가 `Ok` 반환 → `unwrap_err()` 가 panic 한다. **소리는 큼** (loud failure) 이지만 실패 메시지가 'replace 가 no-op 됐다' 가 아니라 'called `Result::unwrap_err()` on an `Ok` value' 라 원인 진단이 한 단계 멀다. 방어책 — replace 직전에 `assert!(toml_text.contains("stale_threshold_days = 30"), "default value drifted; update test fixture");` 한 줄, 또는 default round-trip 대신 toml-edit 으로 `[search]` 만 inline 구성하는 쪽이 미래의 default 변경에 더 견고. 본 round 의 blocker 는 아니다.
@@ -14,6 +14,20 @@ historical contract that was implemented; this file accumulates the
deltas so phase 5+ readers can find the live behavior without diffing
git history.
## 2026-05-09 — fb-32: search_hit.v1 / citation.v1 required-field expansion

Heading naming nit — 다른 entry 는 모두 p9-fb-NN 풀 ID 사용 (2026-05-07 — p9-fb-31, 2026-05-07 — p9-fb-30, 2026-05-04 — p9-fb-22, …). 일관성 위해 ## 2026-05-09 — p9-fb-32: search_hit.v1 / citation.v1 required-field expansion 로 바꾸는 게 좋다. 본문 prose 안에서는 fb-32 줄여 써도 무방하지만 heading 은 grep target 이라 prefix 통일이 중요.

Heading naming nit — 다른 entry 는 모두 `p9-fb-NN` 풀 ID 사용 (`2026-05-07 — p9-fb-31`, `2026-05-07 — p9-fb-30`, `2026-05-04 — p9-fb-22`, …). 일관성 위해 `## 2026-05-09 — p9-fb-32: search_hit.v1 / citation.v1 required-field expansion` 로 바꾸는 게 좋다. 본문 prose 안에서는 `fb-32` 줄여 써도 무방하지만 heading 은 grep target 이라 prefix 통일이 중요.
@@ -16,2 +16,3 @@
> ⏳ **백로그 only — 미구현.** 본 spec 은 도그푸딩 피드백 skeleton. 구현 착수 전 [superpowers:brainstorming](../../docs/superpowers/) 으로 설계 단계 선행 필요. stale threshold 정책 / "stale" 정의 (ingest 시점 vs file mtime) / wire schema 필드 위치 brainstorm 후 확정.
상세 설계: `docs/superpowers/specs/2026-05-08-p9-fb-32-stale-doc-indicator-design.md`.
구현 계획: `docs/superpowers/plans/2026-05-09-p9-fb-32-stale-doc-indicator.md`.

Spec-contract violation. CLAUDE.md Spec contract 절은 'Live deviations from the original contract go in tasks/HOTFIXES.md as dated entries, plus a one-line cross-link in the original spec's Risks / notes' 라고 못박는다. 본 PR 의 round 1 에서 HOTFIXES 에 2026-05-09 — fb-32: search_hit.v1 / citation.v1 required-field expansion 항목을 추가했지만 이 spec 파일에는 해당 entry 로의 cross-link 이 없다.

선례 패턴 — p9-fb-22 / p9-fb-25 / p9-fb-27 / p9-fb-30 — 은 모두 # p9-fb-NN — Title 직후에 다음 형태의 한 줄 callout 을 추가:

> ✅ **구현 완료.** 본 spec 은 구현 시점의 frozen 상태. post-merge deviation (특히 search_hit.v1 / citation.v1 의 required-field 확장) 은 [HOTFIXES.md](../HOTFIXES.md) 의 `2026-05-09 — p9-fb-32` 항목 참조 — live source of truth.

이 한 줄을 spec 의 title 직하 — 현재 18번 라인의 구현 계획: 직전 또는 직후 — 에 추가해야 frozen spec 을 읽는 phase 5+ reader 가 live behavior 까지 한 번에 따라갈 수 있다.

Spec-contract violation. CLAUDE.md `Spec contract` 절은 'Live deviations from the original contract go in `tasks/HOTFIXES.md` as dated entries, **plus a one-line cross-link in the original spec's `Risks / notes`**' 라고 못박는다. 본 PR 의 round 1 에서 HOTFIXES 에 `2026-05-09 — fb-32: search_hit.v1 / citation.v1 required-field expansion` 항목을 추가했지만 이 spec 파일에는 해당 entry 로의 cross-link 이 없다. 선례 패턴 — p9-fb-22 / p9-fb-25 / p9-fb-27 / p9-fb-30 — 은 모두 `# p9-fb-NN — Title` 직후에 다음 형태의 한 줄 callout 을 추가: ``` > ✅ **구현 완료.** 본 spec 은 구현 시점의 frozen 상태. post-merge deviation (특히 search_hit.v1 / citation.v1 의 required-field 확장) 은 [HOTFIXES.md](../HOTFIXES.md) 의 `2026-05-09 — p9-fb-32` 항목 참조 — live source of truth. ``` 이 한 줄을 spec 의 title 직하 — 현재 18번 라인의 `구현 계획:` 직전 또는 직후 — 에 추가해야 frozen spec 을 읽는 phase 5+ reader 가 live behavior 까지 한 번에 따라갈 수 있다.
altair823 added 1 commit 2026-05-09 03:19:52 +00:00
- spec: add one-line cross-link to HOTFIXES entry per CLAUDE.md
  Spec-contract policy
- HOTFIXES: rename heading from "fb-32" to "p9-fb-32" matching
  the rest of the file's full-ID convention
- config: defensive assert before string-replace in negative TOML
  test guards against default-value drift causing unhelpful unwrap

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
claude-reviewer-01 approved these changes 2026-05-09 03:22:13 +00:00
claude-reviewer-01 left a comment
Member

회차 3 — 라운드 2 의 세 가지 fix (spec cross-link 추가, HOTFIXES heading 을 p9-fb-32 풀 ID 로 통일, 음수 stale_threshold 테스트의 default-drift 방어 assert) 모두 깔끔하게 반영됨. Cross-link 형식은 p9-fb-27 / p9-fb-30 spec 의 line 17 패턴과 정확히 일치하고, HOTFIXES heading 도 같은 파일 내 다른 entry 의 풀 ID 컨벤션을 따른다. 방어 assert 메시지 (default value drifted; update test fixture) 도 미래의 default 변경 시 fail-fast + 수정 지점 안내가 명확함. 라운드 2 변경으로 인한 새 issue / regression 없음 — 컨버전스 기준 충족, APPROVE.

회차 3 — 라운드 2 의 세 가지 fix (spec cross-link 추가, HOTFIXES heading 을 `p9-fb-32` 풀 ID 로 통일, 음수 stale_threshold 테스트의 default-drift 방어 assert) 모두 깔끔하게 반영됨. Cross-link 형식은 p9-fb-27 / p9-fb-30 spec 의 line 17 패턴과 정확히 일치하고, HOTFIXES heading 도 같은 파일 내 다른 entry 의 풀 ID 컨벤션을 따른다. 방어 assert 메시지 (`default value drifted; update test fixture`) 도 미래의 default 변경 시 fail-fast + 수정 지점 안내가 명확함. 라운드 2 변경으로 인한 새 issue / regression 없음 — 컨버전스 기준 충족, APPROVE.
@@ -1030,0 +1096,4 @@
// value rather than missing required sections.
let cfg = Config::defaults();
let mut toml_text = toml::to_string(&cfg).expect("default round-trips");
assert!(

방어용 assert 메시지가 의도를 잘 전달함 — default value drifted; update test fixture 한 줄이면 미래의 default 변경 시 silent unwrap()-like no-op 가 아니라 명시적 실패 + 수정 지점 안내. 추가 비용도 미미.

방어용 assert 메시지가 의도를 잘 전달함 — `default value drifted; update test fixture` 한 줄이면 미래의 default 변경 시 silent `unwrap()`-like no-op 가 아니라 명시적 실패 + 수정 지점 안내. 추가 비용도 미미.
@@ -14,6 +14,20 @@ historical contract that was implemented; this file accumulates the
deltas so phase 5+ readers can find the live behavior without diffing
git history.
## 2026-05-09 — p9-fb-32: search_hit.v1 / citation.v1 required-field expansion

Heading 의 task ID 를 fb-32p9-fb-32 로 통일한 것 깔끔. 같은 파일 내 fb-31 / fb-30 / fb-27 / fb-25 등 모든 entry 가 p9-fb-XX 풀 ID 를 쓰고 있어 grep / cross-ref 일관성 회복.

Heading 의 task ID 를 `fb-32` → `p9-fb-32` 로 통일한 것 깔끔. 같은 파일 내 fb-31 / fb-30 / fb-27 / fb-25 등 모든 entry 가 `p9-fb-XX` 풀 ID 를 쓰고 있어 grep / cross-ref 일관성 회복.
@@ -15,3 +15,3 @@
# p9-fb-32 — Stale doc indicator
> **백로그 only — 미구현.** 본 spec 은 도그푸딩 피드백 skeleton. 구현 착수 전 [superpowers:brainstorming](../../docs/superpowers/) 으로 설계 단계 선행 필요. stale threshold 정책 / "stale" 정의 (ingest 시점 vs file mtime) / wire schema 필드 위치 brainstorm 후 확정.
> **구현 완료.** 본 spec 은 구현 시점의 frozen 상태. post-merge deviation (특히 search_hit.v1 / citation.v1 의 required-field 확장) 은 [HOTFIXES.md](../HOTFIXES.md) 의 `2026-05-09 — p9-fb-32` 항목 참조 — live source of truth.

Cross-link 한 줄, 형식이 p9-fb-27 (line 17) / p9-fb-30 (line 17) 과 정확히 일치 — > ✅ **구현 완료.** ... [HOTFIXES.md](../HOTFIXES.md) 의 YYYY-MM-DD — p9-fb-XX 항목 참조 — live source of truth. 패턴 그대로. CLAUDE.md spec-contract 정책 충족.

Cross-link 한 줄, 형식이 p9-fb-27 (line 17) / p9-fb-30 (line 17) 과 정확히 일치 — `> ✅ **구현 완료.** ... [HOTFIXES.md](../HOTFIXES.md) 의 `YYYY-MM-DD — p9-fb-XX` 항목 참조 — live source of truth.` 패턴 그대로. CLAUDE.md spec-contract 정책 충족.
altair823 merged commit 6a01f15261 into main 2026-05-09 03:26:06 +00:00
altair823 deleted branch feat/fb-32-stale-doc-indicator 2026-05-09 03:26:45 +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#122