Skip to content

Add public export surface review guidance to scorecard AGENTS.md #4777

Open

Description

@fullsend-ai-retro

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 #4602

Activity

  1. fullsend-ai-triage commented on Sep 15, 2026

    @fullsend-ai-triage

    🤖 Finished Triage · ✅ Success · Started 4:03 PM UTC · Completed 4:07 PM UTC

    Commit: 47ac76d · View workflow run →

    Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $0.80

  2. fullsend-ai-triage commented on Sep 15, 2026

    @fullsend-ai-triage

    The proposed guidance section is well-defined and fits naturally after the existing "Review guidance for threshold changes" section in the scorecard AGENTS.md. The three rules cover the full lifecycle: flag test-only exports, suggest local type aliases as alternatives, and verify changeset bump level when the author justifies the export.

    Related open issues also adding review guidance to scorecard AGENTS.md (these are independent, not blocking):

    Proposed test case

    Scenario: Review agent flags test-only public export
      Given a PR adds a new public type export to scorecard-common
        And the export is only imported by test files (no production consumers)
      When the review agent runs on the PR
      Then the agent flags the export as [medium] on its first review pass
        And suggests using a local type alias within the test file instead
        And the finding is actionable enough for the author to respond
    

    Labels: Documentation change to scorecard workspace AGENTS.md, consistent with labeling on similar issues (#4734, #4733, #4499).


    Next steps:

    • /fs-code — agent creates a PR to implement this issue
    • /fs-code <your instruction> — agent implements with your specific guidance
  3. fullsend-ai-coder commented on Sep 15, 2026

    @fullsend-ai-coder
    Contributor

    🤖 Finished Code · ✅ Success · Started 4:08 PM UTC · Completed 4:17 PM UTC

    Commit: 47ac76d · View workflow run →

    Runtime: claude · Model: opus → claude-opus-4-6 · Cost: $1.08

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions