[ARCHIVED] Benchmark issue 471 - #471
Conversation
Instruct code agents to read existing sibling methods before implementing new methods that call the same API endpoint or use the same multi-step API pattern. Covers replicating error handling (truncation checks, 404-to-ErrNotFound mapping), retry logic (retryOnTransient), and response validation. Prevents a class of medium-severity review findings where new methods omit error handling that existing methods already implement for the same endpoint. Closes #366
|
/fs-review |
|
🤖 Finished Review · ✅ Success · Started 10:15 AM UTC · Completed 10:21 AM UTC |
ReviewFindingsMedium
Low
Labels: PR is tagged as an LSP experiment with RTK+Ponytail methodology per title conventions. Previous runReview — ApprovePR: #471 — Add API pattern replication guidance to AGENTS.md SummaryClean, well-scoped documentation change that adds an "API pattern replication" section to Dimensions evaluated
No findings at any severity level.
Labels: PR adds agent guidance documentation to AGENTS.md |
|
🤖 Finished Review · ✅ Success · Started 10:26 AM UTC · Completed 10:36 AM UTC |
|
|
||
| **When reviewing PRs:** Flag any direct `exec.Command("gh", ...)`, raw GitHub API calls, or other forge-specific operations outside `internal/forge/github/` as a medium-severity or higher finding. This is an architectural violation, not a style preference. | ||
|
|
||
| ## API pattern replication |
There was a problem hiding this comment.
[medium] protected-path
This PR modifies AGENTS.md, which is a protected governance file. The change is well-justified (linked to issue #470 with clear rationale from a real incident on PR fullsend-ai#2360), but human approval is always required for protected-path changes regardless of context.
|
|
||
| ## API pattern replication | ||
|
|
||
| When implementing a new method that calls the same API endpoint or uses the same multi-step API pattern as an existing method in the file, read the existing implementation first. Replicate its error handling (e.g., truncation checks, 404-to-ErrNotFound mapping), retry logic (e.g., `retryOnTransient`), and response validation. In `internal/forge/github/github.go`, `commitFilesTo` implements the Git Trees API pattern (refs → commit → tree) with truncation checks — any new method using the same endpoint must handle truncation consistently. |
There was a problem hiding this comment.
[low] specificity_level
The new section names specific functions (commitFilesTo) and API details (Git Trees API), which is more granular than some sections. However, this is consistent with existing sections — the Go code section names specific files and the Forge abstraction section references specific commands. No change warranted.
Closes #470
LSP experiment v2: clean first-review with gopls-lsp enabled.
Code identical to PR #418 (issue #366).
Branch created from commit bc05aa0.