redhat-developer / redhat-developer/rhdh-plugins
Add public export surface review guidance to scorecard AGENTS.md
Nobody has claimed this yet.
- Dominant language
- TypeScript
- Stars
- 48
- Forks
- 120
- Avg merge
- 2d 6h
- Merged PRs (30d)
- 337
Description
What happened
On PR #4602, human reviewer @PatAKnight flagged on Sep 11 that the new ScalarAggregationType public type export from scorecard-common was only consumed by test files, suggesting a local type alias instead of expanding public API surface. The review agent ran 9 times (costing ~$60.87) but did not raise this concern until Sep 14—three days later—and framed it as a changeset bump-level issue rather than an API surface concern. The agent's framing ("changeset marks as minor but changes are limited to type annotation refactoring") was less actionable than the human's direct question of whether the export was needed at all.
What could go better
The review agent lacks specific guidance about evaluating whether new public exports from shared packages (scorecard-common, scorecard-node) are consumed by production code. The scorecard workspace AGENTS.md has detailed guidance for metric IDs, threshold resolution, and provider architecture, but nothing about public API surface management for shared packages. This gap meant the agent focused on style/convention concerns while missing the higher-value architectural question. I'm moderately confident this guidance would have helped on the first review pass: the agent already reads workspace AGENTS.md and follows its review guidance sections (e.g., threshold change severity). Adding explicit API surface guidance should produce similar compliance. Uncertainty: the agent might still not catch every case, since determining whether an export is test-only requires cross-file analysis.
Proposed change
Add a "Review guidance for public API changes" section to workspaces/scorecard/AGENTS.md (after the existing "Review guidance for threshold changes" section). The guidance should instruct: (1) When a PR adds or modifies public type exports in scorecard-common or scorecard-node, verify the exported types are consumed by production (non-test) code in at least one other package. (2) If a new public export is only consumed by test files, flag it as [medium] and suggest using a local type alias within the test file or test utilities instead. (3) If the author intentionally broadens usage to justify the export (as happened in this PR), accept that as a valid resolution but verify the changeset bump level reflects the API surface addition.
Validation criteria
On the next scorecard PR that adds a public type export to scorecard-common or scorecard-node, the review agent should flag the export and ask whether it is consumed by non-test production code. Verify over the next 3 scorecard PRs that touch shared package exports. Success: the agent raises the API surface concern on the first review pass (not the 4th+), and the finding is actionable enough that the author responds to it.
Generated by retro agent from https://github.com/redhat-developer/rhdh-plugins/pull/4602
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Open workspaces/scorecard/AGENTS.md and read the existing “Review guidance for threshold changes” section. Add the requested public API changes guidance after it, covering production-code usage, test-only exports, and changeset bump validation for scorecard-common and scorecard-node. Done means the three proposed rules and validation intent are clearly documented in that file.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- documentation
- Issue type
- Documentation
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 86/100