Skip to content

Commit 7653897

Browse files
committed
fix(tests): X1 GitError exception, X6 strict overlap, X16 leading-dot range
1 parent ba20696 commit 7653897

5 files changed

Lines changed: 31 additions & 12 deletions

File tree

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
11
from ._diffctx import * # noqa: F403
2+
from ._diffctx import GitError # noqa: F401 # exception class is not picked up by `import *`

diffctx/src/git.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,15 @@ pub enum GitError {
4949
pub type Result<T> = std::result::Result<T, GitError>;
5050

5151
fn validate_diff_range(diff_range: &str) -> Result<()> {
52-
if !SAFE_RANGE_RE.is_match(diff_range.trim()) {
52+
let trimmed = diff_range.trim();
53+
// Reject leading-dot ranges like `..origin/main` (audit X16): the regex
54+
// character class allows `.`, so a string of only dots/identifier-chars
55+
// passes as a "ref" before being rejected by git itself with a less
56+
// informative `fatal: ambiguous argument`. Surface the dedicated error.
57+
if trimmed.starts_with('.') || trimmed.starts_with('/') {
58+
return Err(GitError::InvalidRange(diff_range.to_string()));
59+
}
60+
if !SAFE_RANGE_RE.is_match(trimmed) {
5361
return Err(GitError::InvalidRange(diff_range.to_string()));
5462
}
5563
Ok(())

diffctx/src/interval.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,12 @@ impl IntervalIndex {
4444
if start == frag.start_line() && end == frag.end_line() {
4545
continue;
4646
}
47-
if end >= frag.start_line() {
47+
// Strict `>`: a fragment starting on the very last line of an
48+
// already-selected fragment is adjacent, not overlapping. Compact
49+
// languages (Rust/Go/Scala one-liners, Lisp `}{` chains) routinely
50+
// produce back-to-back fragments sharing exactly that boundary
51+
// line; treating it as overlap silently drops the next fragment.
52+
if end > frag.start_line() {
4853
return true;
4954
}
5055
}

diffctx/src/pybridge.rs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,27 @@
11
use std::path::Path;
22
use std::sync::Arc;
33

4+
use pyo3::create_exception;
45
use pyo3::prelude::*;
56
use pyo3::types::{PyDict, PyList};
67

78
use crate::config::limits::{
89
DEFAULT_PIPELINE_TIMEOUT_SECONDS, DEFAULT_PPR_ALPHA, DEFAULT_STOPPING_THRESHOLD,
910
};
11+
use crate::git::GitError as RustGitError;
1012
use crate::mode::ScoringMode;
1113
use crate::pipeline;
1214
use crate::render::{DiffContextOutput, FragmentEntry};
1315

16+
create_exception!(_diffctx, GitError, pyo3::exceptions::PyException);
17+
18+
fn map_pipeline_err(e: anyhow::Error) -> PyErr {
19+
if let Some(git_err) = e.downcast_ref::<RustGitError>() {
20+
return GitError::new_err(git_err.to_string());
21+
}
22+
pyo3::exceptions::PyRuntimeError::new_err(e.to_string())
23+
}
24+
1425
#[pyclass]
1526
#[derive(Clone)]
1627
pub struct PyFragment {
@@ -247,7 +258,7 @@ fn build_diff_context<'py>(
247258
timeout,
248259
)
249260
})
250-
.map_err(|e| pyo3::exceptions::PyRuntimeError::new_err(e.to_string()))?;
261+
.map_err(map_pipeline_err)?;
251262
let total_ms = start.elapsed().as_secs_f64() * 1000.0;
252263

253264
let dict = PyDict::new(py);
@@ -569,6 +580,7 @@ pub fn _diffctx(m: &Bound<'_, PyModule>) -> PyResult<()> {
569580
m.add_class::<PyProjectGraph>()?;
570581
m.add_class::<PyQuotientGraph>()?;
571582
m.add_class::<PyModuleMetrics>()?;
583+
m.add("GitError", m.py().get_type::<GitError>())?;
572584
Ok(())
573585
}
574586

src/treemapper/diffctx/__init__.py

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,7 @@
11
from __future__ import annotations
22

3-
from .pipeline import build_diff_context
4-
5-
6-
class GitError(Exception):
7-
"""Raised when an underlying git operation fails.
8-
9-
Mirrors the Rust `_diffctx` error surface as a plain Python exception so
10-
callers do not need to depend on PyO3-side classes.
11-
"""
3+
from _diffctx import GitError
124

5+
from .pipeline import build_diff_context
136

147
__all__ = ["GitError", "build_diff_context"]

0 commit comments

Comments
 (0)