Skip to content

Release v3: advanced context engine, direct replacements, and CLI modernization#2

Merged
dsk-dev-ai merged 1 commit into
mainfrom
codex/release-version-3-with-advanced-features
Apr 12, 2026
Merged

Release v3: advanced context engine, direct replacements, and CLI modernization#2
dsk-dev-ai merged 1 commit into
mainfrom
codex/release-version-3-with-advanced-features

Conversation

@dsk-dev-ai

@dsk-dev-ai dsk-dev-ai commented Apr 12, 2026

Copy link
Copy Markdown
Owner

Motivation

  • Replace the legacy, ad-hoc context builder with a more advanced, maintainable v3 pipeline that provides better file ranking and clearer prompts for LLM analysis.
  • Expose richer explain-time controls and metadata from the engine so callers can tune context size and see which files were used.
  • Improve robustness of file parsing and CLI ergonomics for v3 release readiness.

Description

  • Introduced a new core/v3 module with context_builder.py (keyword-weighted scoring, BuildOptions, rank_files, score_file, and build_context) and prompting.py (stricter, v3 prompt template) and exported them via core/v3/__init__.py.
  • Replaced legacy internals by refactoring core/engine.py into compatibility wrappers that route to v3 via BuildOptions and added build_context_with_files to return selected files alongside context.
  • Reworked core/parser.py to deterministic traversal and safer file reading with configurable ignore/extension sets and support for additional extensions like ".rs" and ".md".
  • Modernized the CLI by replacing manual argv handling with argparse in cli/main.py, adding --max-files, --max-chars, --include-tests, and --json flags, and exposing selected file metadata in explain output.
  • Added documentation and release artifacts including README.md updates, top-level CHANGELOG.md, and VERSION/3.0.0/CHANGELOG.md.
  • Added unit tests under tests/test_v3_context.py covering scoring and test-file inclusion behavior.

Testing

  • Ran pytest -q after adding a test-path bootstrap and the new tests, resulting in 3 passed for tests/test_v3_context.py.
  • Initial pytest collection failed until the test import path was adjusted, after which the suite passed successfully.
  • Invoked python3 -m cli.main --help locally which surfaced a missing runtime dependency (python-dotenv) in the environment, so the CLI help run failed in this environment but is expected to work once dependencies from requirements.txt are installed.

Codex Task

Summary by Sourcery

Introduce a v3 context engine and modernized CLI that replace the legacy context builder and argument handling while improving parser robustness and documentation for the 3.0.0 release.

New Features:

  • Add core v3 module with weighted file scoring, configurable build options, and stricter prompt templates for explain workflows.
  • Expose explain-time controls for max files, max characters, and test-file inclusion via the engine and CLI.
  • Add JSON explain output that includes the LLM result and selected file metadata.

Enhancements:

  • Refactor core engine to act as a compatibility layer that delegates to the v3 context and prompt builders and can return selected files with the context.
  • Improve parser traversal determinism, file type coverage, and error handling when reading code files.
  • Streamline and modernize the README with v3-focused messaging, usage examples, and roadmap updates.
  • Document release history with root and versioned changelogs for v3.0.0.

Tests:

  • Add unit tests for the v3 context builder covering file scoring and optional inclusion of test files in ranking.

@sourcery-ai

sourcery-ai Bot commented Apr 12, 2026

Copy link
Copy Markdown

Reviewer's Guide

Releases v3.0.0 by introducing a new core/v3 context engine and prompt template, routing the legacy engine through typed v3 builders, modernizing the CLI with argparse and richer explain controls/metadata, hardening file parsing, and adding tests and changelog/docs for the new release.

Sequence diagram for v3 explain command pipeline

sequenceDiagram
  actor Developer
  participant CLI as DevOSCLI_main
  participant Parser as build_parser
  participant Engine as EngineCompatibility
  participant V3 as V3ContextBuilder
  participant FS as FileSystem
  participant Prompt as V3Prompting
  participant LLM as LLMProvider

  Developer->>CLI: invoke cli.main with args (explain, target, flags)
  CLI->>Parser: build_parser()
  Parser-->>CLI: ArgumentParser
  CLI->>CLI: parse_args()

  CLI->>Engine: build_context_with_files(target, max_files, max_chars, include_tests)
  Engine->>V3: build_context(repo_path, BuildOptions)
  V3->>FS: get_code_files(repo_path)
  FS-->>V3: list~str~ code_files
  V3->>FS: read_files(selected_files, max_chars)
  FS-->>V3: str context
  V3-->>Engine: context, selected_files
  Engine-->>CLI: context, selected_files

  CLI->>Prompt: build_prompt(context, "Explain this codebase", selected_files)
  Prompt-->>CLI: prompt

  CLI->>LLM: ask_llm(prompt, model)
  LLM-->>CLI: result

  alt json output flag enabled
    CLI-->>Developer: JSON { selected_files, result }
  else plain output
    CLI-->>Developer: human-readable result + selected_files list
  end
Loading

Class diagram for v3 context engine, CLI, and compatibility wrappers

classDiagram

class BuildOptions {
  +int max_files = 8
  +int max_chars = 1400
  +bool include_tests = False
}

class V3ContextBuilder {
  <<module>>
  +int score_file(path: str)
  +list~str~ rank_files(files: list~str~, include_tests: bool)
  +tuple~str, list~str~~ build_context(repo_path: str, options: BuildOptions)
}

class V3Prompting {
  <<module>>
  +str build_prompt(context: str, question: str, selected_files: list~str~)
}

class Parser {
  +list~str~ get_code_files(repo_path: str, ignore_dirs: set~str~, extensions: tuple~str~)
  +str read_files(files: list~str~, max_chars: int)
}

class EngineCompatibility {
  +str build_context(repo_path: str, max_files: int, max_chars: int, include_tests: bool)
  +tuple~str, list~str~~ build_context_with_files(repo_path: str, max_files: int, max_chars: int, include_tests: bool)
  +str build_prompt(context: str, question: str, selected_files: list~str~)
}

class DevOSCLI {
  +argparse_ArgumentParser build_parser()
  +None main()
  +tuple~str, list~str~~ explain(path: str, model: str, max_files: int, max_chars: int, include_tests: bool)
}

class LLMProvider {
  +str ask_llm(prompt: str, model: str)
}

class SearchAgent {
  +str search_code(path: str, query: str, model: str)
}

class DebugAgent {
  +str debug_error(file_path: str, model: str)
}

BuildOptions <-- V3ContextBuilder : uses
Parser <-- V3ContextBuilder : uses
V3Prompting <-- EngineCompatibility : uses
V3ContextBuilder <-- EngineCompatibility : uses
EngineCompatibility <-- DevOSCLI : uses
LLMProvider <-- DevOSCLI : uses
SearchAgent <-- DevOSCLI : uses
DebugAgent <-- DevOSCLI : uses
Loading

File-Level Changes

Change Details Files
Replaced legacy context engine internals with the new v3 pipeline and stricter prompt template while preserving public APIs.
  • Added core/v3/context_builder.py with BuildOptions, keyword/file-type scoring, rank_files, and build_context that returns context plus selected files.
  • Added core/v3/prompting.py with a v3 build_prompt that includes selected file listing and explicit facts-vs-inference guidance.
  • Refactored core/engine.py so build_context and build_prompt become thin compatibility wrappers that delegate to v3, and added build_context_with_files to expose selected files to callers.
  • Exported v3 symbols via core/v3/init.py for centralized imports.
core/v3/context_builder.py
core/v3/prompting.py
core/v3/__init__.py
core/engine.py
Improved file discovery and reading determinism and safety for context building.
  • Replaced ad-hoc os.walk usage with a deterministic _iter_code_files helper that sorts directories/files and uses Path objects.
  • Introduced DEFAULT_IGNORE_DIRS/DEFAULT_EXTENSIONS, expanded extension coverage (including .rs and .md), and made ignore/extension sets configurable.
  • Hardened read_files with explicit typing, higher default max_chars, and narrowed exception handling to filesystem/encoding errors.
core/parser.py
Modernized the CLI with argparse and extended explain-time controls and metadata output.
  • Replaced manual sys.argv parsing with build_parser using argparse, defining subcommands (explain/search/debug), shared options, and help text.
  • Updated explain flow to call build_context_with_files, pass selected files into build_prompt, and return both LLM result and file list.
  • Added CLI flags --max-files, --max-chars, --include-tests, and --json; when --json is set, explain outputs a JSON object containing selected_files and result, and non-JSON mode prints a file list footer.
  • Adjusted search and debug commands to use the new parser argument structure and consistent model handling.
cli/main.py
Documented the v3.0.0 release and updated high-level project docs for the new engine and CLI.
  • Rewrote README.md to focus on v3, summarizing the advanced ranking, new CLI flags, and updated project structure/usage examples.
  • Added a top-level CHANGELOG.md describing v3.0.0 features, replacements, and improvements, and backfilled a v2.0.0 entry.
  • Added VERSION/3.0.0/CHANGELOG.md with v3-specific highlights and technical notes about compatibility wrappers and scoring/prompting changes.
README.md
CHANGELOG.md
VERSION/3.0.0/CHANGELOG.md
Added initial unit tests for the v3 context scoring behavior.
  • Created tests/test_v3_context.py to validate score_file prioritization of core engine files and rank_files test-exclusion/default behavior.
  • Bootstrapped test import paths by appending the repository root to sys.path for direct core.v3 imports.
tests/test_v3_context.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@dsk-dev-ai dsk-dev-ai added the enhancement New feature or request label Apr 12, 2026

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - I've found 5 issues, and left some high level feedback:

  • The new type hints like list[str] and unions via | in core/engine.py, core/parser.py, and core/v3/* require Python 3.10+, so if you still intend to support 3.9 you may want to switch to List[str]/Optional[...] from typing instead.
  • In core/parser.get_code_files, using ignore_dirs or DEFAULT_IGNORE_DIRS and extensions or DEFAULT_EXTENSIONS makes it impossible to intentionally pass an empty set/tuple (they’ll be replaced by defaults); consider explicitly checking for None to distinguish 'no override' from 'empty override'.
  • The CLI argument model uses a single command positional plus a generic query argument, which leads to manual validation for search only; using argparse subparsers with per-command required/optional arguments would give clearer help messages and remove the need for the explicit if not args.query check.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The new type hints like `list[str]` and unions via `|` in `core/engine.py`, `core/parser.py`, and `core/v3/*` require Python 3.10+, so if you still intend to support 3.9 you may want to switch to `List[str]`/`Optional[...]` from `typing` instead.
- In `core/parser.get_code_files`, using `ignore_dirs or DEFAULT_IGNORE_DIRS` and `extensions or DEFAULT_EXTENSIONS` makes it impossible to intentionally pass an empty set/tuple (they’ll be replaced by defaults); consider explicitly checking for `None` to distinguish 'no override' from 'empty override'.
- The CLI argument model uses a single `command` positional plus a generic `query` argument, which leads to manual validation for `search` only; using `argparse` subparsers with per-command required/optional arguments would give clearer help messages and remove the need for the explicit `if not args.query` check.

## Individual Comments

### Comment 1
<location path="core/parser.py" line_range="28-35" />
<code_context>

-    return code_files

+def get_code_files(
+    repo_path: str,
+    ignore_dirs: set[str] | None = None,
+    extensions: tuple[str, ...] | None = None,
+) -> list[str]:
+    ignore = ignore_dirs or DEFAULT_IGNORE_DIRS
+    exts = extensions or DEFAULT_EXTENSIONS
+    return [str(path) for path in _iter_code_files(repo_path, ignore, exts)]

-def read_files(files, max_chars=800):
</code_context>
<issue_to_address>
**suggestion:** Passing an empty ignore_dirs or extensions is ineffective because falsy values fall back to the defaults.

Because `ignore = ignore_dirs or DEFAULT_IGNORE_DIRS` and `exts = extensions or DEFAULT_EXTENSIONS` treat empty sets/tuples as falsy, callers cannot specify “no ignored dirs” or an empty extension list. If you want to support that, use an explicit `None` check instead:

```python
def get_code_files(...):
    ignore = DEFAULT_IGNORE_DIRS if ignore_dirs is None else ignore_dirs
    exts = DEFAULT_EXTENSIONS if extensions is None else extensions
```

```suggestion
def get_code_files(
    repo_path: str,
    ignore_dirs: set[str] | None = None,
    extensions: tuple[str, ...] | None = None,
) -> list[str]:
    ignore = DEFAULT_IGNORE_DIRS if ignore_dirs is None else ignore_dirs
    exts = DEFAULT_EXTENSIONS if extensions is None else extensions
    return [str(path) for path in _iter_code_files(repo_path, ignore, exts)]
```
</issue_to_address>

### Comment 2
<location path="core/v3/context_builder.py" line_range="48-57" />
<code_context>
+    return score
+
+
+def rank_files(files: list[str], include_tests: bool = False) -> list[str]:
+    ranked: list[tuple[int, str]] = []
+
+    for file in files:
+        if not include_tests and "test" in file.lower():
+            continue
+        ranked.append((score_file(file), file))
+
+    ranked.sort(key=lambda item: (item[0], item[1]), reverse=True)
+    return [file for _, file in ranked]
+
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Filtering tests by a plain 'test' substring can unintentionally exclude non-test files.

The current `if not include_tests and "test" in file.lower():` condition will also skip non-test files like `latest_config.py` or `contest_utils.py`. To better distinguish tests, consider matching typical test naming patterns (e.g., `Path(file).name.startswith("test_")`, `Path(file).name.endswith("_test.py")`) and/or checking for a `tests` directory in the path, so relevant non-test files are not excluded from ranking.

```suggestion
def rank_files(files: list[str], include_tests: bool = False) -> list[str]:
    ranked: list[tuple[int, str]] = []

    for file in files:
        path = Path(file)
        name = path.name.lower()
        parts = [part.lower() for part in path.parts]

        is_test = (
            name.startswith("test_")
            or name.endswith("_test.py")
            or any(part == "tests" for part in parts)
        )

        if not include_tests and is_test:
            continue

        ranked.append((score_file(file), file))

    ranked.sort(key=lambda item: (item[0], item[1]), reverse=True)
    return [file for _, file in ranked]
```
</issue_to_address>

### Comment 3
<location path="cli/main.py" line_range="23-30" />
<code_context>
-    model = "auto"
+def build_parser() -> argparse.ArgumentParser:
+    parser = argparse.ArgumentParser(description="DevOS AI CLI v3")
+    parser.add_argument("command", choices=["explain", "search", "debug"], help="Command to run")
+    parser.add_argument("target", help="Repository path for explain/search, file path for debug")
+    parser.add_argument("query", nargs="?", default=None, help="Search query")
+    parser.add_argument("--model", default="auto", choices=["openrouter", "google", "auto"])
+    parser.add_argument("--max-files", type=int, default=8)
+    parser.add_argument("--max-chars", type=int, default=1400)
+    parser.add_argument("--include-tests", action="store_true")
+    parser.add_argument("--json", action="store_true", dest="as_json")
+    return parser

</code_context>
<issue_to_address>
**suggestion:** The --json flag is only honored for the 'explain' command, which may surprise users.

`--json` is accepted for all commands, but only `explain` actually uses `args.as_json`; `search` and `debug` ignore it and still emit plain text.

To make this less surprising, consider either limiting `--json` to the `explain` subcommand or surfacing an error/notice when it’s used with unsupported commands. Alternatively, you could add JSON output support to `search`/`debug` for consistency.

Suggested implementation:

```python
import argparse

from agents.debug_agent import debug_error
from agents.search_agent import search_code
from core.engine import build_context_with_files, build_prompt
from llm.provider import ask_llm


def build_parser() -> argparse.ArgumentParser:
    """
    Build the CLI argument parser.

    Notes:
        --json is only supported for the 'explain' command. Using it with
        other commands will result in a validation error (see validate_args).
    """
    parser = argparse.ArgumentParser(description="DevOS AI CLI v3")
    parser.add_argument(
        "command",
        choices=["explain", "search", "debug"],
        help="Command to run",
    )
    parser.add_argument(
        "target",
        help="Repository path for explain/search, file path for debug",
    )
    parser.add_argument(
        "query",
        nargs="?",
        default=None,
        help="Search query (required for 'search', optional for 'explain')",
    )
    parser.add_argument(
        "--model",
        default="auto",
        choices=["openrouter", "google", "auto"],
        help="LLM provider to use (default: auto)",
    )
    parser.add_argument(
        "--max-files",
        type=int,
        default=8,
        help="Maximum number of files to include in the context (default: 8)",
    )
    parser.add_argument(
        "--max-chars",
        type=int,
        default=1400,
        help="Maximum number of characters per file to include (default: 1400)",
    )
    parser.add_argument(
        "--include-tests",
        action="store_true",
        help="Include test files when building the context",
    )
    # NOTE: This flag is only honored for the 'explain' command.
    parser.add_argument(
        "--json",
        action="store_true",
        dest="as_json",
        help="Output JSON (only supported for the 'explain' command)",
    )
    return parser


def validate_args(args: argparse.Namespace) -> None:
    """
    Validate parsed CLI arguments.

    Currently ensures that --json is only used with the 'explain' command.
    """
    if getattr(args, "as_json", False) and getattr(args, "command", None) != "explain":
        raise SystemExit(
            "--json output is only supported for the 'explain' command; "
            f"got --json with '{args.command}'."
        )


```

To fully implement the behavior:

1. Ensure that wherever `build_parser()` is used, `validate_args()` is called immediately after parsing:
   - If you have a `main()` like:
     ```python
     def main(argv: list[str] | None = None) -> None:
         parser = build_parser()
         args = parser.parse_args(argv)
         ...
     ```
     update it to:
     ```python
     def main(argv: list[str] | None = None) -> None:
         parser = build_parser()
         args = parser.parse_args(argv)
         validate_args(args)
         ...
     ```
2. If you have any unit tests that invoke the CLI with `--json` for `search` or `debug`, update them to either expect a `SystemExit` or remove `--json` for those commands.
3. If you surface CLI errors in a custom way (e.g., catching `SystemExit` and formatting), ensure that this new `SystemExit` raised by `validate_args` is handled consistently with other CLI errors.
</issue_to_address>

### Comment 4
<location path="tests/test_v3_context.py" line_range="1-6" />
<code_context>
+import sys
+from pathlib import Path
+
+sys.path.append(str(Path(__file__).resolve().parents[1]))
+
+from core.v3.context_builder import rank_files, score_file
</code_context>
<issue_to_address>
**suggestion:** Avoid mutating sys.path in tests; prefer package-relative imports or pytest configuration

Modifying `sys.path` in tests tends to create brittle, order-dependent imports. Instead, make the project importable as a package (so you can import `core.v3.context_builder` directly) or configure the test environment (e.g., `PYTHONPATH` / `pytest.ini` with `pythonpath = .`) to avoid manual `sys.path` manipulation.

```suggestion
from core.v3.context_builder import rank_files, score_file
```
</issue_to_address>

### Comment 5
<location path="tests/test_v3_context.py" line_range="9-10" />
<code_context>
+from core.v3.context_builder import rank_files, score_file
+
+
+def test_score_file_prioritizes_engine_and_python():
+    assert score_file("/tmp/core/engine.py") > score_file("/tmp/docs/readme.md")
+
+
</code_context>
<issue_to_address>
**suggestion (testing):** Broaden score_file tests to cover multiple keywords, extensions, and non-matching cases

The existing test only covers a single comparison and doesn’t meaningfully exercise `score_file`’s weighting logic. Please add parametrized cases to cover:
- Extension-based scoring (e.g., `engine.py` vs `engine.js` vs `engine.md`).
- Different keyword-containing paths (`router`, `agent`, `provider`, `config`) so each weight is verified.
- A file with no keywords and a non-preferred extension to confirm it scores below keyword-heavy files.
This will better lock in the v3 ranking behavior and make future refactors safer.

Suggested implementation:

```python
import sys
from pathlib import Path

import pytest

sys.path.append(str(Path(__file__).resolve().parents[1]))

from core.v3.context_builder import rank_files, score_file

```

```python
from core.v3.context_builder import rank_files, score_file


def test_score_file_prioritizes_engine_and_python():
    assert score_file("/tmp/core/engine.py") > score_file("/tmp/docs/readme.md")


@pytest.mark.parametrize(
    "better, worse",
    [
        ("/tmp/core/engine.py", "/tmp/core/engine.js"),
        ("/tmp/core/engine.js", "/tmp/core/engine.md"),
    ],
)
def test_score_file_extension_weights(better: str, worse: str):
    """Extension-based scoring: prefer Python over JS, and JS over Markdown."""
    assert score_file(better) > score_file(worse)


@pytest.mark.parametrize(
    "keyword_segment",
    [
        "router",
        "agent",
        "provider",
        "config",
    ],
)
def test_score_file_prioritizes_keyword_paths(keyword_segment: str):
    """Keyword-containing paths should outrank neutral ones with the same extension."""
    keyword_path = f"/tmp/core/{keyword_segment}.py"
    neutral_path = "/tmp/core/utility.py"

    assert score_file(keyword_path) > score_file(neutral_path)


def test_score_file_demotes_non_matching_non_preferred_extension():
    """Non-keyword, non-preferred extension should score below keyword-heavy Python files."""
    keyword_heavy = "/tmp/core/agent_router_provider_config.py"
    non_matching = "/tmp/misc/notes.txt"

    assert score_file(keyword_heavy) > score_file(non_matching)


def test_rank_files_excludes_tests_by_default():

```
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread core/parser.py
Comment on lines +28 to +35
def get_code_files(
repo_path: str,
ignore_dirs: set[str] | None = None,
extensions: tuple[str, ...] | None = None,
) -> list[str]:
ignore = ignore_dirs or DEFAULT_IGNORE_DIRS
exts = extensions or DEFAULT_EXTENSIONS
return [str(path) for path in _iter_code_files(repo_path, ignore, exts)]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

suggestion: Passing an empty ignore_dirs or extensions is ineffective because falsy values fall back to the defaults.

Because ignore = ignore_dirs or DEFAULT_IGNORE_DIRS and exts = extensions or DEFAULT_EXTENSIONS treat empty sets/tuples as falsy, callers cannot specify “no ignored dirs” or an empty extension list. If you want to support that, use an explicit None check instead:

def get_code_files(...):
    ignore = DEFAULT_IGNORE_DIRS if ignore_dirs is None else ignore_dirs
    exts = DEFAULT_EXTENSIONS if extensions is None else extensions
Suggested change
def get_code_files(
repo_path: str,
ignore_dirs: set[str] | None = None,
extensions: tuple[str, ...] | None = None,
) -> list[str]:
ignore = ignore_dirs or DEFAULT_IGNORE_DIRS
exts = extensions or DEFAULT_EXTENSIONS
return [str(path) for path in _iter_code_files(repo_path, ignore, exts)]
def get_code_files(
repo_path: str,
ignore_dirs: set[str] | None = None,
extensions: tuple[str, ...] | None = None,
) -> list[str]:
ignore = DEFAULT_IGNORE_DIRS if ignore_dirs is None else ignore_dirs
exts = DEFAULT_EXTENSIONS if extensions is None else extensions
return [str(path) for path in _iter_code_files(repo_path, ignore, exts)]

Comment on lines +48 to +57
def rank_files(files: list[str], include_tests: bool = False) -> list[str]:
ranked: list[tuple[int, str]] = []

for file in files:
if not include_tests and "test" in file.lower():
continue
ranked.append((score_file(file), file))

ranked.sort(key=lambda item: (item[0], item[1]), reverse=True)
return [file for _, file in ranked]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

suggestion (bug_risk): Filtering tests by a plain 'test' substring can unintentionally exclude non-test files.

The current if not include_tests and "test" in file.lower(): condition will also skip non-test files like latest_config.py or contest_utils.py. To better distinguish tests, consider matching typical test naming patterns (e.g., Path(file).name.startswith("test_"), Path(file).name.endswith("_test.py")) and/or checking for a tests directory in the path, so relevant non-test files are not excluded from ranking.

Suggested change
def rank_files(files: list[str], include_tests: bool = False) -> list[str]:
ranked: list[tuple[int, str]] = []
for file in files:
if not include_tests and "test" in file.lower():
continue
ranked.append((score_file(file), file))
ranked.sort(key=lambda item: (item[0], item[1]), reverse=True)
return [file for _, file in ranked]
def rank_files(files: list[str], include_tests: bool = False) -> list[str]:
ranked: list[tuple[int, str]] = []
for file in files:
path = Path(file)
name = path.name.lower()
parts = [part.lower() for part in path.parts]
is_test = (
name.startswith("test_")
or name.endswith("_test.py")
or any(part == "tests" for part in parts)
)
if not include_tests and is_test:
continue
ranked.append((score_file(file), file))
ranked.sort(key=lambda item: (item[0], item[1]), reverse=True)
return [file for _, file in ranked]

Comment thread cli/main.py
Comment on lines +23 to +30
parser.add_argument("command", choices=["explain", "search", "debug"], help="Command to run")
parser.add_argument("target", help="Repository path for explain/search, file path for debug")
parser.add_argument("query", nargs="?", default=None, help="Search query")
parser.add_argument("--model", default="auto", choices=["openrouter", "google", "auto"])
parser.add_argument("--max-files", type=int, default=8)
parser.add_argument("--max-chars", type=int, default=1400)
parser.add_argument("--include-tests", action="store_true")
parser.add_argument("--json", action="store_true", dest="as_json")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

suggestion: The --json flag is only honored for the 'explain' command, which may surprise users.

--json is accepted for all commands, but only explain actually uses args.as_json; search and debug ignore it and still emit plain text.

To make this less surprising, consider either limiting --json to the explain subcommand or surfacing an error/notice when it’s used with unsupported commands. Alternatively, you could add JSON output support to search/debug for consistency.

Suggested implementation:

import argparse

from agents.debug_agent import debug_error
from agents.search_agent import search_code
from core.engine import build_context_with_files, build_prompt
from llm.provider import ask_llm


def build_parser() -> argparse.ArgumentParser:
    """
    Build the CLI argument parser.

    Notes:
        --json is only supported for the 'explain' command. Using it with
        other commands will result in a validation error (see validate_args).
    """
    parser = argparse.ArgumentParser(description="DevOS AI CLI v3")
    parser.add_argument(
        "command",
        choices=["explain", "search", "debug"],
        help="Command to run",
    )
    parser.add_argument(
        "target",
        help="Repository path for explain/search, file path for debug",
    )
    parser.add_argument(
        "query",
        nargs="?",
        default=None,
        help="Search query (required for 'search', optional for 'explain')",
    )
    parser.add_argument(
        "--model",
        default="auto",
        choices=["openrouter", "google", "auto"],
        help="LLM provider to use (default: auto)",
    )
    parser.add_argument(
        "--max-files",
        type=int,
        default=8,
        help="Maximum number of files to include in the context (default: 8)",
    )
    parser.add_argument(
        "--max-chars",
        type=int,
        default=1400,
        help="Maximum number of characters per file to include (default: 1400)",
    )
    parser.add_argument(
        "--include-tests",
        action="store_true",
        help="Include test files when building the context",
    )
    # NOTE: This flag is only honored for the 'explain' command.
    parser.add_argument(
        "--json",
        action="store_true",
        dest="as_json",
        help="Output JSON (only supported for the 'explain' command)",
    )
    return parser


def validate_args(args: argparse.Namespace) -> None:
    """
    Validate parsed CLI arguments.

    Currently ensures that --json is only used with the 'explain' command.
    """
    if getattr(args, "as_json", False) and getattr(args, "command", None) != "explain":
        raise SystemExit(
            "--json output is only supported for the 'explain' command; "
            f"got --json with '{args.command}'."
        )

To fully implement the behavior:

  1. Ensure that wherever build_parser() is used, validate_args() is called immediately after parsing:
    • If you have a main() like:
      def main(argv: list[str] | None = None) -> None:
          parser = build_parser()
          args = parser.parse_args(argv)
          ...
      update it to:
      def main(argv: list[str] | None = None) -> None:
          parser = build_parser()
          args = parser.parse_args(argv)
          validate_args(args)
          ...
  2. If you have any unit tests that invoke the CLI with --json for search or debug, update them to either expect a SystemExit or remove --json for those commands.
  3. If you surface CLI errors in a custom way (e.g., catching SystemExit and formatting), ensure that this new SystemExit raised by validate_args is handled consistently with other CLI errors.

Comment thread tests/test_v3_context.py
Comment on lines +1 to +6
import sys
from pathlib import Path

sys.path.append(str(Path(__file__).resolve().parents[1]))

from core.v3.context_builder import rank_files, score_file

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

suggestion: Avoid mutating sys.path in tests; prefer package-relative imports or pytest configuration

Modifying sys.path in tests tends to create brittle, order-dependent imports. Instead, make the project importable as a package (so you can import core.v3.context_builder directly) or configure the test environment (e.g., PYTHONPATH / pytest.ini with pythonpath = .) to avoid manual sys.path manipulation.

Suggested change
import sys
from pathlib import Path
sys.path.append(str(Path(__file__).resolve().parents[1]))
from core.v3.context_builder import rank_files, score_file
from core.v3.context_builder import rank_files, score_file

Comment thread tests/test_v3_context.py
Comment on lines +9 to +10
def test_score_file_prioritizes_engine_and_python():
assert score_file("/tmp/core/engine.py") > score_file("/tmp/docs/readme.md")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

suggestion (testing): Broaden score_file tests to cover multiple keywords, extensions, and non-matching cases

The existing test only covers a single comparison and doesn’t meaningfully exercise score_file’s weighting logic. Please add parametrized cases to cover:

  • Extension-based scoring (e.g., engine.py vs engine.js vs engine.md).
  • Different keyword-containing paths (router, agent, provider, config) so each weight is verified.
  • A file with no keywords and a non-preferred extension to confirm it scores below keyword-heavy files.
    This will better lock in the v3 ranking behavior and make future refactors safer.

Suggested implementation:

import sys
from pathlib import Path

import pytest

sys.path.append(str(Path(__file__).resolve().parents[1]))

from core.v3.context_builder import rank_files, score_file
from core.v3.context_builder import rank_files, score_file


def test_score_file_prioritizes_engine_and_python():
    assert score_file("/tmp/core/engine.py") > score_file("/tmp/docs/readme.md")


@pytest.mark.parametrize(
    "better, worse",
    [
        ("/tmp/core/engine.py", "/tmp/core/engine.js"),
        ("/tmp/core/engine.js", "/tmp/core/engine.md"),
    ],
)
def test_score_file_extension_weights(better: str, worse: str):
    """Extension-based scoring: prefer Python over JS, and JS over Markdown."""
    assert score_file(better) > score_file(worse)


@pytest.mark.parametrize(
    "keyword_segment",
    [
        "router",
        "agent",
        "provider",
        "config",
    ],
)
def test_score_file_prioritizes_keyword_paths(keyword_segment: str):
    """Keyword-containing paths should outrank neutral ones with the same extension."""
    keyword_path = f"/tmp/core/{keyword_segment}.py"
    neutral_path = "/tmp/core/utility.py"

    assert score_file(keyword_path) > score_file(neutral_path)


def test_score_file_demotes_non_matching_non_preferred_extension():
    """Non-keyword, non-preferred extension should score below keyword-heavy Python files."""
    keyword_heavy = "/tmp/core/agent_router_provider_config.py"
    non_matching = "/tmp/misc/notes.txt"

    assert score_file(keyword_heavy) > score_file(non_matching)


def test_rank_files_excludes_tests_by_default():

@dsk-dev-ai
dsk-dev-ai merged commit ae16c9d into main Apr 12, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

codex enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant