Skip to content

feat(assessors): implement File Size Limits assessor (Tier 2) #141

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jeremyeder merged 1 commit into from
Nov 25, 2025
Merged

feat(assessors): implement File Size Limits assessor (Tier 2) #141

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 3 additions & 1 deletion src/agentready/assessors/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
)
from .stub_assessors import (
ConventionalCommitsAssessor,
FileSizeLimitsAssessor,
GitignoreAssessor,
LockFilesAssessor,
create_stub_assessors,
Expand Down Expand Up @@ -58,12 +59,13 @@ def create_all_assessors() -> list[BaseAssessor]:
TypeAnnotationsAssessor(),
StandardLayoutAssessor(),
LockFilesAssessor(),
# Tier 2 Critical (10 assessors - 6 implemented, 4 stubs)
# Tier 2 Critical (10 assessors - 7 implemented, 3 stubs)
TestCoverageAssessor(),
PreCommitHooksAssessor(),
ConventionalCommitsAssessor(),
GitignoreAssessor(),
OneCommandSetupAssessor(),
FileSizeLimitsAssessor(),
SeparationOfConcernsAssessor(),
ConciseDocumentationAssessor(),
InlineDocumentationAssessor(),
Expand Down
141 changes: 126 additions & 15 deletions src/agentready/assessors/stub_assessors.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,131 @@ def assess(self, repository: Repository) -> Finding:
return Finding.error(self.attribute, reason="Could not read .gitignore")


class FileSizeLimitsAssessor(BaseAssessor):
"""Tier 2 - File size limits for context window optimization."""

@property
def attribute_id(self) -> str:
return "file_size_limits"

@property
def tier(self) -> int:
return 2

@property
def attribute(self) -> Attribute:
return Attribute(
id=self.attribute_id,
name="File Size Limits",
category="Context Window Optimization",
tier=self.tier,
description="Files are reasonably sized for AI context windows",
criteria="<5% of files >500 lines, no files >1000 lines",
default_weight=0.03,
)

def assess(self, repository: Repository) -> Finding:
"""Check for excessively large files that strain context windows.

Scoring:
- 100: All files <500 lines
- 75-99: Some files 500-1000 lines
- 0-74: Files >1000 lines exist
"""
# Count files by size
large_files = [] # 500-1000 lines
huge_files = [] # >1000 lines
total_files = 0

# Check common source file extensions
extensions = {".py", ".js", ".ts", ".jsx", ".tsx", ".go", ".java", ".rb", ".rs", ".cpp", ".c", ".h"}

for ext in extensions:
pattern = f"**/*{ext}"
try:
from pathlib import Path
Copy link

@coderabbitai coderabbitai bot Nov 25, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Move import to module level.

Importing Path inside the loop is inefficient and unnecessary since pathlib is already available. The import should be at the top of the file with other imports.

Apply this diff to move the import:

 """Stub implementations for remaining assessors - minimal but functional.
 
 These are simplified implementations to get the MVP working. Each can be
 enhanced later with more sophisticated detection and scoring logic.
 """
 
+from pathlib import Path
+
 from ..models.attribute import Attribute
 from ..models.finding import Finding, Remediation

Then remove line 266 from the loop.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In src/agentready/assessors/stub_assessors.py around line 266, an import
statement "from pathlib import Path" is placed inside a loop; move that import
to the module-level top with the other imports (ensuring you don't duplicate it
if Path is already imported), then remove the in-loop import at line 266 so the
loop uses the module-level Path reference.

for file_path in repository.path.glob(pattern):
if file_path.is_file():
try:
with open(file_path, "r", encoding="utf-8") as f:
lines = len(f.readlines())
total_files += 1

if lines > 1000:
huge_files.append((file_path.relative_to(repository.path), lines))
elif lines > 500:
large_files.append((file_path.relative_to(repository.path), lines))
except (OSError, UnicodeDecodeError):
# Skip files we can't read
pass
except Exception:
pass
Comment on lines +263 to +282
Copy link

@coderabbitai coderabbitai bot Nov 25, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Narrow exception handling to avoid hiding bugs.

The bare except Exception: on line 281 is too broad and can silently suppress programming errors, making debugging difficult. The inner exception handling (line 278) for file reading is appropriate, but the outer catch-all should be more specific or removed.

If the intent is to handle glob failures, be explicit:

         for ext in extensions:
             pattern = f"**/*{ext}"
             try:
-                from pathlib import Path
                 for file_path in repository.path.glob(pattern):
                     if file_path.is_file():
                         try:
                             with open(file_path, "r", encoding="utf-8") as f:
                                 lines = len(f.readlines())
                                 total_files += 1
 
                                 if lines > 1000:
                                     huge_files.append((file_path.relative_to(repository.path), lines))
                                 elif lines > 500:
                                     large_files.append((file_path.relative_to(repository.path), lines))
                         except (OSError, UnicodeDecodeError):
                             # Skip files we can't read
                             pass
-            except Exception:
-                pass
+            except (OSError, PermissionError) as e:
+                # Skip directories we can't access
+                continue
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
for ext in extensions:
pattern = f"**/*{ext}"
try:
from pathlib import Path
for file_path in repository.path.glob(pattern):
if file_path.is_file():
try:
with open(file_path, "r", encoding="utf-8") as f:
lines = len(f.readlines())
total_files += 1
if lines > 1000:
huge_files.append((file_path.relative_to(repository.path), lines))
elif lines > 500:
large_files.append((file_path.relative_to(repository.path), lines))
except (OSError, UnicodeDecodeError):
# Skip files we can't read
pass
except Exception:
pass
for ext in extensions:
pattern = f"**/*{ext}"
try:
for file_path in repository.path.glob(pattern):
if file_path.is_file():
try:
with open(file_path, "r", encoding="utf-8") as f:
lines = len(f.readlines())
total_files += 1
if lines > 1000:
huge_files.append((file_path.relative_to(repository.path), lines))
elif lines > 500:
large_files.append((file_path.relative_to(repository.path), lines))
except (OSError, UnicodeDecodeError):
# Skip files we can't read
pass
except (OSError, PermissionError):
# Skip directories we can't access
continue
🤖 Prompt for AI Agents 
In src/agentready/assessors/stub_assessors.py around lines 263 to 282, remove
the broad `except Exception:` that silently swallows all errors; either delete
the outer try/except entirely or narrow it to the specific errors you expect
from the glob/Path operations (for example OSError and ValueError), and for any
caught cases log a brief debug/error message and allow unexpected exceptions to
propagate (re-raise) so real bugs aren’t hidden.


if total_files == 0:
return Finding.not_applicable(
self.attribute,
reason="No source files found to assess",
)

# Calculate score
if huge_files:
# Penalty for files >1000 lines
percentage_huge = (len(huge_files) / total_files) * 100
score = max(0, 70 - (percentage_huge * 10))
status = "fail"
evidence = [
f"Found {len(huge_files)} files >1000 lines ({percentage_huge:.1f}% of {total_files} files)",
f"Largest: {huge_files[0][0]} ({huge_files[0][1]} lines)",
Copy link

@coderabbitai coderabbitai bot Nov 25, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Sort huge_files before reporting the largest.

Line 298 assumes huge_files[0] is the largest file, but the list is populated in discovery order, not sorted by size. This could report an arbitrary large file rather than the actual largest.

Apply this diff:

         if huge_files:
             # Penalty for files >1000 lines
+            huge_files.sort(key=lambda x: x[1], reverse=True)
             percentage_huge = (len(huge_files) / total_files) * 100
             score = max(0, 70 - (percentage_huge * 10))
             status = "fail"
             evidence = [
                 f"Found {len(huge_files)} files >1000 lines ({percentage_huge:.1f}% of {total_files} files)",
                 f"Largest: {huge_files[0][0]} ({huge_files[0][1]} lines)",
             ]
🤖 Prompt for AI Agents 
In src/agentready/assessors/stub_assessors.py around line 298, the code assumes
huge_files[0] is the largest but the list is in discovery order; sort huge_files
by the size field (the second tuple element) in descending order before
accessing index 0, e.g. sort with key=lambda t: t[1], reverse=True, and also
handle the empty-list case safely (e.g., skip reporting or provide a fallback
message) so the reported "Largest" truly reflects the largest file.

]
elif large_files:
# Partial credit for files 500-1000 lines
percentage_large = (len(large_files) / total_files) * 100
if percentage_large < 5:
score = 90
status = "pass"
else:
score = max(75, 100 - (percentage_large * 5))
status = "pass"
Comment on lines +300 to +308
Copy link

@coderabbitai coderabbitai bot Nov 25, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Fix status logic to match criteria.

The criteria states "<5% of files >500 lines", but when percentage_large >= 5%, the status remains "pass" (lines 307-308). This violates the stated criteria and will incorrectly report passing status for repositories that should fail.

Apply this diff:

         elif large_files:
             # Partial credit for files 500-1000 lines
             percentage_large = (len(large_files) / total_files) * 100
             if percentage_large < 5:
                 score = 90
                 status = "pass"
             else:
                 score = max(75, 100 - (percentage_large * 5))
-                status = "pass"
+                status = "warn"  # or "fail" depending on severity interpretation
 
             evidence = [
                 f"Found {len(large_files)} files 500-1000 lines ({percentage_large:.1f}% of {total_files} files)",
             ]

Note: The Finding model expects "pass", "fail", "skipped", "error", or "not_applicable" status values, so verify if "warn" is supported or use "fail" instead.

Committable suggestion skipped: line range outside the PR's diff.

Comment on lines +304 to +308
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Nov 25, 2025

Choose a reason for hiding this comment

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

P1 Badge Fail when >5% of files exceed 500 lines

The FileSizeLimitsAssessor sets status = "pass" for any repository that only has 500–1000 line files, even when those files make up more than the stated threshold of 5% (percentage_large branch sets status to pass in both paths). Because the attribute criteria/threshold string says <5% files >500 lines, a repo with, for example, 30% of its files at ~800 lines will be marked passing and receive no remediation despite violating the stated limit, so oversized files silently slip through.

Useful? React with 👍 / 👎.


evidence = [
f"Found {len(large_files)} files 500-1000 lines ({percentage_large:.1f}% of {total_files} files)",
]
else:
# Perfect score
score = 100.0
status = "pass"
evidence = [f"All {total_files} source files are <500 lines"]

Comment on lines +291 to +318
Copy link

@coderabbitai coderabbitai bot Nov 25, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Consider using calculate_proportional_score() helper for consistency.

The custom scoring logic works but diverges from the framework's recommended calculate_proportional_score() helper method, which provides consistent, maintainable scoring across assessors.

Based on coding guidelines, assessors should use calculate_proportional_score() for partial compliance scoring.

Example refactor (after fixing the status logic):

# For repositories with huge files (>1000 lines)
if huge_files:
    percentage_huge = (len(huge_files) / total_files) * 100
    # Score degrades as percentage increases (lower is better)
    score = self.calculate_proportional_score(
        measured_value=percentage_huge,
        threshold=5.0,  # Allow up to 5% before severe penalty
        higher_is_better=False
    )
    score = min(score, 70)  # Cap at 70 for huge files
    status = "fail"
    # ... evidence ...

return Finding(
attribute=self.attribute,
status=status,
score=score,
measured_value=f"{len(huge_files)} huge, {len(large_files)} large out of {total_files}",
threshold="<5% files >500 lines, 0 files >1000 lines",
evidence=evidence,
remediation=(
None
if status == "pass"
else Remediation(
summary="Refactor large files into smaller, focused modules",
steps=[
"Identify files >1000 lines",
"Split into logical submodules",
"Extract classes/functions into separate files",
"Maintain single responsibility principle",
],
tools=["refactoring tools", "linters"],
commands=[],
examples=[
"# Split large file:\n# models.py (1500 lines) → models/user.py, models/product.py, models/order.py"
],
citations=[],
)
),
error_message=None,
)

Comment on lines +224 to +347
Copy link

@coderabbitai coderabbitai bot Nov 25, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify unit tests exist for this new assessor.

The coding guidelines require "All new assessors must have unit tests with >80% code coverage maintained". No test file is visible in this PR for FileSizeLimitsAssessor.

Please run the following script to check if unit tests exist:

If tests don't exist, they should be added to meet the >80% coverage requirement. As per coding guidelines, use CLAUDEmdAssessor and READMEAssessor as reference implementations for test patterns.

🏁 Script executed:

#!/bin/bash
# Description: Check for unit tests for FileSizeLimitsAssessor

# Search for test files that reference FileSizeLimitsAssessor
rg -n "FileSizeLimitsAssessor" --type=py -g "*test*.py" -C 3

# Also check for test files in typical test directories
fd -e py -p "test.*file.*size" tests/ test/

Length of output: 51

🏁 Script executed:

#!/bin/bash
# Description: Locate test directories and check for FileSizeLimitsAssessor tests

# Find test directories in the repo
echo "=== Test directories ==="
fd -t d -n "test" | head -20

# Find all Python test files regardless of location
echo -e "\n=== All Python test files ==="
fd -e py -p "test_" | head -20

# Search for any file containing "FileSizeLimitsAssessor"
echo -e "\n=== Files mentioning FileSizeLimitsAssessor ==="
rg "FileSizeLimitsAssessor" --type=py -l

# Check if tests/assessors directory exists
echo -e "\n=== Check specific test paths ==="
ls -la tests/assessors/ 2>/dev/null || echo "tests/assessors/ not found"
ls -la src/agentready/assessors/test*.py 2>/dev/null || echo "No test files in assessors directory"

# Look for any test file related to stub_assessors or file_size
echo -e "\n=== Search for stub assessor tests ==="
rg "stub_assessor" --type=py -l
rg "FileSizeLimits\|file_size_limits" --type=py -l

Length of output: 1434

🏁 Script executed:

#!/bin/bash
# Description: Check for stub assessor or FileSizeLimitsAssessor tests in detail

# Check the test_assessors_structure.py file
echo "=== Content of test_assessors_structure.py ==="
cat tests/unit/test_assessors_structure.py | head -100

# Look for any test file that might test FileSizeLimitsAssessor or stub assessors
echo -e "\n=== Search for assessor test patterns ==="
rg "test.*FileSizeLimits|class.*FileSizeLimits|def.*file_size" --type=py

# Check testing.py to see what testing utilities are available
echo -e "\n=== First 50 lines of testing.py ==="
head -50 src/agentready/assessors/testing.py

# Look for any test file that imports stub_assessors
echo -e "\n=== Files that import stub_assessors ==="
rg "from.*stub_assessor|import.*stub_assessor" --type=py

# Check if there's a conftest or test configuration
echo -e "\n=== pytest configuration ==="
cat pytest.ini 2>/dev/null || echo "No pytest.ini found"
cat pyproject.toml 2>/dev/null | grep -A 20 "\[tool.pytest" || echo "No pytest config in pyproject.toml"

Length of output: 5639

🏁 Script executed:

#!/bin/bash
# Description: Final verification - check if stub_assessors.py has any test coverage

# Check if stub_assessors.py has pragma no cover or similar exclusions
echo "=== Check for coverage pragmas in stub_assessors.py ==="
rg "pragma: no cover|skip|xfail" src/agentready/assessors/stub_assessors.py

# Verify what stub assessors are exported from __init__.py
echo -e "\n=== Stub assessors exported ==="
rg "FileSizeLimits" src/agentready/assessors/__init__.py -A 5 -B 5

# Check overall test file count for assessors
echo -e "\n=== All test files matching assessor patterns ==="
fd -e py "test_" tests/ | rg -i assessor

Length of output: 878

Add unit tests for FileSizeLimitsAssessor to meet >80% coverage requirement.

The coding guidelines mandate "All new assessors must have unit tests with >80% code coverage maintained." The repository's pytest configuration enforces --cov-fail-under=90, which will cause CI to fail if this assessor lacks test coverage. Currently, no unit tests exist for FileSizeLimitsAssessor.

Create a test file (e.g., tests/unit/test_file_size_limits_assessor.py) following the pattern of test_assessors_structure.py. Test scenarios should include:

  • Files below 500 lines (100 score)
  • Files between 500-1000 lines (75-99 score)
  • Files exceeding 1000 lines (0-74 score)
  • No source files found (not_applicable)
  • Unreadable files (graceful degradation)

Reference CLAUDEmdAssessor and READMEAssessor test patterns per guidelines.

🤖 Prompt for AI Agents 
In src/agentready/assessors/stub_assessors.py around lines 224-347, the
FileSizeLimitsAssessor has no unit tests and CI will fail due to coverage;
create tests/unit/test_file_size_limits_assessor.py that follows existing
assessor test patterns (e.g., CLAUDEmdAssessor), using pytest and tmp_path to
build a temporary repo directory and a minimal Repository-like object with a
.path attribute (Path) pointing to it, then cover these scenarios: (1) all
source files <500 lines — assert score == 100 and status == "pass"; (2) some
files 500-1000 lines — assert score in 75-99 and appropriate
measured_value/evidence; (3) files >1000 lines — assert score in 0-74, status
"fail", and evidence about huge files; (4) no source files — assert
Finding.not_applicable; (5) unreadable files — create a file and make it
unreadable (or mock open to raise UnicodeDecodeError/OSError) and assert
assessor skips it and still returns correct result; parametrize inputs where
useful and assert on measured_value, threshold, and remediation presence to
exercise all branches so coverage exceeds the required threshold.


# Create stub assessors for remaining attributes
# These return "not_applicable" for now but can be enhanced later

Expand Down Expand Up @@ -269,20 +394,6 @@ def create_stub_assessors():
"""Create stub assessors for remaining attributes."""
return [
# Tier 2 Critical
StubAssessor(
"one_command_setup",
"One-Command Build/Setup",
"Build & Development",
2,
0.03,
),
StubAssessor(
"file_size_limits",
"File Size Limits",
"Context Window Optimization",
2,
0.03,
),
StubAssessor(
"dependency_freshness",
"Dependency Freshness & Security",
Expand Down Expand Up @@ -317,7 +428,7 @@ def create_stub_assessors():
"Issue & Pull Request Templates",
"Git & Version Control",
4,
0.01,
0.01
),
StubAssessor(
"container_setup",
Expand Down
Loading