From e49d6c0ce551a7bb5c04aaca286cfae940165e4e Mon Sep 17 00:00:00 2001 From: Krish Garg Date: Thu, 15 Oct 2026 02:57:12 -0700 Subject: [PATCH 1/2] Show each slice's files in the desktop's reading order The Code tab listed files in the order the patch had them. The desktop's orderDiffFiles puts source first, each file after the changed files it imports, then the tests after the source they cover, then the files a tool wrote, each slice on its own. t3term now works the same order out once when a slice comes, finding imports in every line of a file's hunks as the desktop's diff library gives them, and comparing paths as the root collation does for ASCII. Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/pull-request-panel.md | 5 +- src/tui/pull_requests/code.rs | 31 +- src/tui/pull_requests/code/order.rs | 332 +++++++++++++++++++++ src/tui/pull_requests/code/order/tests.rs | 341 ++++++++++++++++++++++ src/tui/pull_requests/code/tests.rs | 37 +-- 5 files changed, 713 insertions(+), 33 deletions(-) create mode 100644 src/tui/pull_requests/code/order.rs create mode 100644 src/tui/pull_requests/code/order/tests.rs diff --git a/docs/pull-request-panel.md b/docs/pull-request-panel.md index 38de01e4..334f3e91 100644 --- a/docs/pull-request-panel.md +++ b/docs/pull-request-panel.md @@ -103,6 +103,7 @@ Where the detail's `capabilities.diff` is true, the details have a third tab, Co | Reading again | `codeRefreshToken` (PullRequestDetailPanel.tsx:832) moves with each refresh number, a person's Refresh and a detail whose `updatedAt` changed (836-847), and the tab reads the first slice again | The same three read the first slice again. After `r`, the read waits until the read that sent `pullRequests.invalidate` is done, so T3 forgets the diff it held. The files read before show until the new ones come, and a file keeps its fold. A read again that fails keeps them under `May be out of date: t3term couldn't read the diff again.` and the reason. | | Count | `12 files`, with `+` while more slices come (PullRequestCodeTab.tsx:1124-1125), and a note when the host withheld part of the diff (1183-1195) | `12 files+`, then `The host withheld part of this diff, such as a binary file or a change too large to show.` in yellow where a slice says the host left something out or has text the parser found no file in. | | Files | Folded at first, as the `diffFilesCollapsed` setting is true until changed (pullRequestDiff.logic.ts:36-43, PullRequestCodeTab.tsx:230-232). The header has a chevron, an icon for how the file changed, the path, both paths of a rename, and `+3 -1` unless both are 0, from the host's own counts where the hunks it sent count none (822-823) | `› M src/lib.rs +3 -1`, with A, D, R or M in green, red, blue or yellow for an added, deleted, renamed or changed file. The path wraps under itself. Enter or a click unfolds it, `⌄`, and folds it again. A path one slice holds twice, as git writes a file whose type changed, shows once as its last part, as the desktop's `orderDiffFiles` keeps one file for each path (pullRequestFileOrder.logic.ts:152-153). | +| Order | `orderDiffFiles` (pullRequestFileOrder.logic.ts:145-197) orders each slice on its own, so a later slice never moves the files above it (PullRequestCodeTab.tsx:404-411). Source comes first, each file after the changed files it imports. A quoted specifier after `from`, `import` or `require(` in any line of a file's hunks names a changed file by its path, with `./` and `../` resolved and a script's extension dropped, or by a name only one source file has. Where a cycle leaves no file ready, the lowest path goes next. Then the tests (`.test.`, `.spec.`, or a `test`, `tests` or `__tests__` directory), each after the source file whose name it has, then lockfiles, snapshots, minified files and files under `dist`, `build`, `vendor`, `__generated__` or `__snapshots__`, by path. Paths compare with `localeCompare` | The same order, worked out once when a slice comes. The diff library the desktop uses puts each unchanged line of a hunk on both sides, so an import in an unchanged line counts too (@pierre/diffs 1.5.2, dist/utils/parsePatchFiles.js:208-222), and t3term counts it. Paths compare as the root collation does for ASCII: punctuation, then digits, then letters with case ignored, then lowercase before uppercase. | | Lines | The stacked view: each line with its number, and `N unmodified lines` between hunks (apps/web/src/lib/diffRendering.ts:82) | Each line's number, the new one or the old for a removed line, then `+`, `-` or a space, in green, red or the text's color. A line that wraps leaves the number and the mark blank. Tabs stop every 4 columns, control characters go, and a line past 2,000 characters is cut with `…`. `⋯ 9 unmodified lines` comes before each hunk that doesn't start at the file's first line. A binary file, a file whose changes the host withheld and a rename that changed no line each say so. So does a file past t3term's 200,000 lines of a slice, or with lines t3term couldn't read, such as a hunk shorter or longer than its header counts. | | Waiting and failing | `Loading pull request diff...`, `Loading more files...` after the files, the first slice's error alone, and `The rest of this diff could not be loaded.` with Retry for a later slice (760-776) | `Loading pull request diff…` and `Loading more files…`. The first slice's failure shows alone, and `r` or showing the Code after the details open again reads it again. A later slice's failure shows `t3term couldn't read the rest of this diff.`, the reason and `↻ Retry`, which ↑ and ↓ choose. | | Empty | `This pull request has no file changes.` (1366-1372) | The same, or `t3term couldn't split this patch into files.` where the patch has text but no file the parser could read. | @@ -152,7 +153,7 @@ The desktop opens a link in its own pull request view only when the environment' 4. **Refreshes.** Done: while the details hold a read, t3term follows `pullRequests.subscribeRefreshes` and reads the link again on each new number, and `r` sends `pullRequests.invalidate` with the link's reference before it reads again, as above. The invalidate spends host requests and makes every reader read again, so t3term sends it only for that key. The details also read the detail again every 5 minutes while you use t3term, and when you arrive at them: you come back after 6 minutes away, open the link's details again or the terminal gains focus, as the desktop's live refresh does (apps/web/src/hooks/useLiveRefresh.ts). 5. **The stack.** Done in part: where the host keeps stacks, a link's details read `pullRequests.stack` and list its layers and their states on the Summary, as above. The stack saved on the link shows until the first read of it came. Opening a layer's details from the stack is left. 6. **The project's list.** `pullRequests.list` and `pullRequests.listStats` list a project's pull requests outside any thread. That is a separate view, after the page. -7. **The diff.** Done in part: the Code tab reads the diff a slice at a time and shows its files, as above. Next come the desktop's file order, the review threads beside their lines, the commit picker, more lines around a hunk from `pullRequests.diffFileContents`, viewed files, and the toolbar's toggles. +7. **The diff.** Done in part: the Code tab reads the diff a slice at a time and shows its files, as above. Each slice's files show in the desktop's reading order. Next come the review threads beside their lines, the commit picker, more lines around a hunk from `pullRequests.diffFileContents`, viewed files, and the toolbar's toggles. Linking, unlinking, watching, reviewing, commenting, merging and every other write stay out until a person can confirm each one in t3term. @@ -191,7 +192,7 @@ Linking, unlinking, watching, reviewing, commenting, merging and every other wri - The stack shows only on the Summary, where the desktop's header shows it on every tab. A layer doesn't open its pull request, where the desktop's menu opens it in the panel. - The saved stack comes from the open thread's links. The desktop looks through the links of every thread on the environment and takes the newest by `syncedAt`, so a layer linked only from another thread shows its branch in t3term where the desktop shows its title. Of two links to one pull request on a thread, t3term keeps the first, where the desktop takes the newest. - t3term can't merge or rebase a stack, and doesn't keep the layers' head commits that the desktop's stack merge sends. -- The Code tab shows files in the order the patch has them. The desktop puts source first, in the order its imports need, then the tests that cover it, then what a tool wrote (pullRequestFileOrder.logic.ts:145-149). +- The Code tab compares paths as the desktop's `localeCompare` does for ASCII. A character outside ASCII comes after the letters, where the desktop's collation puts an accented letter with its base letter, so `é.ts` comes after `z.ts` rather than before it. - The Code tab has no review threads, commit picker, more lines around a hunk, viewed files, file tree, raw patch, or the desktop's whitespace, split view, wrap and fold-all toggles. Its lines don't wrap at word breaks or show syntax colors. - The Code tab reads the diff only while it shows. The desktop's stays mounted and reads on a refresh while another tab shows, so its files are ready when the reader comes back. - The files read before show while the first slice is read again. The desktop drops them and shows `Loading pull request diff...`. diff --git a/src/tui/pull_requests/code.rs b/src/tui/pull_requests/code.rs index dc9c4ebd..3802c269 100644 --- a/src/tui/pull_requests/code.rs +++ b/src/tui/pull_requests/code.rs @@ -9,7 +9,9 @@ //! number from T3 or a new `updatedAt` on the detail, as the desktop's refresh token says //! (PullRequestDetailPanel.tsx:832-847). The slices read show until the new first one comes. //! -//! Each file shows folded on one row, as the desktop's do by default: whether the change added, +//! Each slice's files show in the desktop's reading order, source before tests before the files +//! a tool wrote, as `order` says. Each file shows folded on one row, as the desktop's do by +//! default: whether the change added, //! deleted, renamed or modified it, its path, and the lines it adds and removes. Enter or a //! click unfolds it into its hunks. Each line has one number, the one it has after the change, //! or before it for a removed line, as the desktop's stacked view has, and the unchanged lines @@ -53,9 +55,9 @@ const UNREAD: &str = "t3term couldn't split this patch into files."; pub(super) struct Code { /// The slices read, in the order T3 sent them. slices: Vec, - /// The files of every slice in order, as a slice's place and a file's place in it. A path - /// a slice has twice shows once, as its last part, as the desktop's `orderDiffFiles` keeps - /// it. + /// The files of every slice, as a slice's place and a file's place in it: the slices in the + /// order T3 sent them, and each slice's files in reading order. A path a slice has twice + /// shows once, as its last part, as the desktop's `orderDiffFiles` keeps it. files: Vec<(usize, usize)>, /// The lines the host counted for each file whose changes it withheld, by path, from every /// slice. @@ -87,6 +89,8 @@ struct Asked { struct Slice { diff: pull_request::Diff, patch: patch::Patch, + /// The places of its files in reading order, as `order::reading_order` says. + order: Vec, } impl Code { @@ -163,7 +167,12 @@ impl Code { self.at = now; } let patch = patch::Patch::read(&diff.patch); - self.slices.push(Slice { diff: *diff, patch }); + let order = order::reading_order(&diff.patch, &patch); + self.slices.push(Slice { + diff: *diff, + patch, + order, + }); self.failed = None; self.index(); } @@ -177,14 +186,8 @@ impl Code { self.files.clear(); self.omitted.clear(); for (at, slice) in self.slices.iter().enumerate() { - let mut seen = HashSet::new(); - let mut files: Vec<(usize, usize)> = (0..slice.patch.files.len()) - .rev() - .filter(|&file| seen.insert(slice.patch.files[file].path())) - .map(|file| (at, file)) - .collect(); - files.reverse(); - self.files.extend(files); + self.files + .extend(slice.order.iter().map(|&file| (at, file))); for omitted in &slice.diff.omitted { let counts = (omitted.additions, omitted.deletions); self.omitted.insert(omitted.path.clone(), counts); @@ -615,5 +618,7 @@ fn broken(text: &str, width: usize) -> Vec { rows } +mod order; + #[cfg(test)] mod tests; diff --git a/src/tui/pull_requests/code/order.rs b/src/tui/pull_requests/code/order.rs new file mode 100644 index 00000000..effc08e4 --- /dev/null +++ b/src/tui/pull_requests/code/order.rs @@ -0,0 +1,332 @@ +//! The order the Code tab shows a slice's files in, as the desktop's `orderDiffFiles` reads +//! them (apps/web/src/components/pullRequest/pullRequestFileOrder.logic.ts at f570bd2): the +//! source first, each file after the changed files it imports, then the tests, each after the +//! source it covers, then the files a tool wrote. The desktop orders each slice on its own, so a +//! slice that comes later never moves the files above it. +//! +//! Imports are found as the desktop finds them, in every line of a file's hunks: a quoted +//! specifier after `from`, `import` or `require(`. A specifier names a changed file by its +//! path, with `./` and `../` resolved and a script's extension dropped, or by the file's name +//! where only one source file in the slice has it. Paths compare as the desktop's +//! `localeCompare` does in the root collation, as `collate` says. + +use std::cmp::Ordering; +use std::collections::{BTreeSet, HashMap, HashSet}; + +use crate::pull_request::patch::{Kind, Patch}; + +/// Lockfiles, which a tool writes. +const GENERATED_NAMES: [&str; 8] = [ + "pnpm-lock.yaml", + "yarn.lock", + "package-lock.json", + "bun.lockb", + "Cargo.lock", + "go.sum", + "composer.lock", + "Gemfile.lock", +]; +const GENERATED_DIRECTORIES: [&str; 5] = + ["__snapshots__", "__generated__", "dist", "build", "vendor"]; +const TEST_DIRECTORIES: [&str; 3] = ["__tests__", "tests", "test"]; +/// The extensions a specifier may name that the path it means drops, as TypeScript's imports +/// name the `.js` beside a `.ts`. +const MODULE_EXTENSIONS: [&str; 6] = [".ts", ".tsx", ".js", ".jsx", ".mjs", ".cjs"]; + +/// Which of the three readings a file belongs to. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum Tier { + Source, + Test, + Generated, +} + +/// The places of `patch`'s files in reading order, one for each path, the last file that has +/// it, as the desktop's map of the files by path keeps. `text` is the patch the lines are +/// ranges of. +pub(super) fn reading_order(text: &str, patch: &Patch) -> Vec { + // Each path, where it first comes, with the place of the last file that has it. + let mut places: HashMap<&str, usize> = HashMap::new(); + let mut paths: Vec<&str> = Vec::new(); + for (at, file) in patch.files.iter().enumerate() { + if places.insert(file.path(), at).is_none() { + paths.push(file.path()); + } + } + let tiers: HashMap<&str, Tier> = paths.iter().map(|&path| (path, tier(path))).collect(); + let tiers = &tiers; + let of = |wanted: Tier| { + paths + .iter() + .copied() + .filter(move |path| tiers[path] == wanted) + }; + + let source: Vec<&str> = of(Tier::Source).collect(); + let mut modules: HashMap<&str, &str> = HashMap::new(); + let mut names: HashMap<&str, &str> = HashMap::new(); + let mut shared = HashSet::new(); + for &path in &source { + modules.insert(strip_extension(path), path); + let name = base_name(path); + if names.insert(name, path).is_some() { + shared.insert(name); + } + } + // A name two files have could mean either, so it means neither. + names.retain(|name, _| !shared.contains(name)); + let imports: HashMap<&str, HashSet<&str>> = source + .iter() + .map(|&path| { + let file = &patch.files[places[path]]; + let lines = file + .hunks + .iter() + .flat_map(|hunk| &hunk.lines) + .filter(|line| line.kind != Kind::NoNewline) + .map(|line| &text[line.text.clone()]); + (path, imported(path, lines, &modules, &names)) + }) + .collect(); + let source = by_imports(source, &imports); + + // A test comes after the source file whose name it has, by where that file is. + let mut positions: HashMap<&str, usize> = HashMap::new(); + for (at, &path) in source.iter().enumerate() { + positions.entry(base_name(path)).or_insert(at); + } + let mut tests: Vec<&str> = of(Tier::Test).collect(); + tests.sort_by(|left, right| { + let position = |path: &str| { + positions + .get(tested_name(path)) + .copied() + .unwrap_or(usize::MAX) + }; + position(left) + .cmp(&position(right)) + .then_with(|| collate(left, right)) + }); + let mut generated: Vec<&str> = of(Tier::Generated).collect(); + generated.sort_by(|left, right| collate(left, right)); + + source + .into_iter() + .chain(tests) + .chain(generated) + .map(|path| places[path]) + .collect() +} + +/// Which reading `path` belongs to, from its name and the directories it is in. +fn tier(path: &str) -> Tier { + let (directories, name) = match path.rsplit_once('/') { + Some((directories, name)) => (Some(directories), name), + None => (None, path), + }; + let directories = directories.into_iter().flat_map(|path| path.split('/')); + let within = |names: &[&str]| directories.clone().any(|segment| names.contains(&segment)); + if GENERATED_NAMES.contains(&name) + || name.ends_with(".snap") + || name.ends_with(".min.js") + || name.ends_with(".min.css") + || name.contains(".generated.") + || within(&GENERATED_DIRECTORIES) + { + Tier::Generated + } else if name.contains(".test.") || name.contains(".spec.") || within(&TEST_DIRECTORIES) { + Tier::Test + } else { + Tier::Source + } +} + +/// `path` without the extension of its last segment. +fn strip_extension(path: &str) -> &str { + match path.rfind('.') { + Some(dot) if path.rfind('/').is_none_or(|slash| dot > slash) => &path[..dot], + _ => path, + } +} + +/// The name of `path`'s last segment, without its extension. +fn base_name(path: &str) -> &str { + strip_extension(path.rsplit('/').next().unwrap_or_default()) +} + +/// The name of the file a test covers: `foo.test.ts` and `__tests__/foo.ts` both cover `foo`. +fn tested_name(path: &str) -> &str { + let name = path.rsplit('/').next().unwrap_or_default(); + let name = [".test.", ".spec."] + .iter() + .filter_map(|mark| name.find(mark)) + .min() + .map_or(name, |at| &name[..at]); + match name.rfind('.') { + Some(dot) if dot + 1 < name.len() => &name[..dot], + _ => name, + } +} + +/// `specifier` from the directory of `from`, its `.` and `..` segments resolved. +fn resolve_relative(from: &str, specifier: &str) -> String { + let mut segments: Vec<&str> = from.split('/').collect(); + segments.pop(); + for part in specifier.split('/') { + match part { + "." | "" => {} + ".." => { + segments.pop(); + } + _ => segments.push(part), + } + } + segments.join("/") +} + +/// The changed source files other than `path` that `lines` of it import, by path. +fn imported<'a, 'b>( + path: &str, + lines: impl Iterator, + modules: &HashMap<&str, &'a str>, + names: &HashMap<&str, &'a str>, +) -> HashSet<&'a str> { + let mut imported = HashSet::new(); + for line in lines { + for specifier in specifiers(line) { + let named = match specifier.starts_with('.') { + true => resolve_relative(path, specifier), + false => specifier.to_string(), + }; + let resolved = MODULE_EXTENSIONS + .iter() + .find_map(|extension| named.strip_suffix(extension)) + .unwrap_or(&named); + let target = modules + .get(resolved) + .or_else(|| modules.get(format!("{resolved}/index").as_str())) + .or_else(|| names.get(base_name(resolved))); + if let Some(&target) = target.filter(|&&target| target != path) { + imported.insert(target); + } + } + } + imported +} + +/// The specifiers `line` names, as the desktop's `IMPORT_SPECIFIER` finds them: +/// `(?:\bfrom|\bimport|\brequire\s*\()\s*\(?\s*["']([^"']+)["']`, each search starting where +/// the last match ended. +fn specifiers(line: &str) -> Vec<&str> { + let word = |c: char| c.is_ascii_alphanumeric() || c == '_'; + // JavaScript's `\s` also takes the byte order mark. + let space = |c: char| c.is_whitespace() || c == '\u{feff}'; + let skip = |at: usize, wanted: &dyn Fn(char) -> bool| { + line[at..] + .find(|c: char| !wanted(c)) + .map_or(line.len(), |gap| at + gap) + }; + let mut found = Vec::new(); + let mut at = 0; + while at < line.len() { + let boundary = !line[..at].chars().next_back().is_some_and(word); + let keyword = ["from", "import", "require"] + .into_iter() + .find(|keyword| boundary && line[at..].starts_with(keyword)); + let matched = keyword.and_then(|keyword| { + let mut next = at + keyword.len(); + if keyword == "require" { + next = skip(next, &space); + next += line[next..].strip_prefix('(').map(|_| 1)?; + } + next = skip(next, &space); + if line[next..].starts_with('(') { + next += 1; + } + next = skip(next, &space); + let quote = |c: char| c == '"' || c == '\''; + next += line[next..].strip_prefix(quote).map(|_| 1)?; + let end = skip(next, &|c| !quote(c)); + (end > next && end < line.len()).then(|| (&line[next..end], end + 1)) + }); + match matched { + Some((specifier, end)) => { + found.push(specifier); + at = end; + } + None => at += line[at..].chars().next().map_or(1, char::len_utf8), + } + } + found +} + +/// The source files in reading order: each after the files it imports, and of those ready, the +/// lowest path first, so files of one directory stay together. Where a cycle leaves none +/// ready, the lowest path left comes next, as the desktop's Kahn's order does. +fn by_imports<'a>(paths: Vec<&'a str>, imports: &HashMap<&str, HashSet<&'a str>>) -> Vec<&'a str> { + let mut sorted = paths; + sorted.sort_by(|left, right| collate(left, right)); + let rank: HashMap<&str, usize> = sorted.iter().enumerate().map(|(at, &p)| (p, at)).collect(); + let mut waiting: Vec = vec![0; sorted.len()]; + let mut dependents: Vec> = vec![Vec::new(); sorted.len()]; + for (at, path) in sorted.iter().enumerate() { + for dependency in imports.get(path).into_iter().flatten() { + if let Some(&dependency) = rank.get(dependency) { + waiting[at] += 1; + dependents[dependency].push(at); + } + } + } + let mut left: BTreeSet = (0..sorted.len()).collect(); + let mut ready: BTreeSet = left + .iter() + .copied() + .filter(|&at| waiting[at] == 0) + .collect(); + let mut ordered = Vec::with_capacity(sorted.len()); + while let Some(next) = ready.pop_first().or_else(|| left.first().copied()) { + left.remove(&next); + ordered.push(sorted[next]); + for &dependent in &dependents[next] { + if left.contains(&dependent) { + waiting[dependent] -= 1; + if waiting[dependent] == 0 { + ready.insert(dependent); + } + } + } + } + ordered +} + +/// Compares two paths as `localeCompare` does in the root collation, which the desktop's +/// sorts use: first by each character's place, with ASCII letters in either case alike and +/// punctuation before digits before letters, then lowercase before uppercase. Other +/// characters come after the letters, by code point, where the collation puts an accented +/// letter with its base letter. The bytes settle a tie. +fn collate(left: &str, right: &str) -> Ordering { + let upper = |path: &str| { + path.chars() + .map(|c| c.is_ascii_uppercase()) + .collect::>() + }; + left.chars() + .map(weight) + .cmp(right.chars().map(weight)) + .then_with(|| upper(left).cmp(&upper(right))) + .then_with(|| left.cmp(right)) +} + +/// A character's place in the root collation, ASCII letters in either case alike. +fn weight(c: char) -> u32 { + const ORDER: &str = + "\t\n\r _-,;:!?.'\"()[]{}@*/\\&#%`^+<=>|~$0123456789abcdefghijklmnopqrstuvwxyz"; + let c = c.to_ascii_lowercase(); + match ORDER.find(c) { + Some(at) => at as u32, + None => ORDER.len() as u32 + c as u32, + } +} + +#[cfg(test)] +mod tests; diff --git a/src/tui/pull_requests/code/order/tests.rs b/src/tui/pull_requests/code/order/tests.rs new file mode 100644 index 00000000..e630bcdc --- /dev/null +++ b/src/tui/pull_requests/code/order/tests.rs @@ -0,0 +1,341 @@ +use super::*; + +/// A patch of files that each add `lines`, or with no hunk where they add none. +fn patch_of(files: &[(&str, &[&str])]) -> String { + let mut text = String::new(); + for (path, lines) in files { + text.push_str(&format!("diff --git a/{path} b/{path}\n")); + if !lines.is_empty() { + text.push_str(&format!("@@ -0,0 +1,{} @@\n", lines.len())); + for line in *lines { + text.push_str(&format!("+{line}\n")); + } + } + } + text +} + +/// The paths of `files` in reading order. +fn order(files: &[(&str, &[&str])]) -> Vec { + let text = patch_of(files); + let patch = Patch::read(&text); + reading_order(&text, &patch) + .into_iter() + .map(|at| patch.files[at].path().to_string()) + .collect() +} + +fn paths(files: &[&str]) -> Vec { + order( + &files + .iter() + .map(|&path| (path, &[][..])) + .collect::>(), + ) +} + +// The cases from the desktop's pullRequestFileOrder.logic.test.ts, in its order. + +#[test] +fn source_comes_before_tests_and_generated_files_whatever_the_convention() { + let source = ["src/app.ts", "src/dist.ts", "src/testing.ts"]; + let tests = [ + "src/__tests__/app.ts", + "src/app.spec.tsx", + "src/app.test.ts", + "test/app.ts", + "tests/helpers/app.ts", + ]; + let generated = [ + "apps/web/package-lock.json", + "dist/app.js", + "packages/core/vendor/lib.js", + "pnpm-lock.yaml", + "public/app.min.js", + "src/__snapshots__/app.ts", + "src/api.generated.ts", + "src/app.test.ts.snap", + ]; + let mut all: Vec<&str> = [&generated[..], &tests, &source].concat(); + all.reverse(); + assert_eq!(paths(&all), [&source[..], &tests, &generated].concat()); + assert_eq!(paths(&[]), Vec::::new()); +} + +#[test] +fn a_file_comes_after_what_it_imports() { + let b = r#"import { b } from "./b";"#; + let c = r#"import { c } from "./c";"#; + assert_eq!( + order(&[("src/a.ts", &[b]), ("src/b.ts", &[])]), + ["src/b.ts", "src/a.ts"] + ); + // A chain the whole way down. + assert_eq!( + order(&[("src/a.ts", &[b]), ("src/b.ts", &[c]), ("src/c.ts", &[])]), + ["src/c.ts", "src/b.ts", "src/a.ts"] + ); + // A specifier that climbs out of the importer's directory. + assert_eq!( + order(&[ + ("src/ui/a.ts", &[r#"import { b } from "../lib/b";"#]), + ("src/lib/b.ts", &[]) + ]), + ["src/lib/b.ts", "src/ui/a.ts"] + ); + // A directory means its index file. + assert_eq!( + order(&[ + ("src/a.ts", &[r#"import { b } from "./lib";"#]), + ("src/lib/index.ts", &[]) + ]), + ["src/lib/index.ts", "src/a.ts"] + ); + // A path that names nothing changed falls back to its last segment. + assert_eq!( + order(&[ + ("src/a.ts", &[r#"import { b } from "~/lib/b";"#]), + ("lib/b.ts", &[]) + ]), + ["lib/b.ts", "src/a.ts"] + ); + // Unless two files have that name. + assert_eq!( + order(&[ + ("src/a.ts", &[r#"import { b } from "~/somewhere/b";"#]), + ("one/b.ts", &[]), + ("two/b.ts", &[]), + ]), + ["one/b.ts", "src/a.ts", "two/b.ts"] + ); + // A specifier with an extension, the `.js` beside a `.ts` among them. + assert_eq!( + order(&[ + ("src/a.ts", &[r#"import { b } from "./b.js";"#]), + ("src/b.ts", &[]) + ]), + ["src/b.ts", "src/a.ts"] + ); + // The path, not the name two files share, finds the `.ts` the `.js` means. + assert_eq!( + order(&[ + ("src/a.ts", &[r#"import { b } from "./b.js";"#]), + ("src/b.ts", &[]), + ("lib/b.ts", &[]), + ]), + ["lib/b.ts", "src/b.ts", "src/a.ts"] + ); + // A file that imports itself waits on nothing. + assert_eq!( + order(&[ + ("src/a.ts", &[r#"import { a } from "./a";"#]), + ("src/b.ts", &[]) + ]), + ["src/a.ts", "src/b.ts"] + ); + // `require` and a bare import. + assert_eq!( + order(&[ + ( + "src/a.ts", + &[r#"const b = require("./b");"#, r#"import "./c";"#] + ), + ("src/b.ts", &[]), + ("src/c.ts", &[]), + ]), + ["src/b.ts", "src/c.ts", "src/a.ts"] + ); +} + +#[test] +fn a_cycle_and_unrelated_files_go_by_path() { + assert_eq!( + order(&[ + ("src/b.ts", &[r#"import { a } from "./a";"#]), + ("src/a.ts", &[r#"import { b } from "./b";"#]), + ]), + ["src/a.ts", "src/b.ts"] + ); + assert_eq!( + paths(&["src/ui/b.ts", "src/lib/z.ts", "src/lib/a.ts"]), + ["src/lib/a.ts", "src/lib/z.ts", "src/ui/b.ts"] + ); +} + +#[test] +fn tests_follow_the_source_they_cover() { + assert_eq!( + order(&[ + ("pnpm-lock.yaml", &[]), + ("src/a.test.ts", &[]), + ("src/__tests__/b.ts", &[]), + ("src/a.ts", &[r#"import { b } from "./b";"#]), + ("src/b.ts", &[]), + ("dist/a.js", &[]), + ]), + [ + "src/b.ts", + "src/a.ts", + "src/__tests__/b.ts", + "src/a.test.ts", + "dist/a.js", + "pnpm-lock.yaml", + ] + ); + // By where their source is, though the paths would sort the other way, and a test whose + // source isn't in the slice comes after those whose source is. + assert_eq!( + order(&[ + ("src/0.test.ts", &[]), + ("src/a.test.ts", &[]), + ("src/b.test.ts", &[]), + ("src/a.ts", &[r#"import { b } from "./b";"#]), + ("src/b.ts", &[]), + ]), + [ + "src/b.ts", + "src/a.ts", + "src/b.test.ts", + "src/a.test.ts", + "src/0.test.ts" + ] + ); + assert_eq!( + paths(&["src/orphan.spec.ts", "src/a.test.ts", "src/a.ts"]), + ["src/a.ts", "src/a.test.ts", "src/orphan.spec.ts"] + ); +} + +#[test] +fn the_same_files_read_the_same_way_in_any_order() { + let b: &[&str] = &[r#"import { b } from "./b";"#]; + let none: &[&str] = &[]; + let mut files = vec![ + ("src/a.ts", b), + ("src/b.ts", none), + ("src/c.ts", none), + ("src/a.test.ts", none), + ("yarn.lock", none), + ]; + let first = order(&files); + files.reverse(); + assert_eq!(order(&files), first); +} + +// What the desktop's tests leave to the library and the browser. + +#[test] +fn imports_count_in_every_line_of_a_hunk_and_a_path_keeps_its_last_file() { + // The desktop's diff library puts a partial patch's unchanged lines on both sides, so an + // import that didn't change still counts. A removed one does too. + let text = r#"diff --git a/src/a.ts b/src/a.ts +@@ -1,2 +1,2 @@ + import { b } from "./b"; +-import { c } from "./c"; ++export const a = 1; +diff --git a/src/b.ts b/src/b.ts +diff --git a/src/c.ts b/src/c.ts +diff --git a/src/c.ts b/src/c.ts +@@ -0,0 +1 @@ ++import { a } from "./a"; +"#; + let patch = Patch::read(text); + // `src/c.ts` keeps its last file, whose import of `a` makes a cycle. `b` is ready first, + // then the cycle goes by path. + assert_eq!(reading_order(text, &patch), [1, 0, 3]); + let text = text.replace("./a", "./nothing"); + let patch = Patch::read(&text); + assert_eq!(reading_order(&text, &patch), [1, 3, 0]); +} + +#[test] +fn specifiers_are_found_as_the_desktop_s_pattern_finds_them() { + assert_eq!(specifiers(r#"import x from "y";"#), ["y"]); + assert_eq!(specifiers("export * from './y'"), ["./y"]); + assert_eq!(specifiers("const y = require ( 'y' )"), ["y"]); + assert_eq!(specifiers(r#"await import("y")"#), ["y"]); + assert_eq!( + specifiers(r#"from"a" from "b" import 'c'"#), + ["a", "b", "c"] + ); + // The pattern takes a quote of either kind at either end. + assert_eq!(specifiers(r#"from "a'"#), ["a"]); + // A word that only starts or ends like a keyword names nothing, nor does an empty or + // unclosed specifier, nor `require` without its parenthesis. + for line in [ + r#"reimport "a""#, + r#"important "a""#, + r#"_from "a""#, + r#"from """#, + r#"from "a"#, + r#"require "a""#, + "from", + "", + ] { + assert_eq!(specifiers(line), Vec::<&str>::new(), "{line}"); + } + // A keyword after a letter outside ASCII starts a word, as JavaScript's `\b` sees it. + assert_eq!(specifiers(r#"éfrom "a""#), ["a"]); +} + +#[test] +fn paths_collate_as_the_root_collation_does() { + let mut paths = vec![ + "src/b.ts", + "src/Button.tsx", + "src/a.ts", + "src/A.ts", + "src/__tests__/a.ts", + "src/a-b.ts", + "src/a_b.ts", + "src/a/x.ts", + "src/a.b.ts", + "src/10.ts", + "src/9.ts", + "src/é.ts", + "src/z.ts", + ]; + paths.sort_by(|left, right| collate(left, right)); + // Node's ICU 77 sorts these the same way, but for `é`, which it puts with `e`. + assert_eq!( + paths, + [ + "src/__tests__/a.ts", + "src/10.ts", + "src/9.ts", + "src/a_b.ts", + "src/a-b.ts", + "src/a.b.ts", + "src/a.ts", + "src/A.ts", + "src/a/x.ts", + "src/b.ts", + "src/Button.tsx", + "src/z.ts", + "src/é.ts", + ] + ); +} + +#[test] +fn names_tiers_and_relative_paths_read_as_the_desktop_s_do() { + assert_eq!(strip_extension("src/a.b/c"), "src/a.b/c"); + assert_eq!(strip_extension("src/a.test.ts"), "src/a.test"); + assert_eq!(base_name("src/lib/a.ts"), "a"); + assert_eq!(base_name("Makefile"), "Makefile"); + assert_eq!(tested_name("src/a.test.ts"), "a"); + assert_eq!(tested_name("src/a.spec.test.ts"), "a"); + assert_eq!(tested_name("src/__tests__/a.ts"), "a"); + assert_eq!(tested_name("tests/a.b.rs"), "a.b"); + assert_eq!(tested_name("tests/a."), "a."); + assert_eq!(resolve_relative("src/ui/a.ts", "../lib/./b"), "src/lib/b"); + assert_eq!(resolve_relative("a.ts", "../../b"), "b"); + assert_eq!(tier("Cargo.lock"), Tier::Generated); + assert_eq!(tier("crates/x/Cargo.lock"), Tier::Generated); + assert_eq!(tier("build.rs"), Tier::Source); + assert_eq!(tier("build/out.rs"), Tier::Generated); + assert_eq!(tier("src/testing.rs"), Tier::Source); + assert_eq!(tier("tests/cli.rs"), Tier::Test); + assert_eq!(tier("src/a.spec.ts"), Tier::Test); + assert_eq!(tier("tests/__snapshots__/a.snap"), Tier::Generated); +} diff --git a/src/tui/pull_requests/code/tests.rs b/src/tui/pull_requests/code/tests.rs index a52914af..2eab5ba5 100644 --- a/src/tui/pull_requests/code/tests.rs +++ b/src/tui/pull_requests/code/tests.rs @@ -80,21 +80,22 @@ Binary files a/logo.png and b/logo.png differ #[test] fn files_show_folded_and_each_unfolds_into_its_hunks() { + // In reading order, which for files that import nothing is by path. let mut code = read(slice(PATCH, None), 0); assert_eq!( rows(&code, 40), [ "5 files", "", - "› M src/lib.rs +2 -1", - "", "› A docs/new.md +2 -0", "", + "› M logo.png", + "", "› D old.txt +0 -1", "", - "› R from.rs → to.rs", + "› M src/lib.rs +2 -1", "", - "› M logo.png", + "› R from.rs → to.rs", ] ); let (_, targets) = code.lay_out(40, &theme()); @@ -119,6 +120,18 @@ fn files_show_folded_and_each_unfolds_into_its_hunks() { [ "5 files", "", + "⌄ A docs/new.md +2 -0", + "1 + # New", + "2 + Text", + " No newline at end of file", + "", + "⌄ M logo.png", + "A binary file, whose changes have no", + "lines to show.", + "", + "⌄ D old.txt +0 -1", + "1 - gone", + "", "⌄ M src/lib.rs +2 -1", " 1 use std::fmt;", " 2 - fn old() {}", @@ -129,26 +142,14 @@ fn files_show_folded_and_each_unfolds_into_its_hunks() { "11 + fn b() {}", "12 fn c() {}", "", - "⌄ A docs/new.md +2 -0", - "1 + # New", - "2 + Text", - " No newline at end of file", - "", - "⌄ D old.txt +0 -1", - "1 - gone", - "", "⌄ R from.rs → to.rs", "No lines changed.", - "", - "⌄ M logo.png", - "A binary file, whose changes have no", - "lines to show.", ] ); let (_, targets) = code.lay_out(40, &theme()); - assert_eq!(targets[1], (12..13, Control::File(1))); + assert_eq!(targets[1], (7..8, Control::File(1))); code.act(Control::File(0)); - assert_eq!(rows(&code, 40)[2..4], ["› M src/lib.rs +2 -1", ""]); + assert_eq!(rows(&code, 40)[2..4], ["› A docs/new.md +2 -0", ""]); } #[test] From f863b6e81de50fdbef03ea58bbeb4eda9ebc3811 Mon Sep 17 00:00:00 2001 From: Krish Garg Date: Sat, 17 Oct 2026 16:06:40 -0700 Subject: [PATCH 2/2] Check the Code tab's file order in its recipe Each slice's files now come in the desktop's reading order, so the recipe checks the order of the Rust diff's rows and of a TypeScript diff whose files import each other, and chooses the reader with a second Down now that the binary comes first. Co-Authored-By: Claude Opus 5.5 --- .../features/pull-request-code.md | 32 +++++++++++++------ 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/.claude/skills/verify-t3term/features/pull-request-code.md b/.claude/skills/verify-t3term/features/pull-request-code.md index 3fec9653..42466bf5 100644 --- a/.claude/skills/verify-t3term/features/pull-request-code.md +++ b/.claude/skills/verify-t3term/features/pull-request-code.md @@ -6,6 +6,7 @@ - **Reading the diff.** The first time the Code shows after the details open, t3term POSTs `/api/pull-requests/diff` with the link's project, host, repository and number and no cursor. `Loading pull request diff…` shows until the first slice comes. Once a frame shows the end of the files within two pages of the view, t3term reads the next slice with the cursor the last one gave. One slice is read at a time. Nothing reads the diff before the Code shows, and going to another tab and back within the minute reads nothing. - **The count and the note.** The count reads `5 files+` while more slices are left, with `Loading more files…` under the files while the next one is read, then `8 files`. `The host withheld part of this diff, such as a binary file or a change too large to show.` shows in yellow under it when a slice says the host left something out. - **Files.** Each file starts folded: `› M src/config/reader.rs +7 -5`, with `A` green, `D` red, `R` blue and `M` yellow, and the counts in green and red. A rename shows both paths, `› R src/config/source.rs → src/config/sources.rs`, with the counts on the next row where they don't fit. A file whose hunks the host left out takes the host's counts, as in `› M Cargo.lock +40 -12`. +- **Order.** Each slice's files show in the desktop's reading order, not the patch's. Source files come first, each after the changed files it imports and otherwise by path. Tests come next, each after the source file whose name it has, then lockfiles, snapshots and files under `dist`, `build`, `vendor`, `__generated__` or `__snapshots__`, by path. A later slice never moves the files above it. So the first slice reads `assets/config-flow.png`, `src/config/reader.rs`, `src/config/schema.rs`, the rename and `tests/config_reload.rs`, and the second `docs/config.md`, `src/legacy/reader_v1.rs` and `Cargo.lock`. - **Unfolding.** Enter on the chosen file, or a click on it, turns `›` into `⌄` and shows its lines. Each line has its number in grey, the new one or the old for a removed line, then `-` and the line in red, `+` and the line in green, or a space and the line. `⋯ 14 unmodified lines` comes before a hunk that doesn't start at line 1. A line that wraps leaves the number and the mark blank on its later rows, and a tab stops every 4 columns. A binary file reads `A binary file, whose changes have no lines to show.` and a file the host withheld `The host withheld this file's changes.` Enter or a click again folds the file. - **Choosing.** Once a file or `↻ Retry` is in view, ↓ chooses the next one and ↑ the one before, and the status line leads with what Enter does: `Enter expand file`, `Enter collapse file` or `Enter retry`. Where none is in view, they scroll. - **Reading again.** A new refresh number, `r` and a detail whose `updatedAt` changed each read the first slice again. The files read before stay, each with its fold, until the new slice replaces them, and the count goes back to `5 files+` until the end of the files comes near again. After `r` the diff waits for the refresh number `pullRequests.invalidate` raises, or for the end of the read that sent it, so T3 has forgotten the diff it held. @@ -17,7 +18,7 @@ Open a thread, open its details (Alt+B), press → for `Pull requests`, choose a ## Driving it with t3v -Scenario `links` at 100x30. The diff route answers at once with two slices. The first holds `src/config/reader.rs`, `src/config/schema.rs`, the rename `src/config/source.rs → src/config/sources.rs`, the binary `assets/config-flow.png` and the new `tests/config_reload.rs`, with `truncated` and the cursor `s2`. The second holds `docs/config.md`, the deleted `src/legacy/reader_v1.rs` and a `Cargo.lock` with a header and no hunks, which the host counted at +40 -12. Files in the run directory change the reads: `diff-slow` holds each slice 4 s, `diff-fail` fails the first slice and `diff-fail-rest` the second with `GitHub took too long to answer.`, and `diff-off` makes the detail say the host gives no diff. The server logs each read of the diff in events.jsonl as `pr.diff` with its cursor, and `reads` lists those cursors, `null` for the first slice. +Scenario `links` at 100x30. The diff route answers at once with two slices. The first holds `src/config/reader.rs`, `src/config/schema.rs`, the rename `src/config/source.rs → src/config/sources.rs`, the binary `assets/config-flow.png` and the new `tests/config_reload.rs`, with `truncated` and the cursor `s2`. The second holds `docs/config.md`, the deleted `src/legacy/reader_v1.rs` and a `Cargo.lock` with a header and no hunks, which the host counted at +40 -12. Files in the run directory change the reads: `diff-slow` holds each slice 4 s, `diff-fail` fails the first slice and `diff-fail-rest` the second with `GitHub took too long to answer.`, `diff-off` makes the detail say the host gives no diff, and `diff-ts` serves a TypeScript diff in one slice. Its patch has `src/settings/panel.tsx`, which imports `./store` and `../lib/format`, then `src/settings/panel.test.tsx`, `src/settings/store.ts`, which imports `../lib/format`, `pnpm-lock.yaml` and the new `src/lib/format.ts`. The server logs each read of the diff in events.jsonl as `pr.diff` with its cursor, and `reads` lists those cursors, `null` for the first slice. ```bash $T3V start --cols 100 --rows 30 @@ -41,23 +42,25 @@ $T3V capture more-100x30 $T3V wait --text "8 files" --text "› M docs/config.md +2 -1" --text "› D src/legacy/reader_v1.rs +0 -4" --gone "Loading more files…" --idle 300 rm "$RUN/diff-slow" [ "$(reads)" = '[null,"s2"]' ] -S=$($T3V capture files-100x30 | grep 'ans$') +C=$($T3V capture files-100x30); S=$(echo "$C" | grep 'ans$'); T=$(echo "$C" | grep 'txt$') +# Each slice in reading order: source by path, as none imports another, then the test. Cargo.lock is below the view. +[ "$(grep -o '› [AMDR] [^ ]*' "$T" | paste -sd' ' -)" = '› M assets/config-flow.png › M src/config/reader.rs › M src/config/schema.rs › R src/config/source.rs › A tests/config_reload.rs › M docs/config.md › D src/legacy/reader_v1.rs' ] grep -q '38;2;255;185;0mM.\[38;2;245;245;245m src/config/reader.rs .\[38;2;0;212;146m+7.\[38;2;245;245;245m .\[38;2;255;100;103m-5' "$S" && grep -q '38;2;43;127;255mR.\[38;2;245;245;245m src/config/source.rs → src/config/sources.rs' "$S" && grep -q '38;2;0;212;146mA.\[38;2;245;245;245m tests/config_reload.rs' "$S" && grep -q '38;2;255;100;103mD.\[38;2;245;245;245m src/legacy/reader_v1.rs' "$S" && grep -q '38;2;255;185;0mThe host withheld part' "$S" -# ↓ chooses the first file and Enter unfolds it: numbers in grey, removed lines red, added green. -$T3V keys Down && $T3V wait --text "Enter expand file" -$T3V keys Enter && $T3V wait --text "⌄ M src/config/reader.rs +7 -5" --text " 4 - use crate::config::Source;" --text " 4 + use crate::config::schema::Schema;" --text "⋯ 14 unmodified lines" --text "Enter collapse file" +# ↓ chooses the first file, the binary, and ↓ again the reader. Enter unfolds it: numbers in grey, removed lines red, added green. +$T3V keys Down Down && $T3V wait --text "Enter expand file" +$T3V keys Enter && $T3V wait --text "⌄ M src/config/reader.rs +7 -5" --text " 4 - use crate::config::Source;" --text " 4 + use crate::config::schema::Schema;" --text "Enter collapse file" S=$($T3V capture reader-100x30 | grep 'ans$') grep -q '38;2;129;129;129m 4 .\[38;2;255;100;103m- use crate::config::Source;' "$S" && grep -q '38;2;129;129;129m 4 .\[38;2;0;212;146m+ use crate::config::schema::Schema;' "$S" && grep -q '38;2;129;129;129m 1 .\[38;2;245;245;245m use std::fs;' "$S" -# Its tab stops at 4 columns, and a wrapped line leaves the number and the mark blank. -$T3V keys PgDn && $T3V wait --text "28 - Ok(config)" --text "33 + // A reload reads and checks the file a" --text " s a load does, so a file edited on disk since t" -$T3V keys Home && $T3V keys Down && $T3V wait --text "Enter collapse file" +# The unmodified lines before its second hunk, a tab stopping at 4 columns, and a wrapped line leaving the number and the mark blank. +$T3V keys PgDn && $T3V wait --text "⋯ 14 unmodified lines" --text "28 - Ok(config)" --text "33 + // A reload reads and checks the file a" --text " s a load does, so a file edited on disk since t" +$T3V keys Home && $T3V keys Down Down && $T3V wait --text "Enter collapse file" $T3V keys Enter && $T3V wait --text "› M src/config/reader.rs +7 -5" --gone "⋯ 14 unmodified lines" # A click unfolds the others: the binary, the rename and the new file with its tabs. $T3V click --text "tests/config_reload.rs" && $T3V wait --text "⌄ A tests/config_reload.rs +8 -0" --text "4 + fn a_reload_checks_the_schema() {" -$T3V click --text "assets/config-flow.png" && $T3V wait --text "⌄ M assets/config-flow.png" --text "A binary file, whose changes have no lines to show." $T3V click --text "src/config/source.rs" && $T3V wait --text "⌄ R src/config/source.rs → src/config/sources.rs" --text "1 - /// Where a config comes from." --text "1 + /// Where a config comes from: a file, the envir" +$T3V click --text "assets/config-flow.png" && $T3V wait --text "⌄ M assets/config-flow.png" --text "A binary file, whose changes have no lines to show." $T3V capture kinds-100x30 $T3V keys End && $T3V wait --text "› M Cargo.lock +40 -12" --text "5 + let reader = Reader::new(\"tests/newer.toml\")" $T3V click --text "Cargo.lock" && $T3V wait --text "⌄ M Cargo.lock +40 -12" @@ -115,6 +118,15 @@ $T3V refresh && $T3V wait --text "← List Summary Timeline Code" --text "r r $T3V keys Right Right && $T3V wait --text "5 files+" --text "⌄ M src/config/reader.rs +7 -5" settle '[null]' +# A TypeScript diff: format.ts, which the others import, then store.ts, which imports it, then panel.tsx, which imports both, then its test and the lockfile. +touch "$RUN/diff-ts" +mark +$T3V refresh && settle '[null,null]' +$T3V wait --text "› A src/lib/format.ts" --text "› M pnpm-lock.yaml" --gone "src/config/reader.rs" --idle 300 +rm "$RUN/diff-ts" +T=$($T3V capture typescript-100x30 | grep 'txt$') +[ "$(grep -o '› [AMDR] [^ ]*' "$T" | paste -sd' ' -)" = '› A src/lib/format.ts › M src/settings/store.ts › M src/settings/panel.tsx › M src/settings/panel.test.tsx › M pnpm-lock.yaml' ] + # Another thread's link, whose first slice fails: the failure alone. Opening the details again reads it again. touch "$RUN/diff-fail" $T3V keys Escape && $T3V wait --gone "Thread details" @@ -133,7 +145,7 @@ $T3V keys Right Right && $T3V wait --text "8 files" --text "› M src/config/rea # At 80x24 every row still fits the pane. $T3V resize 80 24 && $T3V wait --text "8 files" --idle 300 -$T3V keys Down Enter && $T3V wait --text "⌄ M src/config/reader.rs +7 -5" --text " 4 + use crate::config::schema::Schema;" --text "Enter collapse file" +$T3V keys Down Down Enter && $T3V wait --text "⌄ M src/config/reader.rs +7 -5" --text " 4 + use crate::config::schema::Schema;" --text "Enter collapse file" $T3V capture reader-80x24 $T3V status --json | jq -e '.scopeViolations == 0 and .unknownRpcs == 0 and .fixtureErrors == 0' ```