Skip to content

docs(rules): clarify 800-line review ceiling - #2756

Open
SulimanAbdulrazzaq wants to merge 1 commit into
affaan-m:mainfrom
SulimanAbdulrazzaq:fix/rules-800-line-soft-ceiling
Open

docs(rules): clarify 800-line review ceiling#2756
SulimanAbdulrazzaq wants to merge 1 commit into
affaan-m:mainfrom
SulimanAbdulrazzaq:fix/rules-800-line-soft-ceiling

Conversation

@SulimanAbdulrazzaq

Copy link
Copy Markdown

What Changed

  • clarify that 800 lines is a soft maintainability ceiling for source files
  • document justified exceptions for test, generated, and vendored files
  • align the code-review severity wording with the coding-style guidance

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

  • git diff --check
  • Markdownlint unavailable locally (dependency not installed)

Type of Change

  • docs: Documentation

Security & Quality Checklist

  • No secrets or API keys committed
  • Follows conventional commits format

@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • Documentation
    • Clarified source-file organization and maintainability guidelines.
    • Documented exceptions for test, generated, vendored, or deliberately justified files exceeding recommended size limits.
    • Updated code-review guidance to flag unexplained oversized source files.

Walkthrough

The 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.

Changes

File-size guidance

Layer / File(s) Summary
Align file-size rules
rules/common/coding-style.md, rules/common/code-review.md
The coding-style rule defines typical file size, the soft 800-line ceiling, and exceptions for justified test, generated, and vendored files. The code-review rule permits documented exceptions and classifies unexplained oversized source files as medium severity.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Suggested reviewers: affaan-m

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the documentation change that clarifies the 800-line review ceiling.
Description check ✅ Passed The description explains the clarified threshold, justified exceptions, issue addressed, and validation performed.
Linked Issues check ✅ Passed The changes resolve issue #2580 by defining a soft source-file ceiling and justified exceptions for test, generated, and vendored files.
Out of Scope Changes check ✅ Passed All changes are limited to the two rule documents and directly support issue #2580 objectives.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

📥 Commits

Reviewing files that changed from the base of the PR and between bed96af and 748651b.

📒 Files selected for processing (2)
  • rules/common/code-review.md
  • rules/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 as findAll, findById, create, update, and delete; business logic must depend on the abstraction rather than storage details.

Files:

  • rules/common/code-review.md
  • rules/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.md
  • rules/common/coding-style.md

Comment on lines +39 to +40
- 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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🗄️ 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.md

Repository: 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 || true

Repository: 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-L31
  • rules/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.

@greptile-apps

greptile-apps Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This documentation update clarifies how reviewers should apply the 800-line maintainability guidance and documents justified exceptions for test, generated, and vendored files.

The reported conflict in rules/common/code-review.md was disproved by comparing the policy before and after this change. The current review guidance permits deliberate exceptions, and the companion style guidance expressly permits justified test, generated, and vendored files to exceed the guideline.

No actionable defects remain, and the change is safe to merge.

Confidence Score: 5/5

The documentation changes consistently describe the exception path for files that appropriately exceed the 800-line guideline.

A focused before-and-after policy comparison exercised the reported conflict and observed that the current text explicitly permits the described exceptions, leaving no confirmed findings.

Files Needing Attention: No files need further attention.

T-Rex T-Rex Logs

What T-Rex did

  • I ran the focused policy comparison script against HEAD^ and HEAD and found no contradiction, confirming the policy softened ceiling and explicit exceptions for justified test, generated, and vendored files align with the current guidance.
  • I verified the precise finding location in rules/common/code-review.md:31 and confirmed that the alleged contradiction is not actionable because the updated policy matches the soft ceiling and its exceptions, as shown by the before/after policy logs.

View all artifacts

T-Rex Ran code and verified through T-Rex

Reviews (1): Last reviewed commit: "docs(rules): clarify 800-line review cei..." | Re-trigger Greptile

@daltino daltino 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.

Thanks for working on this! The changes look neat and well-structured.

@haelyra haelyra left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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.

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.

rules/common/coding-style.md's 800-line file cap vs. code-review.md's MEDIUM/non-blocking rating for the same limit — which governs?

3 participants