From d794c7a55f3b36593b56b777e6d0f1adca3cbee4 Mon Sep 17 00:00:00 2001 From: altair823 Date: Sun, 16 Aug 2026 22:02:02 +0900 Subject: [PATCH] =?UTF-8?q?chore:=20PR=20#236=20=ED=9A=8C=EC=B0=A8=203=20?= =?UTF-8?q?=EB=A6=AC=EB=B7=B0=20=EB=B0=98=EC=98=81=20=E2=80=94=20=EB=B9=88?= =?UTF-8?q?=20=EC=8A=A4=EC=BA=94=20=EC=B7=A8=EC=86=8C=20=EC=98=A4=EB=B3=B4?= =?UTF-8?q?=EA=B3=A0=20+=20=ED=85=8C=EC=8A=A4=ED=8A=B8=20=ED=83=90?= =?UTF-8?q?=EC=A7=80=EB=A0=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 3회차 리뷰가 취소 처리에 CRITICAL/HIGH 가 없음을 확인하고(특히 break 후에도 `flush_vector_deletes` 가 돌아 고아 벡터가 안 생기는 것, `examined` 가 네 경계에서 모두 맞는 것) 머지 가능으로 결론냈다. 남은 지적 다섯을 반영한다. 1) 빈 스캔 + sweep 취소가 Aborted 가 아니라 Completed 로 보고됐다 (MEDIUM) `was_cancelled` 는 asset 루프 **본문**에서만 세팅되는데, 스캔이 0건이면 본문이 한 번도 안 돈다. 하필 그게 sweep 이 가장 커지는 경우다 — 스캔이 0건이면 저장된 모든 경로가 후보이므로, "색인된 디렉토리를 통째로 지우고 재색인 → 긴 sweep → Ctrl-C" 가 정확히 이 구멍에 들어간다. 사용자는 취소했는데 "완료"를 본다. 플래그로 시드한다. `cancelling_a_sweep_with_nothing_left_to_scan_still_ reports_aborted` 로 고정했는데, 처음 쓴 버전은 워크스페이스 상위만 지워서 픽스처가 하위 디렉토리에 남았고 asset 루프가 한 번 돌아 **되돌려도 통과했다**. 재귀 삭제로 고치고, 전제(`ScanCompleted { total: 0 }`)를 어서션으로 박았다. 시드를 되돌리면 실패하는 것을 확인했다. 2) CLI 테스트의 position 어서션에 탐지력이 없었다 (LOW) 두 시나리오를 따로 돌려서 두 번째에 `SweepProgress` 가 없었고, position 은 `SweepStarted` 의 `set_position(0)` 이후 계속 0 이었다. 즉 `dress_bar_for_assets` 안의 `set_position(0)` 을 지워도 통과했다. 하나로 합쳐 sweep 이 position 을 3 까지 올린 뒤 복구를 본다. 3) 그 테스트 주석이 실제 범위보다 넓게 주장했다 (LOW) "하트비트 키까지 고정한다" 고 적었는데 실제로는 length/position 만 본다. indicatif 가 스타일을 되읽을 방법을 주지 않으므로, 그 절반은 `dress_bar_for_assets` 가 양쪽 phase 의 유일한 옷 입히는 자리라는 사실에 기댄다 — 주석을 그렇게 고쳐 적었다. 4) 취소 계약 독 주석이 stale 했다 (LOW) `ingest_with_config` 의 §10 계약에 "in-flight asset 이 끝나고 이후는 스킵" 만 있고 sweep 이 이제 취소를 본다는 사실이 없었다. 5) DOGFOOD §1.8 이 자기 모순이었다 (LOW) `sweep_completed.checked == total` 을 단정하고 바로 다음 줄에서 취소 시엔 다르다고 했다. 앞줄에 "취소 없이 완주하면" 을 붙였다. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_017c9JwQq8ZkGvYjpKXMiDhF --- crates/kebab-app/src/ingest.rs | 13 ++++- crates/kebab-app/tests/sweep_progress.rs | 65 +++++++++++++++++++++++ crates/kebab-cli/src/progress.rs | 66 +++++++++++------------- docs/DOGFOOD.md | 2 +- 4 files changed, 109 insertions(+), 37 deletions(-) diff --git a/crates/kebab-app/src/ingest.rs b/crates/kebab-app/src/ingest.rs index f7d72b4..e4211ed 100644 --- a/crates/kebab-app/src/ingest.rs +++ b/crates/kebab-app/src/ingest.rs @@ -84,6 +84,10 @@ pub fn ingest(scope: SourceScope, opts: IngestOpts) -> anyhow::Result(); + ingest_with_config( + env.config.clone(), + env.scope(), + IngestOpts { + progress: Some(tx), + cancel: Some(cancel), + ..Default::default() + }, + ) + .expect("a cancelled ingest still returns a report"); + let events: Vec = rx.into_iter().collect(); + + assert!( + matches!( + events + .iter() + .find(|e| matches!(e, IngestEvent::ScanCompleted { .. })), + Some(IngestEvent::ScanCompleted { total: 0 }) + ), + "the premise: this run must find nothing to index, or the asset \ + loop runs an iteration and records the cancel by itself: {events:?}" + ); + assert!( + matches!(events.last(), Some(IngestEvent::Aborted { .. })), + "a cancelled run must end in Aborted even when the asset loop \ + never ran a single iteration: {:?}", + events.last() + ); +} diff --git a/crates/kebab-cli/src/progress.rs b/crates/kebab-cli/src/progress.rs index aff03bb..ef07f6e 100644 --- a/crates/kebab-cli/src/progress.rs +++ b/crates/kebab-cli/src/progress.rs @@ -547,68 +547,64 @@ pub(crate) fn now_rfc3339() -> anyhow::Result { mod tests { use super::*; - fn feed(events: &[IngestEvent]) -> ProgressDisplay { - // quiet + non-tty: the bar exists with a hidden draw target, so - // its state is observable without a terminal and nothing is - // printed to the test's stderr. + /// The sweep phase (issue #228) borrows the asset bar and puts its + /// own, smaller total on it. If it does not hand the bar back, every + /// asset drawn afterwards carries the sweep's denominator — that + /// shipped once already, so it is pinned here. + /// + /// Scope: length and position only. The same bug also dropped the + /// `{asset_elapsed}` heartbeat key along with the style, and + /// indicatif exposes no way to read a bar's style back, so that half + /// rests on `dress_bar_for_assets` being the single place either + /// phase dresses the bar. + #[test] + fn sweep_hands_the_bar_back_to_the_asset_loop() { let mut d = ProgressDisplay::new(ProgressMode::Human { tty: false, quiet: true, }); - for e in events { - d.handle(e).expect("handle"); - } - d - } - - /// The sweep phase (issue #228) borrows the asset bar and puts its - /// own, smaller total on it. If it does not hand the bar back, every - /// asset drawn afterwards carries the sweep's label and denominator — - /// and the style swap also drops the `{asset_elapsed}` heartbeat that - /// is the only sign a slow asset is alive. That shipped once already; - /// this pins it so the next phase added here cannot repeat it. - #[test] - fn sweep_hands_the_bar_back_to_the_asset_loop() { - let d = feed(&[ + for e in [ IngestEvent::ScanStarted { root: "/ws".to_string(), }, IngestEvent::ScanCompleted { total: 17 }, IngestEvent::SweepStarted { total: 3 }, IngestEvent::SweepProgress { - idx: 1, + idx: 3, total: 3, path: "gone.md".to_string(), removed: true, }, - ]); + ] { + d.handle(&e).expect("handle"); + } assert_eq!( d.bar.as_ref().and_then(indicatif::ProgressBar::length), Some(3), "during the sweep the bar counts sweep candidates" ); + assert_eq!( + d.bar.as_ref().map(indicatif::ProgressBar::position), + Some(3), + "and tracks them" + ); - let d = feed(&[ - IngestEvent::ScanStarted { - root: "/ws".to_string(), - }, - IngestEvent::ScanCompleted { total: 17 }, - IngestEvent::SweepStarted { total: 3 }, - IngestEvent::SweepCompleted { - checked: 3, - purged: 3, - ms: 12, - }, - ]); + d.handle(&IngestEvent::SweepCompleted { + checked: 3, + purged: 3, + ms: 12, + }) + .expect("handle"); assert_eq!( d.bar.as_ref().and_then(indicatif::ProgressBar::length), Some(17), - "and once it ends the bar counts assets again" + "once the sweep ends the bar counts assets again" ); assert_eq!( d.bar.as_ref().map(indicatif::ProgressBar::position), Some(0), - "from the start, not from wherever the sweep left off" + "from the start — leaving the sweep's position would make the \ + asset loop look part-done before it began" ); } diff --git a/docs/DOGFOOD.md b/docs/DOGFOOD.md index 9e6fe5f..7771968 100644 --- a/docs/DOGFOOD.md +++ b/docs/DOGFOOD.md @@ -274,7 +274,7 @@ echo "# stdin content" | "$RELEASE_BIN" ingest-stdin --title "from stdin" --conf - ordering invariant (design §2.4a). - per-asset `idx/total/path/media/result/chunks`. - aggregate `counts` on `completed` / `aborted`. -- sweep 구간: `sweep_started.total` 이 후보 수와 맞고, `sweep_progress.idx` 가 1..=total 로 연속이며, `sweep_completed.checked == total`. +- sweep 구간: `sweep_started.total` 이 후보 수와 맞고, `sweep_progress.idx` 가 1..=total 로 연속이며, 취소 없이 완주하면 `sweep_completed.checked == total`. - sweep 이 끝난 뒤 진행바가 asset 분모·라벨로 돌아오는가 (TTY). sweep 이 같은 바를 빌려 쓰므로 복구가 빠지면 색인 구간 내내 `sweep [..] 4213/21` 로 그려진다. - ndjson 로그에 `purge` 줄과 `sweep_summary` 가 남는가 (이슈 #228 이전에는 이 구간이 0바이트였다). purge 실패 시 `purge_failed`. - sweep 중 Ctrl-C 한 번에 실제로 멈추는가. `sweep_completed.checked` 가 예고한 `total` 이 아니라 실제 검사한 수로 나와야 한다.