[WRONG BRANCH] fix(issue-quality): bound reproduction matching cost - #36
[WRONG BRANCH] fix(issue-quality): bound reproduction matching cost#36luvs01 wants to merge 1 commit into
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
github-actions
Bot
changed the title
⏳ DRAFT
What to do
Its title has been prefixed with |
📝 WalkthroughWalkthroughChangesReproduction detection
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
✅ Deterministic PR hygiene checks passed. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 @.github/scripts/issue-quality.test.cjs:
- Around line 657-662: Update the long-token input in “checks long non-matching
reproduction tokens in linear time” to append an unsupported extension such as
“.unknown”, ensuring REPRO_PATH_RE is exercised. Keep the false assertion and
existing timing threshold unchanged.
🪄 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: 470dacad-612f-4e85-953b-295424563c1c
📒 Files selected for processing (2)
.github/scripts/issue-quality.cjs.github/scripts/issue-quality.test.cjs
| it("checks long non-matching reproduction tokens in linear time", () => { | ||
| const startedAt = performance.now(); | ||
| assert.equal(hasActionableReproductionDetail("a".repeat(60_000)), false); | ||
| assert.ok(performance.now() - startedAt < 500, "actionable reproduction check took too long"); | ||
| }); | ||
|
|
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
Exercise REPRO_PATH_RE in the performance test.
The current input contains no path marker. .github/scripts/issue-quality.cjs returns at Line 1106 before REPRO_PATH_RE runs at Line 1112. This test cannot detect a regression in the bounded filename alternative at Line 1090.
Use a long token with an unsupported extension, such as "a".repeat(60_000) + ".unknown", and keep the false assertion and timing check.
Proposed test adjustment
- assert.equal(hasActionableReproductionDetail("a".repeat(60_000)), false);
+ assert.equal(
+ hasActionableReproductionDetail("a".repeat(60_000) + ".unknown"),
+ false,
+ );📝 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.
| it("checks long non-matching reproduction tokens in linear time", () => { | |
| const startedAt = performance.now(); | |
| assert.equal(hasActionableReproductionDetail("a".repeat(60_000)), false); | |
| assert.ok(performance.now() - startedAt < 500, "actionable reproduction check took too long"); | |
| }); | |
| it("checks long non-matching reproduction tokens in linear time", () => { | |
| const startedAt = performance.now(); | |
| assert.equal( | |
| hasActionableReproductionDetail("a".repeat(60_000) + ".unknown"), | |
| false, | |
| ); | |
| assert.ok(performance.now() - startedAt < 500, "actionable reproduction check took too long"); | |
| }); |
🤖 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 @.github/scripts/issue-quality.test.cjs around lines 657 - 662, Update the
long-token input in “checks long non-matching reproduction tokens in linear
time” to append an unsupported extension such as “.unknown”, ensuring
REPRO_PATH_RE is exercised. Keep the false assertion and existing timing
threshold unchanged.
1 participant
Motivation
Description
REPRO_PATH_REby changing"[\\w.@-]+\\.(?:json|...)\\b"to"(?:^|[^\\w.@-])[\\w.@-]+\\.(?:json|...)\\b"to prevent mid-token quadratic backtracking.ACTIONABLE_REPRO_REand instead testREPRO_COMMAND_REandREPRO_FAILURE_REfirst, only evaluatingREPRO_PATH_REwhen the cleaned text contains path-like characters.hasActionableReproductionDetailto return early for long plain tokens that contain no actionable syntax before runningclean()and heavier path/fence logic.checks long non-matching reproduction tokens in linear timethat assertshasActionableReproductionDetail("a".repeat(60_000))completes quickly and remains false.Testing
node --test .github/scripts/issue-quality.test.cjs, which passed all tests including the new performance regression (113 tests passing).bun run typecheckandbun run privacy:scan, both of which succeeded.git diff --checkwith no issues reported.bun run testsuite in this environment was not completed due to unrelated timeouts and test-time constraints, and uncovered unrelated failures/timeouts outside the changed files; the fix itself is covered by the focused tests above.Summary by CodeRabbit
Bug Fixes
Performance