Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions docs/problems/code-review.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +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: manual string parsing or format manipulation that reimplements a utility available in the project's declared dependencies.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[low] bullet-list-structure

The new bullet item uses a multi-level indented structure (parent bullet with three sub-bullets) that is inconsistent with the existing flat bullet list in the Correctness agent section. All six agent sections in the document use flat single-level bullets exclusively. The closest analog is the Security agent's Non-rendering Unicode characters bullet, which packs comparable elaboration into a single flat bullet using inline bold text and an em-dash rather than nested sub-items.

Suggested fix: Flatten the bullet structure to match existing patterns — either incorporate all information into a single inline bullet (following the Security agent's Unicode bullet pattern) or use multiple top-level bullets at the same indentation level.

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

Expand Down
Loading