fix: skip test files and test functions in Python type annotations check by aviavraham · Pull Request #493 · ambient-code/agentready

aviavraham · 2026-06-08T13:35:44Z

Summary

Fixes #385

Skip test files (test_*.py, *_test.py, conftest.py, tests//test/ dirs) from Python type annotation scoring
Skip test functions (test_*) inside non-test source files
Aligns Python behavior with the existing Go _test.go exclusion
Updated docs/attributes.md to document the exclusion

Test plan

16 new tests covering file detection and integration behavior
Linting passes (black, isort, ruff)

Summary by CodeRabbit

Bug Fixes
- Improved type-annotation scoring accuracy by excluding Python test files and test_* test functions from evaluation.
Documentation
- Updated “Type Annotations (Static Typing)” criteria to clarify that test files and test_* functions are not included in scoring.
Tests
- Added unit and integration tests covering detection of test file patterns (including Windows-style paths) and verifying assessor behavior for mixed and tests-only repositories.

coderabbitai · 2026-06-08T13:35:51Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: d0419a44-2d59-4321-8aea-73f15cab4ebd

📥 Commits

Reviewing files that changed from the base of the PR and between 0c044c1 and dac2946.

📒 Files selected for processing (1)

tests/unit/test_assessors_code_quality.py

📝 Walkthrough

Walkthrough

Type annotation scoring now excludes Python test files and test_* functions. The assessor updates file discovery and AST counting, and the docs and tests match the new scoring rules.

Changes

Skip tests in Python type-annotation scoring

Layer / File(s)	Summary
Test file detection and docs `src/agentready/assessors/code_quality.py`, `docs/attributes.md`	`_is_python_test_file()` classifies test files from filename and path patterns, and the attribute docs state that test files and `test_*` functions are excluded from scoring.
Skip tests during Python scoring `src/agentready/assessors/code_quality.py`	Python type-annotation discovery filters out test files from git and filesystem scans, reads sources as UTF-8, and omits `test_*` functions from AST counts.
Discovery and scoring tests `tests/unit/test_assessors_code_quality.py`	Git-backed helpers support assessor discovery tests, and unit cases cover test-path detection, skipped `test_*` functions, non-test failures, and `not_applicable` for test-only repositories.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The title matches the fix, but it misses the required Conventional Commits scope in `type(scope): description` format.	Rename it to a Conventional Commits title with a scope, such as `fix(code-quality): skip test files and test functions in Python type annotations check`.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check	✅ Passed	The PR skips pytest-style test files and `test_*` functions from Python type annotation scoring, matching issue `#385`'s requirements.
Out of Scope Changes check	✅ Passed	The docs update and tests support the stated fix, and no unrelated code changes are evident.
Docstring Coverage	✅ Passed	Docstring coverage is 84.62% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

✨ Simplify code

Create PR with simplified code

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

github-actions · 2026-06-08T13:42:53Z

📈 Test Coverage Report

Branch	Coverage
This PR	75.2%
Main	75.1%
Diff	✅ +0.1%

Coverage calculated from unit tests only

coderabbitai

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/agentready/assessors/code_quality.py`:
- Around line 162-176: _is_python_test_file currently uses PurePosixPath which
doesn't split Windows backslash separators, so normalize the incoming file_path
by replacing backslashes with forward slashes (e.g., file_path =
file_path.replace("\\\\", "/")) before constructing PurePosixPath or split
parts; update the _is_python_test_file function to perform this normalization
(or alternatively use pathlib.PurePath with explicit normalization) so names
like "tests\\test_app.py" are correctly detected as test files while preserving
the existing name/parts checks (references: function _is_python_test_file,
PurePosixPath).

In `@tests/unit/test_assessors_code_quality.py`:
- Around line 39-65: Add Windows-style path variants to the test cases for
TypeAnnotationsAssessor._is_python_test_file: update the parameter list used by
test_identifies_test_files to include at least "tests\\test_foo.py" (and
optionally "test\\test_bar.py") so the underscore method is validated on
backslash-separated paths; keep the test_identifies_non_test_files list
unchanged unless you want to add non-test backslash examples similarly.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: bf718a70-e4e1-4998-b7eb-87e04829136f

📥 Commits

Reviewing files that changed from the base of the PR and between 0fd1975 and 7681bf8.

📒 Files selected for processing (3)

docs/attributes.md
src/agentready/assessors/code_quality.py
tests/unit/test_assessors_code_quality.py

coderabbitai

♻️ Duplicate comments (1)

tests/unit/test_assessors_code_quality.py (1)
40-51: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add Windows-style path cases to lock in cross-platform test-file detection

Line 43-Line 51 only validate POSIX-style separators. Add at least one backslash path (e.g., tests\\test_foo.py) so _is_python_test_file behavior is enforced for Windows-style paths too; this is the key edge case for path-based exclusion logic.
Suggested minimal test update
     `@pytest.mark.parametrize`(
         "path",
         [
             "tests/test_foo.py",
             "test/test_bar.py",
             "tests/unit/test_baz.py",
             "src/tests/test_helpers.py",
+            r"tests\test_win.py",
+            r"test\unit\test_bar.py",
             "test_something.py",
             "foo_test.py",
             "conftest.py",
             "tests/conftest.py",
         ],
     )
As per coding guidelines, “tests/**: Check for missing edge cases.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unit/test_assessors_code_quality.py` around lines 40 - 51, The
parametric test only covers POSIX-style paths; add at least one Windows-style
path with backslashes (for example "tests\\test_foo.py") to the
pytest.mark.parametrize "path" list so the _is_python_test_file behavior is
validated on Windows separators as well; update the list of test cases in
tests/unit/test_assessors_code_quality.py (the pytest parametrization block) to
include the backslash variant.
Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In `@tests/unit/test_assessors_code_quality.py`:
- Around line 40-51: The parametric test only covers POSIX-style paths; add at
least one Windows-style path with backslashes (for example "tests\\test_foo.py")
to the pytest.mark.parametrize "path" list so the _is_python_test_file behavior
is validated on Windows separators as well; update the list of test cases in
tests/unit/test_assessors_code_quality.py (the pytest parametrization block) to
include the backslash variant.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 5d72f872-bda9-4c05-a349-a47dbdd9e3b1

📥 Commits

Reviewing files that changed from the base of the PR and between 7681bf8 and 3097624.

📒 Files selected for processing (1)

tests/unit/test_assessors_code_quality.py

aviavraham · 2026-06-09T15:44:47Z

@coderabbitai review

coderabbitai · 2026-06-09T15:44:53Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

jwm4

Review from Bill Murdock with assistance from Claude Code

Nice work on this fix, @aviavraham. Clean implementation that aligns well with the existing Go _test.go exclusion pattern. The two-layer exclusion (file-level + function-level), the not_applicable handling for test-only repos, and the Windows path normalization are all well done.

There are merge conflicts with main that need resolving. Sorry about that, I believe my recent PR is what caused them. Should be straightforward to rebase.

Once that's resolved, this is good to go.

…eck (ambient-code#385) Test files (test_*.py, *_test.py, conftest.py, tests/ dirs) and test functions (test_*) were penalizing repos for missing type hints on code that doesn't benefit from annotations. Aligns Python behavior with the existing Go _test.go exclusion. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Addresses CodeRabbit review: PurePosixPath doesn't split Windows backslash separators, so normalize before classification. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

aviavraham · 2026-06-25T12:58:00Z

@jwm4, the PR is ready to merge

aviavraham marked this pull request as ready for review June 8, 2026 13:38

coderabbitai Bot requested changes Jun 8, 2026

View reviewed changes

Comment thread src/agentready/assessors/code_quality.py

Comment thread tests/unit/test_assessors_code_quality.py

coderabbitai Bot reviewed Jun 8, 2026

View reviewed changes

coderabbitai Bot approved these changes Jun 9, 2026

View reviewed changes

jwm4 mentioned this pull request Jun 11, 2026

Type annotations scorer does not count async functions #497

Open

jwm4 approved these changes Jun 11, 2026

View reviewed changes

aviavraham and others added 2 commits June 25, 2026 15:24

fix: normalize backslash path separators in test file detection

0c044c1

Addresses CodeRabbit review: PurePosixPath doesn't split Windows backslash separators, so normalize before classification. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

aviavraham force-pushed the fix/385-type-annotations-skip-tests branch from 2d942e6 to 0c044c1 Compare June 25, 2026 12:25

fix: correct import sorting for CI compatibility

dac2946

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: skip test files and test functions in Python type annotations check#493

fix: skip test files and test functions in Python type annotations check#493
aviavraham wants to merge 3 commits into
ambient-code:mainfrom
aviavraham:fix/385-type-annotations-skip-tests

aviavraham commented Jun 8, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 8, 2026 •

edited

Loading

Reviews paused

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

github-actions Bot commented Jun 8, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

aviavraham commented Jun 9, 2026

Uh oh!

coderabbitai Bot commented Jun 9, 2026 •

edited

Loading

Uh oh!

jwm4 left a comment

Uh oh!

aviavraham commented Jun 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

aviavraham commented Jun 8, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

github-actions Bot commented Jun 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📈 Test Coverage Report

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

aviavraham commented Jun 9, 2026

Uh oh!

coderabbitai Bot commented Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jwm4 left a comment

Choose a reason for hiding this comment

Uh oh!

aviavraham commented Jun 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

aviavraham commented Jun 8, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 8, 2026 •

edited

Loading

github-actions Bot commented Jun 8, 2026 •

edited

Loading

coderabbitai Bot commented Jun 9, 2026 •

edited

Loading