Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions codebase_rag/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
from . import cli_help as ch
from . import constants as cs
from . import logs as ls
from .config import load_cgrignore_patterns, settings
from .config import load_ignore_patterns, settings
from .graph_updater import GraphUpdater
from .main import (
_create_configuration_table,
Expand Down Expand Up @@ -192,7 +192,7 @@ def _run_graph_sync(
clean: bool = False,
output: str | None = None,
) -> None:
cgrignore = load_cgrignore_patterns(repo)
cgrignore = load_ignore_patterns(repo)
cli_excludes = frozenset(exclude) if exclude else frozenset()
exclude_paths = cli_excludes | cgrignore.exclude or None
unignore_paths: frozenset[str] | None
Expand Down Expand Up @@ -535,7 +535,7 @@ def index(

_info(style(cs.CLI_MSG_OUTPUT_TO.format(path=output_proto_dir), cs.Color.CYAN))

cgrignore = load_cgrignore_patterns(repo_to_index)
cgrignore = load_ignore_patterns(repo_to_index)
cli_excludes = frozenset(exclude) if exclude else frozenset()
exclude_paths = cli_excludes | cgrignore.exclude or None
unignore_paths: frozenset[str] | None = None
Expand Down
32 changes: 29 additions & 3 deletions codebase_rag/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -360,13 +360,13 @@ def resolve_batch_size(self, batch_size: int | None) -> int:
settings = AppConfig()

CGRIGNORE_FILENAME = ".cgrignore"
GITIGNORE_FILENAME = ".gitignore"


EMPTY_CGRIGNORE = CgrignorePatterns(exclude=frozenset(), unignore=frozenset())


def load_cgrignore_patterns(repo_path: Path) -> CgrignorePatterns:
ignore_file = repo_path / CGRIGNORE_FILENAME
def _load_ignore_file(ignore_file: Path) -> CgrignorePatterns:
if not ignore_file.is_file():
return EMPTY_CGRIGNORE

Expand Down Expand Up @@ -394,11 +394,37 @@ def load_cgrignore_patterns(repo_path: Path) -> CgrignorePatterns:
exclude=frozenset(exclude),
unignore=frozenset(unignore),
)
except OSError as e:
except (OSError, ValueError) as e:
logger.warning(logs.CGRIGNORE_READ_FAILED.format(path=ignore_file, error=e))
return EMPTY_CGRIGNORE


def load_cgrignore_patterns(repo_path: Path) -> CgrignorePatterns:
return _load_ignore_file(repo_path / CGRIGNORE_FILENAME)


def load_ignore_patterns(repo_path: Path) -> CgrignorePatterns:
# (H) Merged exclude/unignore set for indexing: root .gitignore (gitignored
# (H) paths are build artifacts / generated output whose symbols pollute the
# (H) graph and the dead-code report) plus .cgrignore, which stays the
# (H) authoritative cgr-specific channel. The runtime skip check gives
# (H) excludes precedence over unignores, so a negation can only override a
# (H) .gitignore exclude by CANCELLING the exact same pattern string here at
# (H) load time (`!generated/` drops `generated/`). .cgrignore excludes are
# (H) never cancelled by .gitignore negations.
# (H) ponytail: root .gitignore only, exact-string cancellation only; a
# (H) finer-grained negation (`!dist/keep.py` under excluded `dist/`) still
# (H) cannot rescue -- an ordered PathSpec soft layer in should_skip_path is
# (H) the upgrade path if real repos need it.
cgr = _load_ignore_file(repo_path / CGRIGNORE_FILENAME)
git = _load_ignore_file(repo_path / GITIGNORE_FILENAME)
negations = cgr.unignore | git.unignore
return CgrignorePatterns(
exclude=cgr.exclude | (git.exclude - negations),
unignore=negations,
Comment on lines +421 to +424

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Preserve gitignore order
load_ignore_patterns() collapses .gitignore into sets, so dist/\n!dist/\ndist/ returns dist/ only in unignore and indexes dist/, while Git's documented rule is that within one precedence level the last matching pattern decides the outcome. Repos that re-ignore after a negation will now include generated files that Git ignores; the loader needs to preserve pattern order or compile an ordered PathSpec for .gitignore instead of subtracting all negations globally.

Rule Used: ## Technical Requirements

Agentic Framework

-... (source)

Prompt To Fix With AI
This is a comment left during a code review.
Path: codebase_rag/config.py
Line: 421-424

Comment:
**Preserve gitignore order**
`load_ignore_patterns()` collapses `.gitignore` into sets, so `dist/\n!dist/\ndist/` returns `dist/` only in `unignore` and indexes `dist/`, while Git's documented rule is that within one precedence level the last matching pattern decides the outcome. Repos that re-ignore after a negation will now include generated files that Git ignores; the loader needs to preserve pattern order or compile an ordered `PathSpec` for `.gitignore` instead of subtracting all negations globally.

**Rule Used:** ## Technical Requirements

### Agentic Framework
-... ([source](https://app.greptile.com/graph-code/github/vitali87/code-graph-rag/-/custom-context?memory=d4240b05-b763-467a-a6bf-94f73e8b6859))

How can I resolve this? If you propose a fix, please make it concise.

)
Comment thread
vitali87 marked this conversation as resolved.
Comment thread
vitali87 marked this conversation as resolved.


CGR_INSTRUCTIONS_FILENAME = ".cgr.md"
GLOBAL_CGR_INSTRUCTIONS_PATH = Path.home() / CGR_INSTRUCTIONS_FILENAME

Expand Down
4 changes: 2 additions & 2 deletions codebase_rag/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
from . import constants as cs
from . import exceptions as ex
from . import logs as ls
from .config import ModelConfig, load_cgrignore_patterns, settings
from .config import ModelConfig, load_ignore_patterns, settings
from .models import AppContext
from .prompts import OPTIMIZATION_PROMPT, OPTIMIZATION_PROMPT_WITH_REFERENCE
from .providers.base import get_provider_from_config
Expand Down Expand Up @@ -1449,7 +1449,7 @@ def prompt_for_unignored_directories(
cli_excludes: list[str] | None = None,
) -> frozenset[str]:
detected = detect_excludable_directories(repo_path)
cgrignore = load_cgrignore_patterns(repo_path)
cgrignore = load_ignore_patterns(repo_path)
cli_patterns = frozenset(cli_excludes) if cli_excludes else frozenset()
pre_excluded = cli_patterns | cgrignore.exclude

Expand Down
8 changes: 4 additions & 4 deletions codebase_rag/tests/test_cgrignore.py
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ class TestCgrignoreLoadedWithoutInteractiveSetup:

@patch("codebase_rag.cli.GraphUpdater")
@patch("codebase_rag.cli.load_parsers", return_value=({}, {}))
@patch("codebase_rag.cli.load_cgrignore_patterns")
@patch("codebase_rag.cli.load_ignore_patterns")
def test_start_loads_cgrignore_without_interactive_setup(
self,
mock_load_cgrignore: MagicMock,
Expand Down Expand Up @@ -314,7 +314,7 @@ def test_start_loads_cgrignore_without_interactive_setup(
@patch("codebase_rag.cli.GraphUpdater")
@patch("codebase_rag.cli.load_parsers", return_value=({}, {}))
@patch("codebase_rag.cli.ProtobufFileIngestor")
@patch("codebase_rag.cli.load_cgrignore_patterns")
@patch("codebase_rag.cli.load_ignore_patterns")
def test_index_loads_cgrignore_without_interactive_setup(
self,
mock_load_cgrignore: MagicMock,
Expand Down Expand Up @@ -344,7 +344,7 @@ def test_index_loads_cgrignore_without_interactive_setup(

@patch("codebase_rag.cli.GraphUpdater")
@patch("codebase_rag.cli.load_parsers", return_value=({}, {}))
@patch("codebase_rag.cli.load_cgrignore_patterns")
@patch("codebase_rag.cli.load_ignore_patterns")
def test_start_merges_cli_excludes_with_cgrignore(
self,
mock_load_cgrignore: MagicMock,
Expand Down Expand Up @@ -379,7 +379,7 @@ def test_start_merges_cli_excludes_with_cgrignore(
@patch("codebase_rag.cli.prompt_for_unignored_directories")
@patch("codebase_rag.cli.GraphUpdater")
@patch("codebase_rag.cli.load_parsers", return_value=({}, {}))
@patch("codebase_rag.cli.load_cgrignore_patterns")
@patch("codebase_rag.cli.load_ignore_patterns")
def test_start_does_not_prompt_without_interactive_setup(
self,
mock_load_cgrignore: MagicMock,
Expand Down
6 changes: 3 additions & 3 deletions codebase_rag/tests/test_cli_clean.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ def test_clean_alone_shows_clean_done_message(
class TestCleanWithUpdateGraph:
@patch("codebase_rag.cli.GraphUpdater")
@patch("codebase_rag.cli.load_parsers", return_value=({}, {}))
@patch("codebase_rag.cli.load_cgrignore_patterns")
@patch("codebase_rag.cli.load_ignore_patterns")
def test_clean_with_update_deletes_hash_cache(
self,
mock_cgrignore: MagicMock,
Expand All @@ -145,7 +145,7 @@ def test_clean_with_update_deletes_hash_cache(

@patch("codebase_rag.cli.GraphUpdater")
@patch("codebase_rag.cli.load_parsers", return_value=({}, {}))
@patch("codebase_rag.cli.load_cgrignore_patterns")
@patch("codebase_rag.cli.load_ignore_patterns")
def test_clean_with_update_calls_clean_database(
self,
mock_cgrignore: MagicMock,
Expand All @@ -169,7 +169,7 @@ def test_clean_with_update_calls_clean_database(

@patch("codebase_rag.cli.GraphUpdater")
@patch("codebase_rag.cli.load_parsers", return_value=({}, {}))
@patch("codebase_rag.cli.load_cgrignore_patterns")
@patch("codebase_rag.cli.load_ignore_patterns")
def test_update_without_clean_preserves_hash_cache(
self,
mock_cgrignore: MagicMock,
Expand Down
83 changes: 83 additions & 0 deletions codebase_rag/tests/test_gitignore_patterns.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
from __future__ import annotations

from pathlib import Path

from codebase_rag.config import (
CGRIGNORE_FILENAME,
GITIGNORE_FILENAME,
load_ignore_patterns,
)


def test_gitignore_excludes_are_loaded(tmp_path: Path) -> None:
# (H) A gitignored path is a build artifact or generated output; indexing it
# (H) pollutes the graph and the dead-code report (evals/results fixtures on
# (H) cgr's own repo), so root .gitignore patterns must merge into excludes.
(tmp_path / GITIGNORE_FILENAME).write_text("results/\n*.gen.py\n", encoding="utf-8")

result = load_ignore_patterns(tmp_path)

assert "results/" in result.exclude
assert "*.gen.py" in result.exclude


def test_gitignore_exact_negation_cancels_its_exclude(tmp_path: Path) -> None:
# (H) An exact-match `!pattern` negation cancels the same-string exclude at
# (H) load time; the runtime skip check gives excludes precedence over
# (H) unignores, so leaving both in place would keep the path skipped.
(tmp_path / GITIGNORE_FILENAME).write_text(
"dist/\n!dist/\nbuild/\n", encoding="utf-8"
)

result = load_ignore_patterns(tmp_path)

assert "dist/" not in result.exclude
assert "build/" in result.exclude


def test_gitignore_finer_negation_stays_an_unignore(tmp_path: Path) -> None:
# (H) A finer-grained negation (`!dist/keep.py` under an excluded `dist/`)
# (H) cannot cancel by string match; it flows to unignore, where it rescues
# (H) from built-in ignores only (documented ceiling in config.py).
(tmp_path / GITIGNORE_FILENAME).write_text(
"dist/\n!dist/keep.py\n", encoding="utf-8"
)

result = load_ignore_patterns(tmp_path)

assert "dist/" in result.exclude
assert "dist/keep.py" in result.unignore


def test_cgrignore_unignore_overrides_gitignore_exclude(tmp_path: Path) -> None:
# (H) .cgrignore stays authoritative for cgr-specific choices; a `!pattern`
# (H) there re-includes something .gitignore excludes (the escape hatch for
# (H) indexing generated code on purpose). The cancellation must happen at
# (H) load time or the runtime exclude-first precedence keeps it skipped.
(tmp_path / GITIGNORE_FILENAME).write_text("generated/\n", encoding="utf-8")
(tmp_path / CGRIGNORE_FILENAME).write_text(
"vendor_src\n!generated/\n", encoding="utf-8"
)

result = load_ignore_patterns(tmp_path)

assert "generated/" not in result.exclude
assert "vendor_src" in result.exclude


def test_cgrignore_exclude_wins_over_gitignore_negation(tmp_path: Path) -> None:
# (H) The override channel is one-directional: an explicit .cgrignore exclude
# (H) is authoritative and a .gitignore negation must not cancel it.
(tmp_path / GITIGNORE_FILENAME).write_text("!generated/\n", encoding="utf-8")
(tmp_path / CGRIGNORE_FILENAME).write_text("generated/\n", encoding="utf-8")

result = load_ignore_patterns(tmp_path)

assert "generated/" in result.exclude


def test_no_ignore_files_yields_empty(tmp_path: Path) -> None:
result = load_ignore_patterns(tmp_path)

assert result.exclude == frozenset()
assert result.unignore == frozenset()
Loading