From 9e09b7361cfc54cc7c854c0dccae847c75bfc4b8 Mon Sep 17 00:00:00 2001 From: rohit Date: Tue, 3 Nov 2026 09:30:07 +0000 Subject: [PATCH] fix(compare): fall back to a line diff when semantic rows fail to build When the semantic diff succeeded but its rows did not build a valid document, diff_one returned Error::Build. diff_files collects every file into one Result, so a single bad file failed the whole request, and the Node binding rejected every file with it. That file now gets a line diff like any other semantic failure, with a warning logged and fallback kind semantic, reason invalid. The new reason is added to FORMAT.md and the TypeScript types; adding an enum value is not a breaking change under the format's versioning rules. --- bindings/node/index.d.ts | 2 +- crates/diffy-compare/FORMAT.md | 4 ++-- crates/diffy-compare/src/file.rs | 20 ++++++++++++++++---- crates/diffy-compare/src/lib.rs | 3 +++ crates/diffy-compare/src/wire.rs | 2 ++ crates/diffy-compare/src/wire/tests.rs | 8 ++++++++ 6 files changed, 32 insertions(+), 7 deletions(-) diff --git a/bindings/node/index.d.ts b/bindings/node/index.d.ts index ecdc33a1..419dd755 100644 --- a/bindings/node/index.d.ts +++ b/bindings/node/index.d.ts @@ -134,7 +134,7 @@ export interface Fallback { */ kind: 'no_grammar' | 'grammar_unavailable' | 'semantic' | 'request_budget' /** Present when `kind` is `semantic`. */ - reason?: 'too_large' | 'parse_errors' | 'error_coverage' | 'unparsed' | 'budget' | 'timeout' + reason?: 'too_large' | 'parse_errors' | 'error_coverage' | 'unparsed' | 'budget' | 'timeout' | 'invalid' /** For people; wording may change. Branch on `kind` and `reason`. */ message: string } diff --git a/crates/diffy-compare/FORMAT.md b/crates/diffy-compare/FORMAT.md index 53b3f108..b2347277 100644 --- a/crates/diffy-compare/FORMAT.md +++ b/crates/diffy-compare/FORMAT.md @@ -58,7 +58,7 @@ Render order: for each hunk, `gap_before`, header, rows; then `gap_after`. | Field | Type | | | --- | --- | --- | | `kind` | `"no_grammar"` \| `"grammar_unavailable"` \| `"semantic"` \| `"request_budget"` | `no_grammar`: no language for the path. `grammar_unavailable`: the language's grammar failed to load. `semantic`: the semantic diff gave up, see `reason`. `request_budget`: the request's deadline passed before this file was diffed. | -| `reason` | `"too_large"` \| `"parse_errors"` \| `"error_coverage"` \| `"unparsed"` \| `"budget"` \| `"timeout"`? | Present when `kind` is `semantic`, in order: file too large; too many parse errors; parse errors cover too much; parser gave up; work budget ran out; time budget ran out. | +| `reason` | `"too_large"` \| `"parse_errors"` \| `"error_coverage"` \| `"unparsed"` \| `"budget"` \| `"timeout"` \| `"invalid"`? | Present when `kind` is `semantic`, in order: file too large; too many parse errors; parse errors cover too much; parser gave up; work budget ran out; time budget ran out; its result was unusable (a bug). | | `message` | string | For people, e.g. `"exceeded the semantic time budget"`. Wording may change; branch on `kind` and `reason`. | ## Hunk @@ -213,7 +213,7 @@ type File = { type Fallback = { kind: "no_grammar" | "grammar_unavailable" | "semantic" | "request_budget"; - reason?: "too_large" | "parse_errors" | "error_coverage" | "unparsed" | "budget" | "timeout"; + reason?: "too_large" | "parse_errors" | "error_coverage" | "unparsed" | "budget" | "timeout" | "invalid"; message: string; }; diff --git a/crates/diffy-compare/src/file.rs b/crates/diffy-compare/src/file.rs index 533d8da8..37d9069a 100644 --- a/crates/diffy-compare/src/file.rs +++ b/crates/diffy-compare/src/file.rs @@ -132,10 +132,22 @@ pub(crate) fn diff_one( }) .collect(), }; - Ok(FileDiff { - document: document_from_rows(meta, old, new, chunks, true, semantics)?, - fallback: None, - }) + match document_from_rows(meta.clone(), old, new, chunks, true, semantics) { + Ok(document) => Ok(FileDiff { + document, + fallback: None, + }), + // A bug in the semantic rows; the file still gets a diff. + Err(error) => { + tracing::warn!( + target: "diffy::semantic", + path = input.display_path(), + %error, + "semantic rows did not build; using a line diff" + ); + line(meta, Fallback::Invalid(error.to_string())) + } + } } Err(fallback) => line(meta, Fallback::Semantic(fallback)), } diff --git a/crates/diffy-compare/src/lib.rs b/crates/diffy-compare/src/lib.rs index ac814eb7..d9f96bde 100644 --- a/crates/diffy-compare/src/lib.rs +++ b/crates/diffy-compare/src/lib.rs @@ -73,6 +73,8 @@ pub enum Fallback { GrammarUnavailable(String), /// The semantic diff gave up. Semantic(diffy_semantic::Fallback), + /// The semantic diff's rows did not build a document (a bug). + Invalid(String), /// The request's deadline passed before this file was diffed. RequestBudget, } @@ -83,6 +85,7 @@ impl std::fmt::Display for Fallback { Self::NoGrammar => f.write_str("no grammar for this file"), Self::GrammarUnavailable(tag) => write!(f, "{tag} grammar unavailable"), Self::Semantic(fallback) => fallback.fmt(f), + Self::Invalid(error) => write!(f, "semantic result unusable: {error}"), Self::RequestBudget => f.write_str("request time budget exhausted"), } } diff --git a/crates/diffy-compare/src/wire.rs b/crates/diffy-compare/src/wire.rs index 02d147ce..162b7da8 100644 --- a/crates/diffy-compare/src/wire.rs +++ b/crates/diffy-compare/src/wire.rs @@ -153,6 +153,7 @@ pub enum SemanticReason { Unparsed, Budget, Timeout, + Invalid, } impl From<&crate::Fallback> for Fallback { @@ -162,6 +163,7 @@ impl From<&crate::Fallback> for Fallback { crate::Fallback::NoGrammar => (FallbackKind::NoGrammar, None), crate::Fallback::GrammarUnavailable(_) => (FallbackKind::GrammarUnavailable, None), crate::Fallback::RequestBudget => (FallbackKind::RequestBudget, None), + crate::Fallback::Invalid(_) => (FallbackKind::Semantic, Some(SemanticReason::Invalid)), crate::Fallback::Semantic(semantic) => { let reason = match semantic { Semantic::TooLarge(_) => SemanticReason::TooLarge, diff --git a/crates/diffy-compare/src/wire/tests.rs b/crates/diffy-compare/src/wire/tests.rs index f9ae0d16..f744d89d 100644 --- a/crates/diffy-compare/src/wire/tests.rs +++ b/crates/diffy-compare/src/wire/tests.rs @@ -682,6 +682,14 @@ fn fallbacks_serialize_as_stable_kinds() { "message": "5 bytes exceeds the semantic byte limit", }), ), + ( + crate::Fallback::Invalid("bad rows".into()), + serde_json::json!({ + "kind": "semantic", + "reason": "invalid", + "message": "semantic result unusable: bad rows", + }), + ), ( crate::Fallback::GrammarUnavailable("zig".into()), serde_json::json!({