Repository navigation
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🤖 AI Review
The nine submitted findings merge into seven distinct findings, all confirmed against the checked-out code. The cap comparison and CI base wiring appear sound. The remaining issues affect local file discovery, Git path handling, an automatic fetch, test coverage, diagnostics, and documented repository conventions.
Findings
| Severity | Location | Category | Sources | Claim |
|---|---|---|---|---|
| 🟡 MINOR | .github/scripts/check-test-line-cap.ts:55 |
side-effects |
claude | When a local merge-base lookup fails, the check automatically fetches with --depth=50, making a network call and potentially turning a full clone into a shallow one. |
| 🟡 MINOR | .github/scripts/check-test-line-cap.ts:77 |
correctness |
claude+codex | A local check can pass while a new, oversized test file remains untracked. |
| 🟡 MINOR | .github/scripts/check-test-line-cap.test.ts:30 |
test-coverage |
claude | The test named for a renamed file repeats the growing-file inputs and never exercises rename parsing or old-path lookup. |
| 🟡 MINOR | .github/scripts/check-test-line-cap.ts:77 |
robustness |
claude+codex | Git-quoted test paths are parsed as literal filenames, causing valid files with special or non-ASCII characters to fail during lookup or reading. |
| 🟡 MINOR | .github/scripts/check-test-line-cap.ts:33 |
repository-convention |
codex | The new runtime script uses native async and Promise workflows despite the repository's documented Effect requirement. |
| ⚪ NIT | .github/scripts/check-test-line-cap.ts:20 |
diagnostics |
codex | The violation message reports a cap of 1000 even when a larger base count is the effective cap. |
| ⚪ NIT | .github/scripts/check-test-line-cap.test.ts:1 |
test-convention |
codex | The new unit test's .test.ts filename does not follow the documented .unit.test.ts suffix. |
Stats
Claude findings: 4 · Codex findings: 5 · Confirmed: 7 · Refuted: 0 · Uncertain: 0
Models: claude-opus-5-5 + gpt-6-sol · Trigger: auto · Workflow run
This review runs once per PR. A maintainer can request another with a /ai-review comment.
Drop the automatic local fetch fallback (no network calls from the local check; the skip notice now tells the developer to fetch develop themselves), pick up untracked new test files via `git ls-files`, switch diff parsing to NUL-delimited `--name-status -z` so quoted/renamed paths parse correctly, and report the effective cap (max of the floor and the base count) in violation messages.
jgoux
force-pushed
the
juliengoux/cli-2545-add-a-shrink-only-line-cap-ratchet-for-test-files
branch
from
October 2, 2026 11:43
47db109 to
761a958
Compare
avallete
reviewed
Oct 13, 2026
avallete
left a comment
avallete
left a comment
Member
There was a problem hiding this comment.
This will likely fail in the next PR isn't it ?
I think it would be better to have refactor rounds that merge / deduplicate files and tests rather than imposing a hard line cap.
Contributor
Author
|
Not a good idea. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Nothing stopped test files from growing. 53 test files are over 1,000 lines, and the largest is over 5,000. Pruning campaigns would shrink them, but nothing would keep them small afterwards.
Change
Adds a shrink-only line-cap ratchet for
*.test.tsfiles, modelled on OpenClaw's. A changed test file may not exceed the larger of 1,000 non-blank lines or its size at the base commit, so an oversized file can shrink but not grow, and a new file can't start over the cap. The ratchet compares each PR against its base, so there's no stored baseline to maintain, and a shrink stays in effect because the base moves forward. A renamed file is compared with its old path..github/scripts/check-test-line-cap.tsruns as part ofcheck:all. Locally it compares the working tree with the merge base oforigin/develop, so it catches changes before they're pushed.