Skip to content

Review agent: flag inconsistent retry/backoff strategies within a file #538

Description

@guyoron1

What happened

In PR fullsend-ai#2359, internal/layers/enrollment.go introduced two different backoff strategies in the same file: dispatchRepoMaintenanceWithRetry uses additive backoff (delay += initial, producing 3s, 6s, 9s, 12s, 15s), while awaitWorkflowRun uses exponential backoff via nextInterval (current * 2, producing 2s, 4s, 8s, 15s). Both cap at 15 seconds. The PR was merged without this inconsistency being flagged by either the review agent or the human reviewer.

What could go better

This is a minor code quality issue — both approaches work and converge to the same max interval. However, a review agent could have flagged the inconsistency as a low-severity style finding, prompting the author to adopt a single backoff strategy. Confidence: medium. The inconsistency does not cause bugs, but it creates cognitive overhead for future maintainers who must reason about why two different backoff strategies were chosen in the same file. The additive backoff in dispatchRepoMaintenanceWithRetry may actually be intentional (it was pre-existing code), in which case the review comment would still be useful as a prompt for the author to explain the rationale.

Proposed change

Add guidance to the review agent's code-review skill (or the repo's AGENTS.md) to flag inconsistent retry/backoff patterns within a single file or module as a low-severity finding. Specifically, when a PR introduces a new retry loop in a file that already contains one, the review should note whether the strategies are consistent and ask the author to justify divergence or harmonize. This could be added as a bullet point in the review agent's code-review skill under a 'consistency' or 'patterns' section.

Validation criteria

On the next 5 PRs that introduce or modify retry/backoff logic in a file that already has such logic, the review agent should flag the consistency question. This can be verified by inspecting review agent comments on such PRs. The finding should be low-severity (informational, not blocking).


Generated by retro agent from fullsend-ai#2359

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions