What happened
PR #4582 added a required field to ScalarAggregationResult and widened aggregationChartDisplayColor from string to string | null on WeightedStatusScoreAggregationResult — both @public exports. The review agent flagged these as breaking changes requiring a major bump per the repo's .fullsend/AGENTS.md guidance ("major for breaking API changes — removed exports, changed interfaces, dropped support"). The PR author updated the changeset to describe the changes as "BREAKING" but kept the bump at minor. The review agent's own style conventions sub-agent then found project precedent (the metric-collectors-endpoint changeset) for using minor for this class of change. Two human reviewers approved without addressing the semver concern. The PR was merged with a minor bump on Sep 9.
What could go better
The .fullsend/AGENTS.md changeset guidance says major for "changed interfaces" but the repo's actual practice allows minor for TypeScript-level type additions and widenings that maintain runtime/JSON compatibility. This ambiguity caused the review agent to raise the same missing-version-bump finding across 5+ review passes — a finding that was technically correct per the documented policy but incorrect per established practice. The repeated false-positive wasted review budget (~$32+ across 8 runs, with the semver concern contributing to noise in each pass) and trained reviewers to ignore agent findings. Confidence: high — the evidence is clear from both the documented policy, the established precedent found by the style sub-agent, and the human reviewers' decision to merge with minor.
Proposed change
Update the changeset bump guidance in .fullsend/AGENTS.md to distinguish between different categories of interface changes. Specifically, after the existing major bullet, add clarification such as:
major for breaking API changes that affect runtime behavior or remove existing capabilities (removed exports, narrowed types, removed required fields, dropped support).minor is acceptable for additive or widening type changes on @public interfaces that maintain runtime/JSON compatibility (new optional or required fields on response types, type widening from string to string | null) — these are TypeScript compile-time breaking but runtime-compatible.
Alternatively, if the maintainers intend strict semver for TypeScript consumers, reinforce the current major guidance and update the metric-collectors-endpoint precedent to match. Either resolution eliminates the ambiguity.
Validation criteria
On the next 3 PRs that widen types or add required fields to @public interfaces in this repo, the review agent should not flag the bump level as incorrect when the changeset follows the updated guidance. The missing-version-bump finding should only appear when the bump genuinely violates the clarified policy.
Generated by retro agent from #4582
What happened
PR #4582 added a required field to
ScalarAggregationResultand widenedaggregationChartDisplayColorfromstringtostring | nullonWeightedStatusScoreAggregationResult— both@publicexports. The review agent flagged these as breaking changes requiring amajorbump per the repo's.fullsend/AGENTS.mdguidance ("major for breaking API changes — removed exports, changed interfaces, dropped support"). The PR author updated the changeset to describe the changes as "BREAKING" but kept the bump atminor. The review agent's own style conventions sub-agent then found project precedent (themetric-collectors-endpointchangeset) for usingminorfor this class of change. Two human reviewers approved without addressing the semver concern. The PR was merged with aminorbump on Sep 9.What could go better
The
.fullsend/AGENTS.mdchangeset guidance saysmajorfor "changed interfaces" but the repo's actual practice allowsminorfor TypeScript-level type additions and widenings that maintain runtime/JSON compatibility. This ambiguity caused the review agent to raise the samemissing-version-bumpfinding across 5+ review passes — a finding that was technically correct per the documented policy but incorrect per established practice. The repeated false-positive wasted review budget (~$32+ across 8 runs, with the semver concern contributing to noise in each pass) and trained reviewers to ignore agent findings. Confidence: high — the evidence is clear from both the documented policy, the established precedent found by the style sub-agent, and the human reviewers' decision to merge withminor.Proposed change
Update the changeset bump guidance in
.fullsend/AGENTS.mdto distinguish between different categories of interface changes. Specifically, after the existingmajorbullet, add clarification such as:majorfor breaking API changes that affect runtime behavior or remove existing capabilities (removed exports, narrowed types, removed required fields, dropped support).minoris acceptable for additive or widening type changes on@publicinterfaces that maintain runtime/JSON compatibility (new optional or required fields on response types, type widening fromstringtostring | null) — these are TypeScript compile-time breaking but runtime-compatible.Alternatively, if the maintainers intend strict semver for TypeScript consumers, reinforce the current
majorguidance and update themetric-collectors-endpointprecedent to match. Either resolution eliminates the ambiguity.Validation criteria
On the next 3 PRs that widen types or add required fields to
@publicinterfaces in this repo, the review agent should not flag the bump level as incorrect when the changeset follows the updated guidance. Themissing-version-bumpfinding should only appear when the bump genuinely violates the clarified policy.Generated by retro agent from #4582