docs(rules): clarify 800-line review ceiling - #2756
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe common rules now define 800 lines as a soft source-file ceiling. Documented exceptions are permitted, and unexplained files above the ceiling receive medium severity during review. ChangesFile-size guidance
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@rules/common/coding-style.md`:
- Around line 39-40: Synchronize file-size guidance across
rules/common/coding-style.md:39-40, rules/common/code-review.md:31, and
rules/common/code-review.md:57 with the soft 800-line ceiling policy: require
extraction only when maintainability warrants it, exempt test, generated, and
vendored files when justified, and classify findings as MEDIUM or INFO rather
than HIGH. Apply the same policy to the remaining consumer checklists, including
the cited code-review guidance and coding-style section.
🪄 Autofix
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: Pro Plus
Run ID: 6afac25d-6c52-4c51-8bc4-18188dddfce9
📒 Files selected for processing (2)
rules/common/code-review.mdrules/common/coding-style.md
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (2)
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Delegate complex features to a planner agent, architectural decisions to an architect agent, modified code to a code-reviewer agent, and security-sensitive work to a security-reviewer agent; use parallel agents for independent operations.
Never compromise security; validate all inputs and prevent hardcoded secrets, injection, XSS, CSRF, authentication or authorization failures, sensitive error leakage, and missing rate limits.
Never hardcode secrets; use environment variables or a secret manager, validate required secrets at startup, and rotate exposed secrets immediately.
Always create new objects and never mutate existing ones.
Plan complex features before implementation, identifying dependencies, risks, and phases.
Prefer many small, focused files; keep functions under 50 lines, files under 800 lines where practical, avoid nesting deeper than four levels, and use readable, well-named identifiers.
Handle errors at every level, provide user-friendly UI messages, log detailed server-side context, and never silently swallow errors.
Validate all user input at system boundaries using schema-based validation; fail fast with clear messages and never trust external data.
Required tests include unit tests, integration tests for APIs and database operations, and end-to-end tests for critical user flows.
Follow the mandatory TDD cycle: write a failing test, implement the minimum passing solution, then refactor and verify coverage.
Use a consistent API response envelope containing a success indicator, data payload, error message, and pagination metadata.
Encapsulate data access behind a repository interface with operations such asfindAll,findById,create,update, anddelete; business logic must depend on the abstraction rather than storage details.
Files:
rules/common/code-review.mdrules/common/coding-style.md
{skills,commands,agents,rules}/**
⚙️ CodeRabbit configuration file
{skills,commands,agents,rules}/**: Focus on prompt-injection resilience, tool-permission scope, destructive action guards, and secret exfiltration risks.
Files:
rules/common/code-review.mdrules/common/coding-style.md
| - 200-400 lines typical, with 800 lines as a soft maintainability ceiling for source files | ||
| - Test, generated, and vendored files may exceed the ceiling when their size is justified by their role |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- target files ---'
for f in rules/common/coding-style.md rules/common/code-review.md; do
if [ -f "$f" ]; then
printf '\n### %s\n' "$f"
sed -n '1,90p' "$f"
else
printf 'MISSING %s\n' "$f"
fi
done
printf '\n--- file-size and severity references ---\n'
rg -n -i -C 3 '800[- ]line|large file|file size|maintainability|unconditional|HIGH|MEDIUM|INFO|source files are cohesive' rules .github 2>/dev/null || true
printf '\n--- relevant tracked files ---\n'
git ls-files | rg '(^|/)(rules|skills|commands|agents)(/|$)' | sed -n '1,240p'Repository: affaan-m/ECC
Length of output: 40420
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- relevant code-review sections ---'
sed -n '24,38p' rules/common/code-review.md
sed -n '50,62p' rules/common/code-review.md
sed -n '90,103p' rules/common/code-review.md
printf '\n--- relevant coding-style sections ---\n'
sed -n '34,45p' rules/common/coding-style.md
sed -n '82,91p' rules/common/coding-style.md
printf '\n--- all 800-line policy references in instruction files ---\n'
rg -n -i -C 2 '800[- ]line|files? (under|over|approaching)|large files?|extract (utilities|modules)|file.{0,20}ceiling' \
--glob '*.md' --glob '*.mdc' --glob '*.json' --glob '*.yaml' --glob '*.yml' \
. ':!.git' 2>/dev/null || true
printf '\n--- focused diff summary and target-file diff ---\n'
git diff --stat
git diff -- rules/common/coding-style.md rules/common/code-review.mdRepository: affaan-m/ECC
Length of output: 24790
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- command and agent review guidance ---'
for f in commands/code-review.md agents/code-reviewer.md; do
if [ -f "$f" ]; then
printf '\n### %s\n' "$f"
rg -n -i -C 5 '800[- ]line|large file|file size|HIGH|MEDIUM|INFO|extract' "$f" || true
fi
done
printf '\n--- common-rule inconsistencies ---\n'
rg -n -i -C 4 '800[- ]line|large file|files? focused|extract modules|extract utilities' \
rules/common commands/code-review.md agents/code-reviewer.md \
.github/copilot-instructions.md AGENTS.md 2>/dev/null || trueRepository: affaan-m/ECC
Length of output: 14942
Synchronize all file-size guidance with the soft-ceiling policy.
rules/common/code-review.md:98, commands/code-review.md:48-50, and agents/code-reviewer.md:145-148 still require extraction for every file over 800 lines and classify the finding as HIGH. Apply the documented exceptions and MEDIUM/INFO treatment. Align rules/common/coding-style.md:87 and other consumer checklists with the same policy.
📍 Affects 2 files
rules/common/coding-style.md#L39-L40(this comment)rules/common/code-review.md#L31-L31rules/common/code-review.md#L57-L57
🤖 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 `@rules/common/coding-style.md` around lines 39 - 40, Synchronize file-size
guidance across rules/common/coding-style.md:39-40,
rules/common/code-review.md:31, and rules/common/code-review.md:57 with the soft
800-line ceiling policy: require extraction only when maintainability warrants
it, exempt test, generated, and vendored files when justified, and classify
findings as MEDIUM or INFO rather than HIGH. Apply the same policy to the
remaining consumer checklists, including the cited code-review guidance and
coding-style section.
|
daltino
left a comment
There was a problem hiding this comment.
Thanks for working on this! The changes look neat and well-structured.
haelyra
left a comment
There was a problem hiding this comment.
Approved. This is a useful clarification: 800 lines remains a meaningful source-maintainability signal without incorrectly treating tests, generated files, or justified exceptions as automatic failures. I verified the exact head locally with all 122 rule validations and Markdown lint. Thank you for making the rule more precise.
What Changed
Why This Change
Closes #2580 by removing the conflicting reading where coding-style.md described 800 lines as a hard maximum while code-review.md treated it as a non-blocking maintainability note. The rules now consistently distinguish source files from test/generated/vendor files.
Testing Done
ode scripts/ci/validate-rules.js (122 rule files validated)
ode scripts/ci/check-unicode-safety.js
Type of Change
Security & Quality Checklist