doctor integrity check reports "could not measure" as FAIL, and a real error as OK #209

Closed
opened 2026-09-07 16:54:15 +02:00 by buildagent · 1 comment
Member

Found by dogfooding v0.27.0-rc (caa62fe) immediately after reinstalling the MCP server.

The repro, in two commands with no change to the database

While the daemon was mid-reconcile:

[ FAIL ] integrity   PRAGMA integrity_check: unable to validate the inverted index
                     for FTS5 table main.files_fts: database is locked

Same command, ~2 minutes later, once the resolve had committed:

[  OK  ] integrity   PRAGMA integrity_check ok

Nothing about the database changed. The only difference was a write lock.

So code-index doctor tells an operator their index is corrupt precisely when they are most likely to run it — while indexing is in flight, which is exactly when something feels wrong. integrity is the check whose FAIL causes someone to delete their index and rebuild.

Both directions are wrong

crates/cli/src/doctor.rs:548:

let mut rows = stmt.query([]).unwrap();
while let Ok(Some(row)) = rows.next() {
    let s: String = row.get(0).unwrap_or_default();
    if s != "ok" {
        return Check { severity: Severity::Fail, detail: format!("PRAGMA integrity_check: {s}") };
    }
}
Check { severity: Severity::Ok, detail: "PRAGMA integrity_check ok".into() }
  1. Unmeasurable renders as measured-and-bad. Any row that is not the literal "ok" becomes Fail. SQLite's FTS5 arm emits unable to validate the inverted index …: database is locked, which is a statement that the check DID NOT RUN. There is no third state.

  2. A real error renders as an all-clear. while let Ok(Some(row)) treats Err as end-of-iteration and falls through to Severity::Ok, "PRAGMA integrity_check ok". An SQLITE_CORRUPT surfacing mid-iteration — the case this check exists for — reports OK.

  3. stmt.query([]).unwrap() panics rather than reporting anything.

This is the tree's own three-state rule inverted on both sides: absent/unmeasured is not a measurement, and it is not the same as either verdict.

Why it survived

doctor.rs::check_integrity is ungraded. The only integrity string in crates/cli/tests/ is a doc comment in plugin_gc_doctor_cli.rs:361. The test that does exist, corrupt_db_is_detected_by_integrity_check, grades a DIFFERENT function — crates/indexer/src/db.rs::check_integrity — with the same name in another crate. The operator-facing one has never been driven by a test.

What the fix has to distinguish

Three states, not two:

  • verified ok — every row is "ok".
  • verified bad — a row reports actual corruption. Stays Fail.
  • not verified — the check could not run (lock contention is the known case, and database is locked is the marker). Must NOT be Fail, must NOT be Ok, and must say that it did not run and that retrying when the daemon is idle is the remedy. Severity::Warn with wording that names the reason, or a dedicated state.

An Err during row iteration is also the "not verified" state, not the OK it currently produces.

Mutations the fix must run

  • Make the ok-path return the not-verified state → the ok assertion fails.
  • Make the locked-path return Fail → the not-verified assertion fails.
  • Make the iteration Err arm fall through to Ok (i.e. restore today's bug) → the error assertion fails.

The locked case is drivable in a test: hold a write transaction on the DB from a second connection and run the check.

Related: the same "did not measure vs measured" rule is what index_coverage's pending/never split, search_symbols's empty_population, and project_overview's availability fields all implement correctly. doctor is the surface that did not get it.

Found by dogfooding v0.27.0-rc (`caa62fe`) immediately after reinstalling the MCP server. ## The repro, in two commands with no change to the database While the daemon was mid-reconcile: ``` [ FAIL ] integrity PRAGMA integrity_check: unable to validate the inverted index for FTS5 table main.files_fts: database is locked ``` Same command, ~2 minutes later, once the resolve had committed: ``` [ OK ] integrity PRAGMA integrity_check ok ``` Nothing about the database changed. The only difference was a write lock. **So `code-index doctor` tells an operator their index is corrupt precisely when they are most likely to run it** — while indexing is in flight, which is exactly when something feels wrong. `integrity` is the check whose FAIL causes someone to delete their index and rebuild. ## Both directions are wrong `crates/cli/src/doctor.rs:548`: ```rust let mut rows = stmt.query([]).unwrap(); while let Ok(Some(row)) = rows.next() { let s: String = row.get(0).unwrap_or_default(); if s != "ok" { return Check { severity: Severity::Fail, detail: format!("PRAGMA integrity_check: {s}") }; } } Check { severity: Severity::Ok, detail: "PRAGMA integrity_check ok".into() } ``` 1. **Unmeasurable renders as measured-and-bad.** Any row that is not the literal `"ok"` becomes `Fail`. SQLite's FTS5 arm emits `unable to validate the inverted index …: database is locked`, which is a statement that the check DID NOT RUN. There is no third state. 2. **A real error renders as an all-clear.** `while let Ok(Some(row))` treats `Err` as end-of-iteration and falls through to `Severity::Ok, "PRAGMA integrity_check ok"`. An `SQLITE_CORRUPT` surfacing mid-iteration — the case this check exists for — reports OK. 3. `stmt.query([]).unwrap()` panics rather than reporting anything. This is the tree's own three-state rule inverted on both sides: absent/unmeasured is not a measurement, and it is not the same as either verdict. ## Why it survived **`doctor.rs::check_integrity` is ungraded.** The only `integrity` string in `crates/cli/tests/` is a doc comment in `plugin_gc_doctor_cli.rs:361`. The test that does exist, `corrupt_db_is_detected_by_integrity_check`, grades a DIFFERENT function — `crates/indexer/src/db.rs::check_integrity` — with the same name in another crate. The operator-facing one has never been driven by a test. ## What the fix has to distinguish Three states, not two: * **verified ok** — every row is `"ok"`. * **verified bad** — a row reports actual corruption. Stays `Fail`. * **not verified** — the check could not run (lock contention is the known case, and `database is locked` is the marker). Must NOT be `Fail`, must NOT be `Ok`, and must say that it did not run and that retrying when the daemon is idle is the remedy. `Severity::Warn` with wording that names the reason, or a dedicated state. An `Err` during row iteration is *also* the "not verified" state, not the OK it currently produces. ## Mutations the fix must run * Make the ok-path return the not-verified state → the ok assertion fails. * Make the locked-path return `Fail` → the not-verified assertion fails. * Make the iteration `Err` arm fall through to `Ok` (i.e. restore today's bug) → the error assertion fails. The locked case is drivable in a test: hold a write transaction on the DB from a second connection and run the check. Related: the same "did not measure vs measured" rule is what `index_coverage`'s `pending`/`never` split, `search_symbols`'s `empty_population`, and `project_overview`'s `availability` fields all implement correctly. `doctor` is the surface that did not get it.
Author
Member

Fixed in c566f87 — "doctor stops rendering 'could not measure' as a verdict" — on master, shipping in v0.27.0.

Both directions were wrong and both are fixed: PRAGMA integrity_check returning "unable to validate … : database is locked" said the check did not run and was reported as corruption — hardest exactly when an operator was most likely to run it, mid-reconcile — while an error raised mid-iteration reported an all-clear. The three states (did not run / ran clean / ran and found something) are now distinct in both the check and its rendering.

Closing on merge; the release itself is held on an unrelated Windows archive question (#231).

Fixed in `c566f87` — *"`doctor` stops rendering 'could not measure' as a verdict"* — on `master`, shipping in v0.27.0. Both directions were wrong and both are fixed: `PRAGMA integrity_check` returning *"unable to validate … : database is locked"* said the check did not run and was reported as corruption — hardest exactly when an operator was most likely to run it, mid-reconcile — while an error raised mid-iteration reported an all-clear. The three states (did not run / ran clean / ran and found something) are now distinct in both the check and its rendering. Closing on merge; the release itself is held on an unrelated Windows archive question (#231).
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
h-dv/code-index#209
No description provided.