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
13 changes: 8 additions & 5 deletions .github/workflows/claude.yml
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ jobs:
with:
claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
github_token: ${{ secrets.GITHUB_TOKEN }}
allowed_non_write_users: Copilot
allowed_bots: "github-actions[bot],copilot[bot],dependabot[bot],copilot,github-actions,gemini[bot],claude[bot]"
allowed_non_write_users: Copilot,copilot,jules[bot],jules
allowed_bots: "github-actions[bot],copilot[bot],dependabot[bot],copilot,github-actions,gemini[bot],claude[bot],jules[bot]"
trigger_phrase: "@claude"
assignee_trigger: claude[bot]
label_trigger: claude
Expand Down Expand Up @@ -105,7 +105,8 @@ jobs:
with:
claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
github_token: ${{ secrets.GITHUB_TOKEN }}
allowed_bots: Copilot
allowed_non_write_users: Copilot,copilot,jules[bot],jules
allowed_bots: "github-actions[bot],copilot[bot],dependabot[bot],copilot,github-actions,gemini[bot],claude[bot],jules[bot]"
trigger_phrase: "@claude"
assignee_trigger: claude
label_trigger: claude
Expand Down Expand Up @@ -141,7 +142,8 @@ jobs:
with:
claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
github_token: ${{ secrets.GITHUB_TOKEN }}
allowed_bots: Copilot
allowed_non_write_users: Copilot,copilot,jules[bot],jules
allowed_bots: "github-actions[bot],copilot[bot],dependabot[bot],copilot,github-actions,gemini[bot],claude[bot],jules[bot]"
trigger_phrase: "@claude"
assignee_trigger: claude
label_trigger: claude
Expand Down Expand Up @@ -179,7 +181,8 @@ jobs:
with:
claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
github_token: ${{ secrets.GITHUB_TOKEN }}
allowed_bots: Copilot
allowed_non_write_users: Copilot,copilot,jules[bot],jules
allowed_bots: "github-actions[bot],copilot[bot],dependabot[bot],copilot,github-actions,gemini[bot],claude[bot],jules[bot]"
trigger_phrase: "@claude"
assignee_trigger: claude
label_trigger: claude
Expand Down
30 changes: 24 additions & 6 deletions src/codeweaver/core/utils/generation.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,21 @@


if sys.version_info < (3, 14):
from uuid_extensions import uuid7 as uuid7_gen
try:
from uuid_extensions import uuid7 as uuid7_gen
Comment on lines 21 to +23
except ImportError:
def uuid7_gen(*args, **kwargs) -> UUID7:

Check failure on line 25 in src/codeweaver/core/utils/generation.py

View workflow job for this annotation

GitHub Actions / Lint / Lint and Format

ruff (ANN002)

src/codeweaver/core/utils/generation.py:25:23: ANN002 Missing type annotation for `*args`
from pydantic import UUID7
from uuid import uuid4
return cast(UUID7, uuid4())
else:
Comment on lines +24 to 29
from uuid import uuid7 as uuid7_gen
try:
from uuid import uuid7 as uuid7_gen
except ImportError:
def uuid7_gen(*args, **kwargs) -> UUID7:
from pydantic import UUID7

Check failure on line 34 in src/codeweaver/core/utils/generation.py

View workflow job for this annotation

GitHub Actions / Lint / Lint and Format

ruff (ANN002)

src/codeweaver/core/utils/generation.py:34:23: ANN002 Missing type annotation for `*args`
from uuid import uuid4
return cast(UUID7, uuid4())
Comment on lines 21 to +36
Comment on lines 21 to +36
Comment on lines 21 to +36


def uuid7() -> UUID7:
Expand All @@ -44,10 +56,16 @@
) -> int | datetime.datetime | None:
"""Utility to extract the timestamp from a UUID7, optionally as a datetime."""
if sys.version_info < (3, 14):
from uuid_extensions import time_ns, uuid_to_datetime

return uuid_to_datetime(uuid) if as_datetime else time_ns(uuid)
from uuid import uuid7
try:
from uuid_extensions import time_ns, uuid_to_datetime

return uuid_to_datetime(uuid) if as_datetime else time_ns(uuid)
except ImportError:
return datetime.datetime.now(datetime.UTC) if as_datetime else int(datetime.datetime.now(datetime.UTC).timestamp() * 1e9)
try:
from uuid import uuid7
except ImportError:
return datetime.datetime.now(datetime.UTC) if as_datetime else int(datetime.datetime.now(datetime.UTC).timestamp() * 1e9)
Comment on lines 56 to +68
Comment on lines +64 to +68
Copy link
Contributor

Choose a reason for hiding this comment

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

Agree with Copilot's assessment - this is a critical issue in the UUID fallback logic.

Problem

The current fallback in uuid7_as_timestamp() returns datetime.datetime.now() when UUID7 support is unavailable. This is semantically incorrect because:

  1. Data corruption: The function claims to extract a timestamp from the UUID, but instead returns the current time, which is completely unrelated to the input
  2. Silent failures: Code that relies on UUID ordering or age calculations (like "oldest item in registry") will get incorrect results without any indication something is wrong
  3. Debugging nightmare: Since the fallback silently returns a timestamp, developers won't know their UUID7 timestamps are fabricated

Impact on This PR

Looking at generation.py:56-68, the fallback at line 64 is:

return datetime.datetime.now(datetime.UTC) if as_datetime else int(datetime.datetime.now(datetime.UTC).timestamp() * 1e9)

This means if uuid7 support is missing on Python 3.14t/3.13t, any code calling uuid7_as_timestamp(my_uuid) will get the current time instead of the time embedded in my_uuid.

Recommended Fix

Option 1 (Preferred): Raise a clear error

except ImportError:
    raise ImportError(
        "UUID7 timestamp extraction requires uuid_extensions (Python <3.14) or uuid7 support (Python >=3.14). "
        "Cannot extract timestamp from UUID7 on this Python version."
    )

Option 2: Return None (if callers can handle it)

except ImportError:
    return None  # Signal that timestamp extraction is unavailable

Then update the return type annotation from int | datetime.datetime | None to explicitly document that None means "extraction unavailable".


Verdict: This fallback should be removed or changed to raise an error. The current implementation is worse than failing fast because it corrupts data silently.


uuid = uuid7(uuid) if isinstance(uuid, str | int) else uuid
return (
Expand Down
14 changes: 10 additions & 4 deletions src/codeweaver/providers/config/clients/multi.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,20 @@
GoogleCredentials = Any

if has_package("fastembed") is not None or has_package("fastembed_gpu") is not None:
from fastembed.common.types import OnnxProvider
try:
from fastembed.common.types import OnnxProvider
except ImportError:
OnnxProvider = Any # type: ignore[assignment, misc]
else:
OnnxProvider = object
OnnxProvider = Any # type: ignore[assignment, misc]

if has_package("torch") is not None:
from torch.nn import Module
try:
from torch.nn import Module
except ImportError:
Module = Any # type: ignore[assignment, misc]
else:
Module = object
Module = Any # type: ignore[assignment, misc]
if has_package("sentence_transformers") is not None:
# SentenceTransformerModelCardData contains these forward references:
# - eval_results_dict: dict[SentenceEvaluator, dict[str, Any]] | None
Expand Down
88 changes: 88 additions & 0 deletions tests/unit/core/test_discovery.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
# SPDX-FileCopyrightText: 2026 Knitli Inc.
# SPDX-FileContributor: Adam Poulemanos <adam@knit.li>
#
# SPDX-License-Identifier: MIT OR Apache-2.0

"""Unit tests for core discovery logic."""

from pathlib import Path
from unittest.mock import patch

import pytest

from codeweaver.core.discovery import DiscoveredFile
from codeweaver.core.metadata import ExtCategory


pytestmark = [pytest.mark.unit]


@pytest.fixture
def temp_project(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path:
"""Fixture to provide a temporary project directory and set the environment variable."""
project_dir = tmp_path / "project"
project_dir.mkdir()
monkeypatch.setenv("CODEWEAVER_PROJECT_PATH", str(project_dir))
return project_dir


def test_absolute_path_when_path_is_absolute() -> None:
"""Test absolute_path property when the file path is already absolute."""
abs_path = Path("/tmp/some_absolute_file.txt").resolve()
df = DiscoveredFile(
path=abs_path,
ext_category=ExtCategory.from_file(abs_path),
project_path=Path("/tmp/project")
Comment on lines +31 to +35
)
Comment on lines +31 to +36
Comment on lines +32 to +36
result = df.absolute_path
assert result == abs_path


def test_absolute_path_when_path_is_relative_and_project_path_set() -> None:
"""Test absolute_path property when the file path is relative and project_path is set."""
rel_path = Path("src/main.py")
proj_path = Path("/home/user/project")
df = DiscoveredFile(
path=rel_path,
ext_category=ExtCategory.from_file(rel_path),
project_path=proj_path
Comment on lines +29 to +48
)
result = df.absolute_path
assert result == proj_path / rel_path


def test_absolute_path_when_project_path_is_none_success(temp_project: Path) -> None:
"""Test absolute_path property when project_path is falsy and get_project_path succeeds."""
rel_path = Path("src/main.py")
df = DiscoveredFile(
path=rel_path,
ext_category=ExtCategory.from_file(rel_path),
project_path=temp_project
)
# The property checks `if self.project_path:`. We can fake this by setting it to empty.
object.__setattr__(df, "project_path", "")
Comment on lines +60 to +63
Comment on lines +62 to +63
Comment on lines +57 to +63

result = df.absolute_path

# It should fall back to get_project_path() which is temp_project due to the fixture
assert result == temp_project / rel_path
Comment on lines +54 to +68


@patch('codeweaver.core.utils.get_project_path')
def test_absolute_path_when_project_path_is_none_filenotfound(mock_get_project_path) -> None:
Comment on lines +71 to +72
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion (testing): Strengthen the FileNotFoundError branch test by asserting get_project_path is actually called

In test_absolute_path_when_project_path_is_none_filenotfound, get_project_path is patched with side_effect = FileNotFoundError(), but the mock is never asserted. This means the test would still pass even if absolute_path stops calling get_project_path. Please add an assertion like:

mock_get_project_path.assert_called_once()

(or assert mock_get_project_path.called) so the test verifies the intended control flow for this branch.

"""Test absolute_path property when project_path is falsy and get_project_path raises FileNotFoundError."""
Comment on lines +71 to +73
mock_get_project_path.side_effect = FileNotFoundError()

rel_path = Path("src/main.py")
df = DiscoveredFile(
path=rel_path,
ext_category=ExtCategory.from_file(rel_path),
project_path=Path("/tmp")
)
# The property checks `if self.project_path:`. We can fake this by setting it to empty.
object.__setattr__(df, "project_path", "")
Comment on lines +77 to +83
Comment on lines +77 to +83
Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with this assessment. The type-inconsistent approach is problematic for several reasons:

  1. Type Safety: Setting project_path to an empty string when it's typed as DirectoryPath creates a mismatch that could hide real bugs
  2. Realism: Real code will never have project_path as an empty string - it will be a valid Path or potentially None
  3. Future Maintainability: If the code is refactored to properly handle Path | None, these tests won't catch regressions

Suggested Fix

Instead of:

object.__setattr__(df, "project_path", "")

Consider one of these approaches:

Option 1: Test with actual None (requires code changes)

def test_absolute_path_when_project_path_is_none_success(temp_project: Path) -> None:
    rel_path = Path("src/main.py")
    df = DiscoveredFile(
        path=rel_path,
        ext_category=ExtCategory.from_file(rel_path),
        project_path=None  # This would require updating the type hint
    )
    result = df.absolute_path
    assert result == temp_project / rel_path

Option 2: Mock get_project_path directly (preferred)

@patch('codeweaver.core.discovery.get_project_path')
def test_absolute_path_fallback_to_get_project_path(mock_get_project_path, temp_project: Path) -> None:
    """Test that absolute_path uses get_project_path when needed."""
    mock_get_project_path.return_value = temp_project
    
    rel_path = Path("src/main.py")
    # Create with a valid project_path, then test the property directly
    df = DiscoveredFile(
        path=rel_path,
        ext_category=ExtCategory.from_file(rel_path),
        project_path=temp_project
    )
    
    result = df.absolute_path
    assert result == temp_project / rel_path

The key is to test the actual behavior without violating type constraints.


result = df.absolute_path

# It should catch FileNotFoundError and return self.path
assert result == rel_path
Loading