Skip to content

docs(#834): add library-utility reimplementation check to correctness agent - #864

Open
fullsend-ai-coder[bot] wants to merge 2 commits into
mainfrom
agent/834-library-utility-reimplement-check
Open

docs(#834): add library-utility reimplementation check to correctness agent#864
fullsend-ai-coder[bot] wants to merge 2 commits into
mainfrom
agent/834-library-utility-reimplement-check

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown

Summary

Adds a library-utility reimplementation check to the correctness agent's review dimensions in the code-review problem document. This new dimension instructs the correctness agent to flag manual string parsing that reimplements a utility available in the project's declared dependencies.

Related Issue

Closes #834

Changes

  • Added a new bullet point to the correctness agent section in docs/problems/code-review.md
  • The new dimension covers manual string parsing/format manipulation (e.g., indexOf, split, substring, startsWith chains) where a library utility exists (e.g., parseLocationRef, semver.parse, URL constructor)
  • Includes skip conditions: no utility exists, or manual parsing is intentional (e.g., performance-critical hot path)

Testing

  • Change is documentation-only — no code tests affected
  • Secret scan passed
  • make lint could not run (sandbox network restriction); post-script runs authoritative pre-commit

Checklist

  • PR title follows Conventional Commits (correct type, ! for breaking changes)
  • No DCO sign-off (autonomous agent session)

Closes #834

Post-script verification

  • Branch is not main/master (agent/834-library-utility-reimplement-check)
  • Secret scan passed (gitleaks — c887fc47c59d8b87b5282ca44044d2933d30df41..HEAD)
  • PR body secret scan passed (gitleaks — no-git)
  • Pre-commit hooks passed (authoritative run on runner)
  • Tests ran inside sandbox

… 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 fullsend-ai#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
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 2, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 9:44 AM UTC · Completed 9:55 AM UTC
Commit: 9c85ebe · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 2, 2026

Copy link
Copy Markdown

Review

Findings

Low

  • [bullet-list-structure] docs/problems/code-review.md:67 — 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 (Correctness, Security, Intent & Coherence, Style, Docs Currency, Cross-repo) 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.
    Remediation: 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.
Previous run

Review

Findings

Low

  • [list-item-length] docs/problems/code-review.md:67 — The new bullet point is longer than most other bullets in the Correctness agent list, though other bullets in the document (e.g., the "Split-payload attacks" bullet and the "Non-rendering Unicode characters" bullet in the Security section) are comparably long, so this is a minor stylistic note.
    Remediation: Consider splitting into a short directive bullet and a nested sub-list with the detailed criteria, though this is stylistic preference.

Labels: PR is a pure documentation change to a problem document

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge documentation Improvements or additions to documentation labels Aug 2, 2026
@guyoron1

guyoron1 commented Aug 2, 2026

Copy link
Copy Markdown
Owner

/fs-fix

@fullsend-ai-coder

fullsend-ai-coder Bot commented Aug 2, 2026

Copy link
Copy Markdown
Author

🤖 Finished Fix · ✅ Success · Started 10:31 AM UTC · Completed 10:36 AM UTC
Commit: 9c85ebe · View workflow run →

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
@fullsend-ai-coder

Copy link
Copy Markdown
Author

🔧 Fix agent — iteration 1 (human-triggered)

Addressed 1 review finding. Split the long library-utility reimplementation bullet (line 67) into a short directive with nested sub-items, as suggested by the reviewer.

Fixed (1):

  1. list-item-length: library-utility reimplementation bullet too long (docs/problems/code-review.md): Split the long single bullet into a concise directive with three nested sub-items covering the detection pattern, severity guidance, and skip conditions. Preserves all original content while improving readability.

Tests: passed

Next steps:

  • /fs-review — request a re-review of the changes
  • /fs-fix <your instruction> — run another fix pass with specific guidance
  • Push commits directly — review re-runs automatically on push
    Updated by fullsend fix agent

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 2, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 10:40 AM UTC · Completed 10:50 AM UTC
Commit: eed2b00 · View workflow run →

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

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown

This pull request has been automatically marked as stale because it has not had any activity in the last month. It will be closed in 2 weeks if no further activity occurs. Remove the stale label to reset the inactivity timer.

@github-actions github-actions Bot added the stale label Sep 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation ready-for-merge All reviewers approved — ready to merge stale

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Review agent should flag manual parsing that reimplements available library utilities

1 participant