feat(fb-34): output budget controls #125

Merged
altair823 merged 14 commits from feat/fb-34-output-budget-controls into main 2026-05-09 12:52:40 +00:00
Owner

Summary

  • adds kebab search --max-tokens / --snippet-chars / --cursor plus the equivalent inputs on mcp__kebab__search
  • wraps --json output in search_response.v1 ({hits, next_cursor, truncated}) — wire breaking; agent that parses bare search_hit.v1[] must adapt
  • token estimation = chars/4 (no tokenizer dep); truncate priority: snippet shorten → k pop → ≥1 hit floor
  • cursor = opaque base64({offset, corpus_revision}); mismatch returns error.v1.code = stale_cursor
  • App::search signature preserved as thin wrapper — TUI / kebab-rag callers unaffected
  • ask path scope out (rag.max_context_tokens already covers it)

Test plan

  • cargo test --workspace --no-fail-fast -j 1 — 129 test result OK / 0 FAILED
  • cargo clippy --workspace --all-targets -- -D warnings — clean (allow on cursor::decode for result_large_err — ErrorV1 is workspace-wide wire type, boxing pointless)
  • new tests:
    • cursor (kebab-app): encode/decode round-trip + stale_cursor mismatch (3 tests)
    • search_budget_integration (kebab-app): passthrough + snippet shorten + cursor pagination + corpus_revision bump (4 tests)
    • wire_search_response (kebab-cli): wire wrapper + max-tokens truncation + cursor pagination + plain stderr hint (4 tests, lexical-only)
    • tools_call_search (kebab-mcp): updated to assert search_response.v1 wrapper

Architectural notes

  • App::search signature unchanged → TUI / kebab-rag callers unaffected.
  • App::search_with_opts is the new public API; CLI / MCP go through it via search_with_opts_with_config.
  • chars/4 token estimation matches rag::pack_context convention; no tokenizer dep added.
  • Cursor opaque on purpose — internal schema may change; agent must not parse.
  • Plan deviation: k_effective uses query.k when nonzero (fall back to default_k only when k==0) instead of plan's query.k.max(default_k) — the max form forced k=2 → 10 and broke pagination semantics.
  • Wire breaking documented in HOTFIXES 2026-05-09 — p9-fb-34.

Files of interest

  • spec: docs/superpowers/specs/2026-05-09-p9-fb-34-output-budget-controls-design.md
  • plan: docs/superpowers/plans/2026-05-09-p9-fb-34-output-budget-controls.md
  • core: crates/kebab-core/src/search.rs (SearchOpts)
  • app: crates/kebab-app/src/{cursor,app}.rs (SearchResponse + budget loop)
  • CLI: crates/kebab-cli/src/main.rs (Cmd::Search), crates/kebab-cli/src/wire.rs
  • MCP: crates/kebab-mcp/src/tools/search.rs
  • wire: docs/wire-schema/v1/search_response.schema.json
## Summary - adds `kebab search --max-tokens / --snippet-chars / --cursor` plus the equivalent inputs on `mcp__kebab__search` - wraps `--json` output in `search_response.v1` (`{hits, next_cursor, truncated}`) — wire breaking; agent that parses bare `search_hit.v1[]` must adapt - token estimation = `chars/4` (no tokenizer dep); truncate priority: snippet shorten → k pop → ≥1 hit floor - cursor = opaque base64(`{offset, corpus_revision}`); mismatch returns `error.v1.code = stale_cursor` - `App::search` signature preserved as thin wrapper — TUI / kebab-rag callers unaffected - ask path scope out (rag.max_context_tokens already covers it) ## Test plan - [x] `cargo test --workspace --no-fail-fast -j 1` — 129 test result OK / 0 FAILED - [x] `cargo clippy --workspace --all-targets -- -D warnings` — clean (allow on cursor::decode for `result_large_err` — ErrorV1 is workspace-wide wire type, boxing pointless) - [x] new tests: - `cursor` (kebab-app): encode/decode round-trip + stale_cursor mismatch (3 tests) - `search_budget_integration` (kebab-app): passthrough + snippet shorten + cursor pagination + corpus_revision bump (4 tests) - `wire_search_response` (kebab-cli): wire wrapper + max-tokens truncation + cursor pagination + plain stderr hint (4 tests, lexical-only) - `tools_call_search` (kebab-mcp): updated to assert `search_response.v1` wrapper ## Architectural notes - `App::search` signature unchanged → TUI / kebab-rag callers unaffected. - `App::search_with_opts` is the new public API; CLI / MCP go through it via `search_with_opts_with_config`. - `chars/4` token estimation matches `rag::pack_context` convention; no tokenizer dep added. - Cursor opaque on purpose — internal schema may change; agent must not parse. - Plan deviation: `k_effective` uses `query.k` when nonzero (fall back to `default_k` only when k==0) instead of plan's `query.k.max(default_k)` — the `max` form forced k=2 → 10 and broke pagination semantics. - Wire breaking documented in HOTFIXES `2026-05-09 — p9-fb-34`. ## Files of interest - spec: `docs/superpowers/specs/2026-05-09-p9-fb-34-output-budget-controls-design.md` - plan: `docs/superpowers/plans/2026-05-09-p9-fb-34-output-budget-controls.md` - core: `crates/kebab-core/src/search.rs` (SearchOpts) - app: `crates/kebab-app/src/{cursor,app}.rs` (SearchResponse + budget loop) - CLI: `crates/kebab-cli/src/main.rs` (Cmd::Search), `crates/kebab-cli/src/wire.rs` - MCP: `crates/kebab-mcp/src/tools/search.rs` - wire: `docs/wire-schema/v1/search_response.schema.json`
altair823 added 12 commits 2026-05-09 11:22:55 +00:00
`kebab search` 에 --max-tokens / --snippet-chars / --cursor 신규.
chars/4 token approximation. truncate priority: snippet → k → 멈춤
(최소 1 hit 보장). cursor = opaque base64(offset + corpus_revision)
— mismatch 시 error.v1.code = stale_cursor.

wire breaking: stdout array → search_response.v1 wrapper. agent 갱신
필요. App::search 시그니처는 thin wrapper 로 보존 (TUI 무영향).

ask path 는 scope out (rag.max_context_tokens 가 이미 budget 담당).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
11 tasks: SearchOpts (kebab-core), cursor module + base64 dep
(kebab-app), error_wire stale_cursor convention, App::search_with_opts
+ SearchResponse + budget loop, wire schema search_response.v1, CLI
flags + plain truncated hint, CLI integration tests, MCP wrapper +
inputs, workspace+clippy gate, docs (README/SMOKE/INDEX/HOTFIXES/
skill), smoke+PR.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3 optional knobs (max_tokens, snippet_chars, cursor); Default = all
None = no enforcement (backwards-compat existing search behavior).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Opaque base64(JSON{offset, corpus_revision}). Mismatch or
malformed input returns ErrorV1 with code = stale_cursor.
base64 promoted to workspace dep.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
stale_cursor is built by cursor::decode, not classify. Test
locks the invariant.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Budget loop: snippet shorten → k pop → ≥1 hit floor. Cursor
encode/decode threads corpus_revision; mismatch surfaces as
stale_cursor anyhow error. App::search retained as thin wrapper.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wrapper around search_hit.v1[] with next_cursor + truncated.
Wire breaking — agent that parses bare array must adapt.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
JSON output wrapped in search_response.v1 (breaking — agent must
adapt). Plain output unchanged + [truncated; use --cursor X]
stderr hint when budget tripped.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 lexical-only tests covering search_response.v1 wrapper shape,
--max-tokens truncation, --cursor pagination, plain stderr hint.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
SearchInput gains max_tokens / snippet_chars / cursor (all optional).
Output wrapped in search_response.v1 to match CLI; existing
tools_call_search test updated to read v["hits"] instead of the bare
array.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ErrorV1 is the workspace wire error struct; boxing here would
force every call site to deref through a Box for no win — the
err-path is rare. Single allow at the function level.

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 11:31:39 +00:00
claude-reviewer-01 left a comment
Member

회차 1 — fb-34 의 surface 설계 (SearchOpts/SearchResponse 분리, App::search 시그니처 보존, kebab-core 도메인 isolation, base64 workspace dep promotion, HOTFIXES 기록, SKILL.md jq 패턴 갱신) 는 깔끔하고 spec 의도와 잘 맞는다. 그러나 단일 critical 이슈가 있다 — App::search_with_optscursor::decode 의 typed ErrorV1 { code: "stale_cursor" }anyhow!("stale_cursor: {}", e.message) 로 string 화해 버려, CLI/MCP 가 classify(&err, ...) 로 wire 변환할 때 어떤 downcast 분기에도 안 걸리고 code: "generic" 으로 떨어진다. spec / search_response.schema.json description / SKILL.md 가 모두 error.v1.code = stale_cursor 를 약속하는데 실제 wire 는 일치 안 함. 그리고 error_wire 단위 테스트가 "classify 가 stale_cursor 안 만든다" 는 false-positive invariant 를 lock 하고 있고, 어떤 테스트도 CLI/MCP exec → stderr ndjson 의 code 필드를 직접 검증하지 않아 contract 깨짐이 안 잡혔다. 추가로 snippet-only 단축 케이스에서 next_cursor 의미가 misleading 하고 (다음 페이지를 가리키지만 SKILL 안내는 "widen or paginate" 로 동등하게 제시), --no-cacheclear_search_cache 후 cached path 로 바뀌면서 long-lived caller 에서 race window 가 생긴 점이 후속 정리 대상이다.

회차 1 — fb-34 의 surface 설계 (SearchOpts/SearchResponse 분리, App::search 시그니처 보존, `kebab-core` 도메인 isolation, base64 workspace dep promotion, HOTFIXES 기록, SKILL.md jq 패턴 갱신) 는 깔끔하고 spec 의도와 잘 맞는다. 그러나 단일 critical 이슈가 있다 — `App::search_with_opts` 가 `cursor::decode` 의 typed `ErrorV1 { code: "stale_cursor" }` 를 `anyhow!("stale_cursor: {}", e.message)` 로 string 화해 버려, CLI/MCP 가 `classify(&err, ...)` 로 wire 변환할 때 어떤 downcast 분기에도 안 걸리고 `code: "generic"` 으로 떨어진다. spec / `search_response.schema.json` description / `SKILL.md` 가 모두 `error.v1.code = stale_cursor` 를 약속하는데 실제 wire 는 일치 안 함. 그리고 `error_wire` 단위 테스트가 "classify 가 stale_cursor 안 만든다" 는 false-positive invariant 를 lock 하고 있고, 어떤 테스트도 CLI/MCP exec → stderr ndjson 의 `code` 필드를 직접 검증하지 않아 contract 깨짐이 안 잡혔다. 추가로 snippet-only 단축 케이스에서 `next_cursor` 의미가 misleading 하고 (다음 페이지를 가리키지만 SKILL 안내는 "widen or paginate" 로 동등하게 제시), `--no-cache` 가 `clear_search_cache` 후 cached path 로 바뀌면서 long-lived caller 에서 race window 가 생긴 점이 후속 정리 대상이다.
@@ -277,0 +314,4 @@
let corpus_revision = self.sqlite.corpus_revision().to_string();
let offset = match opts.cursor.as_ref() {
Some(c) => cursor::decode(c, &corpus_revision)
.map_err(|e| anyhow!("stale_cursor: {}", e.message))?,

[critical] spec contract 위반: cursor::decode 가 만든 구조화된 ErrorV1 { code: "stale_cursor", ... } 가 여기서 anyhow!("stale_cursor: {}", e.message) 로 string 으로 변환되면서 typed payload 가 사라짐. 이후 CLI/MCP 가 classify(&err, ...) 로 wire 변환할 때 어떤 downcast_ref 도 매칭 안 돼서 code: "generic" 으로 떨어짐. 결과적으로 spec / search_response.schema.json description / SKILL.md 가 모두 error.v1.code = stale_cursor 약속하지만 실제 wire 는 code: generic. 권장: cursor::decodeErrorV1 를 별도 typed error (e.g. StaleCursorSignal) 로 wrap → anyhow::Error::new(...) 로 보존 → error_wire::classify 에 downcast 분기 추가. test 도 kebab search --cursor <stale> --json exec 후 stderr ndjson 의 code 필드를 직접 검증해야 함.

**[critical]** spec contract 위반: `cursor::decode` 가 만든 구조화된 `ErrorV1 { code: "stale_cursor", ... }` 가 여기서 `anyhow!("stale_cursor: {}", e.message)` 로 string 으로 변환되면서 typed payload 가 사라짐. 이후 CLI/MCP 가 `classify(&err, ...)` 로 wire 변환할 때 어떤 `downcast_ref` 도 매칭 안 돼서 `code: "generic"` 으로 떨어짐. 결과적으로 spec / `search_response.schema.json` description / `SKILL.md` 가 모두 `error.v1.code = stale_cursor` 약속하지만 실제 wire 는 `code: generic`. 권장: `cursor::decode` 의 `ErrorV1` 를 별도 typed error (e.g. `StaleCursorSignal`) 로 wrap → `anyhow::Error::new(...)` 로 보존 → `error_wire::classify` 에 downcast 분기 추가. test 도 `kebab search --cursor <stale> --json` exec 후 stderr ndjson 의 `code` 필드를 직접 검증해야 함.
@@ -277,0 +360,4 @@
// Budget loop.
let mut truncated = false;
if let Some(max_tokens) = opts.max_tokens {

[nit] max_tokens=0 엣지케이스 미커버. 0_usize.saturating_mul(4) = 0 → step 1 이 snippet 을 60-char floor 까지 줄이고, step 2 가 hits 를 1개까지 pop, 결국 1 hit + 60-char snippet + truncated=true 반환. 동작 자체는 합리적 (≥1 hit floor 보장) 하지만 spec 에는 정의 안 됨. 테스트 한 줄 추가 (max_tokens: Some(0) → 정확히 1 hit + truncated=true) 하거나 spec 에 "max_tokens 0 = effectively 1-hit floor" 명시 권장.

**[nit]** `max_tokens=0` 엣지케이스 미커버. `0_usize.saturating_mul(4) = 0` → step 1 이 snippet 을 60-char floor 까지 줄이고, step 2 가 hits 를 1개까지 pop, 결국 1 hit + 60-char snippet + `truncated=true` 반환. 동작 자체는 합리적 (≥1 hit floor 보장) 하지만 spec 에는 정의 안 됨. 테스트 한 줄 추가 (`max_tokens: Some(0)` → 정확히 1 hit + truncated=true) 하거나 spec 에 "max_tokens 0 = effectively 1-hit floor" 명시 권장.
@@ -277,0 +402,4 @@
Some(cursor::encode(offset + returned, &corpus_revision))
} else {
None
};

[significant] snippet-only truncation case 에서 misleading next_cursor. budget 이 hit 을 pop 안 하고 snippet 만 줄였을 때 truncated=true 이지만 returned == k_effective 이라 full_page 분기로 들어가 next_cursor = offset + k_effective 로 다음 페이지 를 가리킴. 그런데 SKILL.md 에 "either widen max_tokens or paginate via next_cursor" 라고 안내 — agent 가 next_cursor 를 따라가면 같은 hit 들의 full-snippet 이 아니라 다음 hit 들을 받음. 두 옵션의 의미가 다름. 명확히 하려면: (1) snippet-only-shrunk 케이스에서는 next_cursor 를 emit 하지 않거나, (2) truncated_kind: "snippet" | "k_pop" 같은 enum 추가, (3) 최소한 SKILL.md 와 schema description 에 "snippet 단축 시에는 widen 만 의미 있음" 명시.

**[significant]** snippet-only truncation case 에서 misleading `next_cursor`. budget 이 hit 을 pop 안 하고 snippet 만 줄였을 때 `truncated=true` 이지만 `returned == k_effective` 이라 `full_page` 분기로 들어가 `next_cursor = offset + k_effective` 로 다음 *페이지* 를 가리킴. 그런데 SKILL.md 에 "either widen max_tokens or paginate via next_cursor" 라고 안내 — agent 가 `next_cursor` 를 따라가면 같은 hit 들의 full-snippet 이 아니라 다음 hit 들을 받음. 두 옵션의 의미가 다름. 명확히 하려면: (1) snippet-only-shrunk 케이스에서는 `next_cursor` 를 emit 하지 않거나, (2) `truncated_kind: "snippet" | "k_pop"` 같은 enum 추가, (3) 최소한 SKILL.md 와 schema description 에 "snippet 단축 시에는 widen 만 의미 있음" 명시.
@@ -630,0 +791,4 @@
hits.iter()
.map(|h| serde_json::to_string(h).map(|s| s.len()).unwrap_or(0))
.sum()
}

[nit] doc-comment 부정확. "so a single broken hit never makes the loop loop forever" 가 의도 — 하지만 serde_json::to_string 실패 시 0 을 반환하면 오히려 loop 가 조기 종료 됨 (estimate_chars(&hits) > max_chars 가 false 가 되니까). 무한 루프 방지가 아니라 graceful degradation. 코멘트 한 줄 정정하거나 panic-on-error 가 더 명확할 수도 (SearchHit 직렬화 실패는 본질적으로 invariant 위반).

**[nit]** doc-comment 부정확. "so a single broken hit never makes the loop loop forever" 가 의도 — 하지만 `serde_json::to_string` 실패 시 0 을 반환하면 오히려 loop 가 *조기 종료* 됨 (`estimate_chars(&hits) > max_chars` 가 false 가 되니까). 무한 루프 방지가 아니라 graceful degradation. 코멘트 한 줄 정정하거나 panic-on-error 가 더 명확할 수도 (SearchHit 직렬화 실패는 본질적으로 invariant 위반).
@@ -0,0 +39,4 @@
pub fn decode(s: &str, expected_revision: &str) -> Result<usize, ErrorV1> {
let bytes = URL_SAFE_NO_PAD
.decode(s.as_bytes())
.map_err(|_| stale("<malformed>", expected_revision))?;

[nit] base64 decode 실패 시 stale("<malformed>", expected_revision) 호출 → 메시지가 "cursor was issued against corpus_revision '<malformed>'; current revision is 'rev-xyz'" 형태가 되는데, 실제로는 base64 자체가 깨졌으므로 "issued against" 표현이 부정확. 원인 구분된 메시지가 agent debugging 에 더 도움. 예: malformed 케이스는 "cursor is not valid base64", JSON 파싱 실패는 "cursor payload is malformed", revision mismatch 만 현재 메시지. 모두 code: "stale_cursor" 유지 (caller 의 retry 동작은 동일).

**[nit]** base64 decode 실패 시 `stale("<malformed>", expected_revision)` 호출 → 메시지가 `"cursor was issued against corpus_revision '<malformed>'; current revision is 'rev-xyz'"` 형태가 되는데, 실제로는 base64 자체가 깨졌으므로 "issued against" 표현이 부정확. 원인 구분된 메시지가 agent debugging 에 더 도움. 예: malformed 케이스는 `"cursor is not valid base64"`, JSON 파싱 실패는 `"cursor payload is malformed"`, revision mismatch 만 현재 메시지. 모두 `code: "stale_cursor"` 유지 (caller 의 retry 동작은 동일).
@@ -200,0 +211,4 @@
// (single source of truth). classify routes anyhow strings to the
// generic "unknown" code. This test pins that contract — adding a
// classify branch for stale_cursor would create two sources.
assert_ne!(v1.code, "stale_cursor", "classify must not produce stale_cursor — cursor::decode is sole source");

[critical, related] 이 테스트가 잘못된 invariant 를 lock 하고 있음. "classify 가 stale_cursor 안 만든다" 를 단언 = cursor::decode 가 single source — 인데 App::search_with_optsanyhow! 로 string 화하면서 single source 자체가 wire 까지 도달 못 함. 진짜로 검증해야 할 invariant 는 "kebab search --cursor <stale> --json 의 stderr 에 error.v1.code == "stale_cursor" 가 나타난다" 이고 그게 통과하려면 위의 app.rs:317 수정이 선행돼야 함. 현 상태는 false-positive — 테스트는 통과하지만 실제 contract 는 깨짐.

**[critical, related]** 이 테스트가 잘못된 invariant 를 lock 하고 있음. "classify 가 stale_cursor 안 만든다" 를 단언 = `cursor::decode` 가 single source — 인데 `App::search_with_opts` 가 `anyhow!` 로 string 화하면서 single source 자체가 wire 까지 도달 못 함. 진짜로 검증해야 할 invariant 는 "`kebab search --cursor <stale> --json` 의 stderr 에 `error.v1.code == "stale_cursor"` 가 나타난다" 이고 그게 통과하려면 위의 `app.rs:317` 수정이 선행돼야 함. 현 상태는 false-positive — 테스트는 통과하지만 실제 contract 는 깨짐.
@@ -0,0 +100,4 @@
.unwrap();
let cursor = page1.next_cursor;
if let Some(c) = cursor {

[nit] 테스트가 if let Some(c) = cursor { ... } else 통과 패턴이라, page1 에 next_cursor 가 없으면 stale-cursor 검증을 한 줄도 실행 안 하고 silent pass. 현재 fixture (1 doc + k=1) 에서는 returned == k_effective == 1 이라 full_page 분기로 next_cursor 가 emit 되지만, 추후 cursor 발급 조건이 바뀌면 이 테스트가 의도와 달리 noop 으로 변할 위험. let cursor = page1.next_cursor.expect("k=1 page must emit next_cursor") 로 강제하거나 fixture 를 2 docs + k=1 로 늘려서 명시적으로 보장 권장.

**[nit]** 테스트가 `if let Some(c) = cursor { ... } else 통과` 패턴이라, page1 에 next_cursor 가 없으면 stale-cursor 검증을 한 줄도 실행 안 하고 silent pass. 현재 fixture (1 doc + k=1) 에서는 `returned == k_effective == 1` 이라 `full_page` 분기로 next_cursor 가 emit 되지만, 추후 cursor 발급 조건이 바뀌면 이 테스트가 의도와 달리 noop 으로 변할 위험. `let cursor = page1.next_cursor.expect("k=1 page must emit next_cursor")` 로 강제하거나 fixture 를 2 docs + k=1 로 늘려서 명시적으로 보장 권장.
@@ -534,0 +554,4 @@
if *no_cache {
app.clear_search_cache();
}
let resp = app.search_with_opts(q, opts)?;

[nit] --no-cache semantic 약화. 이전 코드는 search_uncached_with_config 으로 cache 자체를 우회. 새 코드는 clear_search_cache()search_with_opts → self.search() 로 cache miss path 를 타는 방식 — CLI (fresh App) 에서는 무���하지만 long-lived (TUI/MCP) 에서는 clear 와 fetch 사이 다른 thread 의 search 가 cache 를 다시 채울 수 있어 race 가 가능. spec/HOTFIXES 에 --no-cache 의미 변경이 명시 안 돼 있음. App::search_uncached 를 활용하는 search_with_opts_uncached variant 를 추가하거나 (현 facade 는 항상 cached path) HOTFIXES 에 deviation 한 줄 기록 권장.

**[nit]** `--no-cache` semantic 약화. 이전 코드는 `search_uncached_with_config` 으로 cache 자체를 우회. 새 코드는 `clear_search_cache()` 후 `search_with_opts → self.search()` 로 cache miss path 를 타는 방식 — CLI (fresh App) 에서는 무���하지만 long-lived (TUI/MCP) 에서는 clear 와 fetch 사이 다른 thread 의 search 가 cache 를 다시 채울 수 있어 race 가 가능. spec/HOTFIXES 에 `--no-cache` 의미 변경이 명시 안 돼 있음. `App::search_uncached` 를 활용하는 `search_with_opts_uncached` variant 를 추가하거나 (현 facade 는 항상 cached path) HOTFIXES 에 deviation 한 줄 기록 권장.
@@ -0,0 +2,4 @@
"$schema": "https://json-schema.org/draft/2020-12/schema",
"$id": "https://kb.local/wire/v1/search_response.schema.json",
"title": "SearchResponse v1",
"description": "Top-level wrapper for `kebab search --json` output. Replaces the bare `search_hit.v1[]` array — wraps it with pagination + truncation metadata. Token counts are approximate (chars/4 estimate, no tokenizer dep). On `truncated: true`, caller may either widen `--max-tokens` or follow `next_cursor` for the next page. Stale `next_cursor` (corpus_revision changed since issued) returns `error.v1.code = stale_cursor`.",

[nit, blocker for ship] description 끝에 error.v1.code = stale_cursor 명시 — 위 app.rs:317 이슈 때문에 실제 wire 는 code: "generic" 으로 떨어짐. 둘 중 하나를 맞춰야 함: (a) app.rs 의 typed-error 보존을 고치고 schema 그대로 유지 (권장), (b) schema description 을 code: "generic" 또는 "the error message contains 'stale_cursor:'" 로 수정. (a) 가 spec contract 와 맞으므로 권장.

**[nit, blocker for ship]** description 끝에 `error.v1.code = stale_cursor` 명시 — 위 `app.rs:317` 이슈 때문에 실제 wire 는 `code: "generic"` 으로 떨어짐. 둘 중 하나를 맞춰야 함: (a) `app.rs` 의 typed-error 보존을 고치고 schema 그대로 유지 (권장), (b) schema description 을 `code: "generic"` 또는 "the error message contains 'stale_cursor:'" 로 수정. (a) 가 spec contract 와 맞으므로 권장.
@@ -104,2 +106,3 @@
- MCP tools return JSON content blocks; CLI prints **one JSON value to stdout**, progress / warnings to stderr. Capture stdout only: `kebab search ... --json 2>/dev/null`.
- `search` output can be large for broad queries. Project relevant fields when summarizing — for CLI: `jq '.[] | {rank, doc_path, heading: .heading_path[-1], snippet}'`.
- `search` output can be large for broad queries. Project relevant fields when summarizing — for CLI: `jq '.hits[] | {rank, doc_path, heading: .heading_path[-1], snippet}'` (note: `.hits[]`, not `.[]` — fb-34 wrapped the array). Use `--max-tokens N` (CLI) / `max_tokens` (MCP) to cap wire size in advance.
- Pagination: `search_response.v1.next_cursor` is opaque base64 — pass back as `--cursor` (CLI) or `cursor` (MCP) for the next page. `null` means no more hits. `corpus_revision` mismatch returns `error.v1.code = stale_cursor` — re-issue search to obtain a fresh cursor.

[nit, blocker for ship] 같은 이슈 — error.v1.code = stale_cursor 약속하지만 현재 구현은 code: generic. agent 가 SKILL 가이드 따라 code 로 분기 작성하면 fall-through 됨. app.rs:317 의 typed-error 보존이 들어간 후에야 이 SKILL.md 가 truthful 해짐.

**[nit, blocker for ship]** 같은 이슈 — `error.v1.code = stale_cursor` 약속하지만 현재 구현은 `code: generic`. agent 가 SKILL 가이드 따라 `code` 로 분기 작성하면 fall-through 됨. `app.rs:317` 의 typed-error 보존이 들어간 후에야 이 SKILL.md 가 truthful 해짐.
altair823 added 1 commit 2026-05-09 11:50:15 +00:00
- error_wire: StructuredError wrapper preserves ErrorV1 through
  anyhow → classify pipeline. Adds downcast short-circuit so
  cursor::decode's typed code = "stale_cursor" reaches the wire
  instead of being string-formatted to code = "generic".
- app: search_with_opts now wraps cursor::decode error in
  StructuredError instead of anyhow! string format.
- test: error_wire pins both negative (bare anyhow → not
  stale_cursor) AND positive (StructuredError → stale_cursor)
  invariants. CLI integration test runs end-to-end and asserts
  error.v1.code on stderr.
- app: next_cursor only emitted on full-page (k-pop) path; drop
  speculative emit on snippet-only truncation that would point at
  a different page than the agent expected.
- cursor: differentiate malformed-base64 / malformed-payload /
  revision-mismatch error messages; all keep code = stale_cursor.
- test: cursor_rejected fixture uses .expect() to fail loud on
  cursor non-emission instead of silent skip.
- test: max_tokens=0 → 1-hit floor + truncated=true.
- docs: SKILL.md + schema description distinguish snippet-shrink
  (widen) vs k-pop (paginate) truncated cases. HOTFIXES notes
  --no-cache semantic shift (cached path + clear vs uncached path).

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

회차 2 — round-1 fix 10건 중 9건은 깔끔하게 안착했다. StructuredError wrapper 의 Display/Error impl 과 classify downcast 우선 순위, cursor 의 3-way 메시지 분기, 두 invariant 를 모두 잠그는 error_wire 테스트 + end-to-end CLI 통합 테스트, expect() 기반 silent skip 제거, HOTFIXES --no-cache race-window 노트, schema/SKILL 문구 추가는 모두 합당하다. 다만 next_cursor emission 로직이 docs 와 정반대로 동작한다 — 새 조건 returned == k_effective 는 snippet-only shrinkage 케이스에서 cursor 를 emit 하고 k-pop 케이스에서는 emit 하지 않는데, SKILL.md / schema description / 코드 주석은 모두 그 반대로 약속하고 있다 (next_cursor != null → k reduced, next_cursor null → widen). agent 가 문서를 따르면 두 케이스 모두 잘못된 방향으로 fallback 하게 된다. 코드 또는 문서 한쪽을 sync 시키는 결정이 필요하므로 REQUEST_CHANGES.

회차 2 — round-1 fix 10건 중 9건은 깔끔하게 안착했다. `StructuredError` wrapper 의 `Display`/`Error` impl 과 `classify` downcast 우선 순위, cursor 의 3-way 메시지 분기, 두 invariant 를 모두 잠그는 error_wire 테스트 + end-to-end CLI 통합 테스트, `expect()` 기반 silent skip 제거, HOTFIXES `--no-cache` race-window 노트, schema/SKILL 문구 추가는 모두 합당하다. 다만 `next_cursor` emission 로직이 docs 와 정반대로 동작한다 — 새 조건 `returned == k_effective` 는 snippet-only shrinkage 케이스에서 cursor 를 emit 하고 k-pop 케이스에서는 emit 하지 않는데, SKILL.md / schema description / 코드 주석은 모두 그 반대로 약속하고 있다 (`next_cursor != null → k reduced`, `next_cursor null → widen`). agent 가 문서를 따르면 두 케이스 모두 잘못된 방향으로 fallback 하게 된다. 코드 또는 문서 한쪽을 sync 시키는 결정이 필요하므로 REQUEST_CHANGES.
@@ -53,0 +55,4 @@
///
/// `next_cursor` is `Some(_)` when the retriever returned a full
/// `k_effective` page (more hits may exist) or when the budget loop
/// truncated mid-page; the caller threads it back through

회차 2 — nit (위 main 이슈의 후속). SearchResponse doc 도 "or when the budget loop truncated mid-page" 라고 적혀 있는데, 새 로직은 mid-page truncation 중 k-pop 케이스에서는 cursor 를 emit 하지 않고 snippet-only 케이스에서만 emit 함. 코드 동작과 일치하도록 수정 필요. 라인 303-304 의 search_with_opts doc 도 동일한 문구.

회차 2 — nit (위 main 이슈의 후속). `SearchResponse` doc 도 "or when the budget loop truncated mid-page" 라고 적혀 있는데, 새 로직은 mid-page truncation 중 k-pop 케이스에서는 cursor 를 emit 하지 않고 snippet-only 케이스에서만 emit 함. 코드 동작과 일치하도록 수정 필요. 라인 303-304 의 search_with_opts doc 도 동일한 문구.
@@ -277,0 +407,4 @@
// shrinkage with `truncated: true` and `next_cursor: null`, the
// documented guidance is "widen max_tokens".
let returned = hits.len();
let next_cursor = if returned == k_effective

회차 2 — REQUEST_CHANGES — code/doc 의미가 정반대로 갈렸다. 새 조건은 returned == k_effective 일 때만 cursor 를 emit. 그러나 budget 루프 step-2 (hits.pop()) 가 실행되면 hits.len() < k_effective 가 되므로 k-pop 케이스에서는 cursor 가 null 이고, snippet-only shrinkage 케이스에서는 hits.len() == k_effective 이라 cursor 가 non-null 이다. 즉 실제 동작:

  • snippet-only shrunk → truncated=true, next_cursor=Some(...)
  • k-popped → truncated=true, next_cursor=None

반면 SKILL.md / schema / 바로 위 주석은 정반대로 약속함 (next_cursor != null → k reduced → paginate, next_cursor null → snippet shrunk → widen). 위 주석의 both produce returned == k_effectivepop() 이후에는 성립하지 않음. agent 가 docs 를 따르면 snippet-shrunk 페이지에서는 widen 해야 하는데 paginate 하고, k-popped 페이지에서는 paginate 해야 하는데 widen 하게 됨 — 둘 다 잘못된 방향.

선택지:

  1. docs 를 코드 동작에 맞춰 뒤집기: "snippet-shrunk → cursor 있음 (다음 페이지에서 fuller snippet 받기 위해 widen + paginate 가능)", "k-popped → cursor 없음 (re-issue with bigger budget)". 하지만 이러면 k-popped 시 진행 불가 — 사용자가 widen 했는데도 같은 query 만 가능하고, popped 된 hits 를 fetch 하려면 cursor 가 필요.
  2. 코드를 docs 에 맞추기: k-pop 분기일 때 offset + (popped 이전의 returned) 로 cursor emit, snippet-only shrunk 일 때 cursor 생략. 즉 truncated_by_pop 플래그를 step-2 안에서 set 하고 그걸로 분기.

fb-34 spec 의도가 next_cursor=null → widen 이라면 옵션 2 가 맞음. 결정과 함께 주석/docs/code 셋 다 sync 시켜야 함.

회차 2 — REQUEST_CHANGES — code/doc 의미가 정반대로 갈렸다. 새 조건은 `returned == k_effective` 일 때만 cursor 를 emit. 그러나 budget 루프 step-2 (`hits.pop()`) 가 실행되면 `hits.len() < k_effective` 가 되므로 **k-pop 케이스에서는 cursor 가 null** 이고, snippet-only shrinkage 케이스에서는 `hits.len() == k_effective` 이라 **cursor 가 non-null** 이다. 즉 실제 동작: - snippet-only shrunk → `truncated=true, next_cursor=Some(...)` - k-popped → `truncated=true, next_cursor=None` 반면 SKILL.md / schema / 바로 위 주석은 정반대로 약속함 (`next_cursor != null → k reduced → paginate`, `next_cursor null → snippet shrunk → widen`). 위 주석의 `both produce returned == k_effective` 는 `pop()` 이후에는 성립하지 않음. agent 가 docs 를 따르면 snippet-shrunk 페이지에서는 widen 해야 하는데 paginate 하고, k-popped 페이지에서는 paginate 해야 하는데 widen 하게 됨 — 둘 다 잘못된 방향. 선택지: 1. **docs 를 코드 동작에 맞춰 뒤집기**: "snippet-shrunk → cursor 있음 (다음 페이지에서 fuller snippet 받기 위해 widen + paginate 가능)", "k-popped → cursor 없음 (re-issue with bigger budget)". 하지만 이러면 k-popped 시 진행 불가 — 사용자가 widen 했는데도 같은 query 만 가능하고, popped 된 hits 를 fetch 하려면 cursor 가 필요. 2. **코드를 docs 에 맞추기**: k-pop 분기일 때 `offset + (popped 이전의 returned)` 로 cursor emit, snippet-only shrunk 일 때 cursor 생략. 즉 `truncated_by_pop` 플래그를 step-2 안에서 set 하고 그걸로 분기. fb-34 spec 의도가 `next_cursor=null → widen` 이라면 옵션 2 가 맞음. 결정과 함께 주석/docs/code 셋 다 sync 시켜야 함.
@@ -0,0 +151,4 @@
)
.unwrap();
assert_eq!(resp.hits.len(), 1, "max_tokens=0 collapses to 1-hit floor");
assert!(resp.truncated);

회차 2 — nit (선택). max_tokens=0 테스트가 hits.len()==1, truncated==true 까지만 assert 함. 위 issue 와 연결되는데, 이 케이스는 정확히 k-pop 케이스 (5→1) 이므로 새 로직에 따라 next_cursor==None 이어야 함. assertion 하나만 추가하면 cursor-emission 의미 표가 테스트로 pin 됨 — 향후 회귀 catch 에 도움. (만약 main 이슈가 옵션 2 로 해결되면 None 이 아니라 Some 이 되겠지만, 어느 쪽이든 명시 assertion 이 contract 를 잠금.)

회차 2 — nit (선택). `max_tokens=0` 테스트가 `hits.len()==1`, `truncated==true` 까지만 assert 함. 위 issue 와 연결되는데, 이 케이스는 정확히 k-pop 케이스 (5→1) 이므로 새 로직에 따라 `next_cursor==None` 이어야 함. assertion 하나만 추가하면 cursor-emission 의미 표가 테스트로 pin 됨 — 향후 회귀 catch 에 도움. (만약 main 이슈가 옵션 2 로 해결되면 None 이 아니라 Some 이 되겠지만, 어느 쪽이든 명시 assertion 이 contract 를 잠금.)
altair823 added 1 commit 2026-05-09 12:07:47 +00:00
Previous round-1 fix dropped the speculative cursor branch on
the truncated path, leaving a contradiction with the docs:
- snippet-only shrunk → cursor emitted (returned == k_effective)
- k-popped → cursor null (returned < k_effective)
But docs promised the opposite.

R2 resolution: emit cursor whenever more hits may be reachable
(either retriever filled the page OR budget popped hits — the
popped ones remain fetchable from offset+returned). Drop the
artificial "widen vs paginate" copy; truncated and next_cursor
are now independent signals — caller may do either or both.

Updates: app.rs::search_with_opts logic + SearchResponse doc +
schema description + SKILL.md two bullets + max_tokens=0 test
asserts cursor IS emitted on k-pop case.

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

회차 3 — R2 에서 잡힌 next_cursor 의미 반전 (snippet-shrunk 에는 cursor 가 나가고 k-popped 에는 안 나가던) 이 e084b30 에서 깔끔히 수정됐다 — 술어가 returned == k_effective || truncated 로 바뀌어 두 경로 모두 cursor 를 emit, 동시에 SearchResponse / schema / SKILL.md 두 bullet 이 일관되게 "truncated 와 next_cursor 는 독립 신호" 프레이밍으로 정렬됐다. max_tokens=0 테스트가 k-pop 경로에서 next_cursor.is_some() 을 직접 pin 하므로 같은 inversion 이 재발하면 즉시 빨강. R1 10건 + R2 3건 모두 수렴, 새로 발견된 이슈 없음 — APPROVE.

회차 3 — R2 에서 잡힌 next_cursor 의미 반전 (snippet-shrunk 에는 cursor 가 나가고 k-popped 에는 안 나가던) 이 `e084b30` 에서 깔끔히 수정됐다 — 술어가 `returned == k_effective || truncated` 로 바뀌어 두 경로 모두 cursor 를 emit, 동시에 SearchResponse / schema / SKILL.md 두 bullet 이 일관되게 "truncated 와 next_cursor 는 독립 신호" 프레이밍으로 정렬됐다. `max_tokens=0` 테스트가 k-pop 경로에서 `next_cursor.is_some()` 을 직접 pin 하므로 같은 inversion 이 재발하면 즉시 빨강. R1 10건 + R2 3건 모두 수렴, 새로 발견된 이슈 없음 — APPROVE.
@@ -53,0 +62,4 @@
///
/// `truncated` is independent of `next_cursor`: it signals that
/// the budget loop modified the page (snippet shorten or k pop).
/// Caller may either widen `max_tokens` (and re-issue the same

Doc rewrite is sharp — "non-null whenever more hits may be reachable" is the inverse of what R2 caught, and the explicit "truncated is independent of next_cursor" line means a future reader can't slide back into the widen-vs-paginate dichotomy. Schema description + SKILL.md two bullets all carry the same framing — no surface left contradicting another.

Doc rewrite is sharp — "non-null whenever more hits may be reachable" is the inverse of what R2 caught, and the explicit "truncated is independent of next_cursor" line means a future reader can't slide back into the widen-vs-paginate dichotomy. Schema description + SKILL.md two bullets all carry the same framing — no surface left contradicting another.
@@ -277,0 +407,4 @@
// returned fewer than k_effective AND we didn't pop. That means
// end of available results.
let returned = hits.len();
let next_cursor = if returned == k_effective || truncated {

Predicate inversion fixed cleanly: returned == k_effective || truncated covers both the speculative-full-page path and the k-pop path. The three-case comment block (a/b/c) walks through the invariants exactly the way an agent author would want to read them — and pinning case (c) as falling under (a) closes the snippet-only-shrink corner. Nice.

Predicate inversion fixed cleanly: `returned == k_effective || truncated` covers both the speculative-full-page path and the k-pop path. The three-case comment block (a/b/c) walks through the invariants exactly the way an agent author would want to read them — and pinning case (c) as falling under (a) closes the snippet-only-shrink corner. Nice.
@@ -0,0 +155,4 @@
// p9-fb-34 R2: cursor IS emitted on k-pop case so the popped
// hits remain reachable.
assert!(
resp.next_cursor.is_some(),

Right test to pin the R2 fix: max_tokens=0 is the canonical k-pop scenario (3 fixture docs → 1 hit floor), and the new next_cursor.is_some() assert directly guards against the inversion regressing. The error message even explains why ("popped hits at offset+returned") so a future failure points the reader straight at the contract.

Right test to pin the R2 fix: `max_tokens=0` is the canonical k-pop scenario (3 fixture docs → 1 hit floor), and the new `next_cursor.is_some()` assert directly guards against the inversion regressing. The error message even explains *why* ("popped hits at offset+returned") so a future failure points the reader straight at the contract.
altair823 merged commit 9817a3de59 into main 2026-05-09 12:52:40 +00:00
altair823 deleted branch feat/fb-34-output-budget-controls 2026-05-09 12:52:41 +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#125