docs(vcs): workdir_root doctest asserts None == None, not the convergence it claims

[dekobon]

Summary

The no_run doctest on vcs::workdir_root claims to demonstrate that two
files in the same checkout share one work-tree root, but as written it
asserts None == None and proves nothing about that claim.

Location

• src/vcs/mod.rs:366-372 (the doc example on workdir_root)

Evidence

/// ```no_run
/// use std::path::Path;
/// // Two files in the same checkout share one work-tree root.
/// let a = big_code_analysis::vcs::workdir_root(Path::new("repo/src/a.rs"));
/// let b = big_code_analysis::vcs::workdir_root(Path::new("repo/tests/b.rs"));
/// assert_eq!(a, b);
/// ```

workdir_root (impl in src/vcs/git/repo.rs:59-72) resolves the directory
to probe like so: for a non-existent path it takes path.parent()
(repo/src and repo/tests respectively), then calls open(directory).
Both repo/src and repo/tests are relative paths that do not exist
relative to the doctest's working directory, so open(...) returns Err
and workdir_root returns None for both inputs.

The assertion therefore reduces to assert_eq!(None, None), which holds
regardless of whether the "same checkout shares one root" property is true.
The two inputs also resolve via different parent directories
(repo/src vs repo/tests), so the example does not even feed
workdir_root two paths that would converge on one real work tree.

The example is no_run, so it never executes in CI and cannot fail; its
only effect is documentation. A reader who removes no_run to verify the
behavior would get a test that passes for the wrong reason (the
vacuous-assert failure mode tracked elsewhere, e.g. #787).

Expected Behavior

The example should either:

• demonstrate the documented "same checkout, same root" property against a
  real repository (necessarily no_run, but using paths that, when run in a
  checkout, actually return Some(root) for both and converge), with a
  comment making clear it requires a real checkout; or
• be reduced to a prose statement of the contract if a self-contained,
  meaningful runnable example is not feasible without fixture setup.

The current form should not present assert_eq!(a, b) as evidence of the
convergence property when both sides are None for unrelated reasons.

Actual Behavior

The doc example asserts None == None for two non-existent relative paths
and is presented as demonstrating that two files in the same checkout share
one work-tree root. It demonstrates neither convergence nor the Some-root
case.

Impact

Documentation only. A contributor reading or attempting to run the example
is misled about what is being verified; the example provides false
assurance about the convergence contract that issue #670 relies on.

---

Resolution

Rewrote the workdir_root no_run doctest to illustrate same-checkout paths sharing one Some(root) instead of the vacuous None == None assertion. Commit a7e35ad.

[dekobon]

General Plan for Fix

Root Cause

The example was written to illustrate the issue #670 convergence property
("two files in one checkout share one work-tree root"), but it uses two
non-existent relative paths. workdir_root returns None for both because
their parent directories (repo/src, repo/tests) do not exist relative to
the doctest cwd, so the assert_eq!(a, b) collapses to None == None and
verifies nothing. Because the block is no_run, the weakness is invisible
to CI.

Implementation Steps

1. Rewrite the doc example so it demonstrates the actual contract without
   relying on filesystem state at doctest time. Options, simplest first:
   • Keep no_run, but make the example clearly conditional on a real
     checkout and assert the meaningful shape, e.g. derive both paths from a
     single discovered root so a reader sees the convergence intent:

     /// # use std::path::Path;
     /// // In a real checkout, both files resolve to the same work-tree root:
     /// let root = std::env::current_dir().unwrap();
     /// let a = big_code_analysis::vcs::workdir_root(&root.join("src"));
     /// let b = big_code_analysis::vcs::workdir_root(&root.join("tests"));
     /// assert_eq!(a, b); // both Some(<repo root>) inside this repo
     

     (still no_run if it must not depend on cwd, but at least the inputs
     are real and convergent rather than two Nones).
   • Or drop the runnable assertion entirely and state the convergence
     contract in prose, cross-referencing feat(python): vcs and vcs_per_function kwargs on analyze_batch #670.
2. Ensure the surrounding doc text continues to name feat(python): vcs and vcs_per_function kwargs on analyze_batch #670 as the motivation.

Tests to Add/Update

• Doc example only. If a runnable form is chosen, confirm cargo test --doc
  exercises it (i.e., not no_run) and that it passes against the real
  checkout; otherwise keep no_run with real, convergent inputs.
• No production-code change is required, so no unit test is needed; the fix
  is purely the example's correctness.

Verification

• cargo doc --no-deps -p big-code-analysis clean (no broken intra-doc
  links after the rewrite)
• cargo test --doc -p big-code-analysis passes (if the example is made
  runnable)
• Manual read: the example, as written, can only pass when the
  convergence property actually holds — it must not reduce to
  None == None

[dekobon]

Fixed in commit a7e35ad. The workdir_root no_run doctest no longer asserts the vacuous None == None. It now illustrates two same-checkout paths sharing one Some(root) and only asserts equality when both resolve, keeping it no_run and compiling under cargo test --doc.

[dekobon]

Fixed and merged to main in #957. GitHub did not auto-close this from the batched commit's single Fixes #a #b #c … trailer (only the first referenced issue auto-closes); closing manually. See the Resolution section in the body for details.