From 9c85ebef7bdef9130fbe677bb65e605c0259da94 Mon Sep 17 00:00:00 2001 From: fullsend-code <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Sun, 2 Aug 2026 09:34:16 +0000 Subject: [PATCH 1/2] docs(#834): add library-utility reimplementation check to correctness agent Add a new review dimension to the correctness agent specification in the code-review problem document. When a diff introduces manual string parsing for a structured format (indexOf, split, substring chains), the correctness agent should check whether the project's dependencies export a dedicated parsing utility and flag reimplementations that miss edge cases the utility handles. Motivated by rhdh-plugins PR #3956 where manual indexOf(':') parsing of a Backstage location ref missed edge cases that parseLocationRef handles (trimming, error cases). A human reviewer caught this; the agent did not. Note: pre-commit could not run in sandbox (network restriction). Closes #834 --- docs/problems/code-review.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/problems/code-review.md b/docs/problems/code-review.md index 113bb16bfe..580e6a791a 100644 --- a/docs/problems/code-review.md +++ b/docs/problems/code-review.md @@ -64,6 +64,7 @@ Evaluates whether the code does what it claims to do. - If test files covering the changed code were recently modified, did those modifications weaken the test's ability to catch regressions? - Split-payload attacks: a production change paired with a test modification that masks the real behavior. (See [security-threat-model.md](security-threat-model.md#cross-cutting-attack-pattern-temporal-split-payload-test-poisoning) for why this matters.) - Coverage reduction: does the change remove or weaken existing test coverage? +- Library-utility reimplementation: when the diff introduces manual string parsing or format manipulation (e.g., `indexOf`, `split`, `substring`, `startsWith` chains on structured strings), check whether the project's declared dependencies export a dedicated parsing utility for that format (e.g., `parseLocationRef`, `semver.parse`, `URL` constructor). Flag manual reimplementations that miss edge cases the utility handles as medium-severity with a concrete suggestion. Skip if no utility exists or the manual implementation is intentional (e.g., performance-critical hot path). **Context needed:** The diff, relevant surrounding code, test files, git history of relevant test files, existing patterns in the repo. From eed2b00d63a83e16ca0a72690f5a963eac43d0f1 Mon Sep 17 00:00:00 2001 From: fullsend-fix <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Sun, 2 Aug 2026 10:35:17 +0000 Subject: [PATCH 2/2] docs(#834): split long library-utility bullet into sub-list Break the library-utility reimplementation bullet into a concise directive with nested sub-items for the detailed criteria, improving readability and matching the list style of adjacent entries. Addresses review feedback on #864 --- docs/problems/code-review.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/problems/code-review.md b/docs/problems/code-review.md index 580e6a791a..befb687d08 100644 --- a/docs/problems/code-review.md +++ b/docs/problems/code-review.md @@ -64,7 +64,10 @@ Evaluates whether the code does what it claims to do. - If test files covering the changed code were recently modified, did those modifications weaken the test's ability to catch regressions? - Split-payload attacks: a production change paired with a test modification that masks the real behavior. (See [security-threat-model.md](security-threat-model.md#cross-cutting-attack-pattern-temporal-split-payload-test-poisoning) for why this matters.) - Coverage reduction: does the change remove or weaken existing test coverage? -- Library-utility reimplementation: when the diff introduces manual string parsing or format manipulation (e.g., `indexOf`, `split`, `substring`, `startsWith` chains on structured strings), check whether the project's declared dependencies export a dedicated parsing utility for that format (e.g., `parseLocationRef`, `semver.parse`, `URL` constructor). Flag manual reimplementations that miss edge cases the utility handles as medium-severity with a concrete suggestion. Skip if no utility exists or the manual implementation is intentional (e.g., performance-critical hot path). +- Library-utility reimplementation: manual string parsing or format manipulation that reimplements a utility available in the project's declared dependencies. + - Look for patterns like `indexOf`, `split`, `substring`, or `startsWith` chains on structured strings where a dedicated parsing utility exists (e.g., `parseLocationRef`, `semver.parse`, `URL` constructor). + - Flag as medium-severity with a concrete suggestion when the manual reimplementation misses edge cases the utility handles. + - Skip if no utility exists or the manual implementation is intentional (e.g., performance-critical hot path). **Context needed:** The diff, relevant surrounding code, test files, git history of relevant test files, existing patterns in the repo.