Skip to content

chore: add tests for schema/chunking + ruff cleanup - #2

Merged
filhocf merged 3 commits into
mainfrom
chore/housekeeping-review
May 4, 2026
Merged

filhocf merged 3 commits into
mainfrom
chore/housekeeping-review

Conversation

@filhocf

@filhocf filhocf commented May 3, 2026

Copy link
Copy Markdown
Owner

Housekeeping PR to improve test coverage for untested modules.

New tests for:

  • schema.py: CodeChunk, QueryResult dataclasses
  • chunking.py: public API exports (Chunk, TextPosition, ChunkerFn, CHUNKER_REGISTRY)

@gemini-code-assist review

filhocf added 2 commits May 3, 2026 20:59
- CodeChunk: creation, embedding type flexibility
- QueryResult: creation, score range
- Chunking exports: Chunk, TextPosition, ChunkerFn, CHUNKER_REGISTRY
- 8 new tests, all passing

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request introduces a new test suite for the schema and chunking modules and performs minor cleanup in the protocol definitions. Feedback focuses on improving the test suite by removing unnecessary dependencies on numpy and pytest, replacing numpy array usage with standard Python lists for better portability, and avoiding brittle assertions based on internal string representations.

I am having trouble creating individual review comments. Click here to see my feedback.

tests/test_schema_chunking.py (5-6)

medium

The numpy and pytest imports are unnecessary in this file. pytest is not explicitly used, and numpy introduces a heavy dependency that contradicts the 'relaxed compatibility' goal mentioned in src/cocoindex_code/schema.py. Removing these will make the tests more portable and faster to collect in environments where numpy is not installed.

tests/test_schema_chunking.py (24)

medium

To avoid a hard dependency on numpy in the test suite, consider using a standard Python list for the embedding. This is consistent with the Any type hint in the CodeChunk dataclass and the project's goal of maintaining compatibility without requiring numpy as a mandatory dependency.

            embedding=[0.0] * 384,

tests/test_schema_chunking.py (81)

medium

Asserting against the string representation of CHUNKER_REGISTRY is brittle as it depends on the internal implementation of ContextKey.__str__. A more robust test would verify the object's type or other public attributes directly.

@filhocf
filhocf merged commit 68f9d2c into main May 4, 2026
@filhocf
filhocf deleted the chore/housekeeping-review branch May 4, 2026 11:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant