Skip to content

pr-review: Add type derivation patterns from PR #1737 learnings#1759

Merged
nick-inkeep merged 2 commits intomainfrom
pr-review-improvements/pr-1737-learnings
Feb 6, 2026
Merged

pr-review: Add type derivation patterns from PR #1737 learnings#1759
nick-inkeep merged 2 commits intomainfrom
pr-review-improvements/pr-1737-learnings

Conversation

@nick-inkeep
Copy link
Collaborator

Summary

Adds generalizable patterns identified from human reviewer feedback on PR #1737 (feat: add image support to chat messages).

Source PR: #1737
Human Comments Analyzed: 6

Proposed Improvements

Target Agent Change Type Summary
pr-review-types New checklist section Type Definition Discipline - prefer derived types over manual definitions
pr-review-consistency New detection patterns Structural similarity search, utility reuse checks

Generalizability Justification

These patterns pass the generalizability test:

  1. Cross-codebase applicability: Type derivation (z.infer, Pick, Omit) is universal across TypeScript codebases
  2. Universal principle: DRY applies to types as much as utilities - reducing schema/type duplication
  3. Expressible as checklist: "Check if new types can derive from existing schemas"
  4. Industry recognition: Standard TypeScript best practice (Zod docs, TS handbook)

Changes in this PR

.claude/agents/pr-review-types.md

  • Added "Type Definition Discipline" checklist section
  • Added type derivation patterns: z.infer, Pick, Omit, discriminated unions
  • Added failure mode: "Type proliferation blindness"

.claude/agents/pr-review-consistency.md

  • Added find-similar-patterns skill integration
  • Added structural similarity search for peer comparison
  • Added utility reuse detection before accepting new helpers

Evidence

Human comment from @amikofalvy on PR #1737:

"Are we redefining types and schemas? I think it's a mistake made throughout our codebase, but we try to reduce schemas and types where we can. You can infer types from zod schemas."

This feedback identified a pattern gap: the existing pr-review-* agents weren't checking for type/schema derivation opportunities.


Generated by closed-pr-review-auto-improver analysis (manual test run)

Patterns extracted from human reviewer feedback:
- Type Definition Discipline: use z.infer<> instead of manual type definitions
- Type Composition Safety: discriminated unions for mutually exclusive states

Co-Authored-By: Claude <noreply@anthropic.com>
@changeset-bot
Copy link

changeset-bot bot commented Feb 6, 2026

⚠️ No Changeset found

Latest commit: e46ec3f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link

vercel bot commented Feb 6, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
agents-api Ready Ready Preview, Comment Feb 6, 2026 3:51am
agents-docs Ready Ready Preview, Comment Feb 6, 2026 3:51am
agents-manage-ui Ready Ready Preview, Comment Feb 6, 2026 3:51am

Request Review

Agents should be self-contained without referencing what other agents do.
- pr-review-consistency: Remove "→ see pr-review-types" reference
- pr-review-types: Change "reviewed by pr-review-consistency" to "out of scope"

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
nick-inkeep added a commit that referenced this pull request Feb 6, 2026
The pr-review-consistency.md and pr-review-types.md improvements belong
in PR #1759, not this auto-improver feature branch.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
nick-inkeep added a commit that referenced this pull request Feb 6, 2026
* feat: Add closed-pr-review-auto-improver agent

Automated system that analyzes human reviewer feedback after PRs are merged
to identify generalizable improvements for the pr-review-* subagent system.

- Workflow triggers on merged PRs, extracts human/bot comments
- Agent applies 4-criteria generalizability test
- Creates draft PRs with improvements to pr-review-*.md files

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* feat: Add context-gathering phase for deeper comment analysis

- Include diffHunk in GraphQL query (shows code each comment is on)
- Add Phase 2 "Deep-Dive on Promising Comments" with explicit guidance:
  - Read the full file to understand broader context
  - Grep for schemas/types/patterns mentioned in comments
  - Understand the anti-pattern before judging generalizability
- Update Tool Policy to emphasize context gathering
- Renumber phases (now 6 phases total)

The agent now actively investigates each comment rather than
judging based on comment text alone.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* refactor: Apply write-agent best practices

Based on write-agent skill guidance:

1. Add near-miss example (questions/discussions ≠ reviewer feedback)
2. Strengthen Role & Mission - describe what "excellence looks like"
3. Failure modes now use contrastive examples (❌ vs ✅)
4. Phase 2 now checklist format with stop condition
5. Example shows completed checklist, not just steps

Key insight: "Stop here if you can't articulate a clear principle"
prevents vague improvements from polluting reviewers.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* feat: Add git time-travel for progressive context gathering

- Phase 2 now uses git rev-list + git show to see code at comment time
- Progressive gathering: diffHunk → full file → PR diff → other files
- GraphQL query now includes createdAt for all comment types
- Added git rev-list and git show to allowedTools

This ensures the agent sees what the human reviewer saw, not the
final merged state which may have fixes applied.

Co-Authored-By: Claude <noreply@anthropic.com>

* feat: Add explicit stop conditions for context gathering

Two exit paths at each level:
- EXIT A: Not generalizable (repo-specific, one-off bug, style preference)
- EXIT B: Pattern found (can articulate anti-pattern + universal principle)

Includes decision flow diagram and two contrasting examples showing
early exit (repo-specific DateUtils) vs pattern discovery (type/schema DRY).

Co-Authored-By: Claude <noreply@anthropic.com>

* refactor: Strengthen agent per write-agent best practices

- Role & Mission: Add "what the best human analyst would do" section
- Failure modes: Add "Asserting when uncertain" with contrastive example
- Generalizability: Add confidence calibration guidance
- Add explicit conservative default: "when torn, choose lower confidence"

Per write-agent skill review: personality should describe best human
behavior, failure modes should include asserting when uncertain
(relevant for classification tasks).

Co-Authored-By: Claude <noreply@anthropic.com>

* pr-review: Add Schema-Type Derivation Discipline learnings

Pattern extracted from PR #1737 human reviewer feedback (amikofalvy):
- Types should derive from Zod schemas using z.infer<typeof schema>
- Use Pick/Omit/Partial instead of manually redefining type subsets
- Extract shared enum/union schemas instead of inline string literals

Changes:
- pr-review-types.md: New anti-pattern + analysis step 6 with detection patterns
- pr-review-consistency.md: Extended "Reuse" section to cover types

This demonstrates the closed-pr-review-auto-improver output — these are
the exact changes the agent proposed when run against PR #1737.

Co-Authored-By: Claude <noreply@anthropic.com>

* pr-review: Expand type derivation sources

Extended "Schema-Type Derivation Discipline" to cover full spectrum:
- Zod/validation schemas (z.infer)
- Database schemas (Prisma, Drizzle generated types)
- Internal packages (@inkeep/*, shared types)
- External packages/SDKs (OpenAI, Vercel AI SDK)
- Function signatures (Parameters<>, ReturnType<>)
- Existing domain types (Pick, Omit, Partial)

Added table format for clarity and comprehensive detection patterns.

Co-Authored-By: Claude <noreply@anthropic.com>

* pr-review: Add advanced type derivation patterns from codebase research

Expanded type derivation guidance based on actual patterns found in agents repo:
- Awaited<ReturnType<>> for async function returns
- keyof typeof for constants-derived types
- interface extends and intersection (&) for composition
- Discriminated unions with type guards
- satisfies operator for type-safe constants
- Re-exports for API surface boundaries
- Type duplication detection signals

Patterns sourced from agents-api codebase analysis including:
- env.ts, middleware/*, types/app.ts, domains/run/*

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* pr-review: Add Zod schema composition patterns

Added guidance for Zod schema extension/derivation patterns based on
codebase research (packages/agents-core/src/validation/schemas.ts):

- .extend() for adding/overriding fields
- .pick()/.omit() for field subsetting
- .partial() for Insert → Update schema derivation
- .extend().refine() for cross-field validation
- Anti-patterns: parallel schemas, duplicated fields

Examples from codebase:
- SubAgentInsertSchema.extend({ id: ResourceIdSchema })
- SubAgentUpdateSchema = SubAgentInsertSchema.partial()
- StopWhenSchema.pick({ transferCountIs: true })

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* pr-review: Rationalize scope between types and consistency agents

Clear separation of concerns:
- pr-review-types: Illegal states, invariants, unsafe narrowing
- pr-review-consistency: DRY, schema reuse, convention conformance

Moved to consistency:
- Zod schema composition patterns (.extend, .pick, .partial)
- Type derivation detection signals
- satisfies operator, re-exports conventions

Kept in types (type safety focus):
- Discriminated unions vs optional fields (prevents illegal states)
- Type guards vs unsafe `as` assertions
- Detection of union types without discriminants

Added cross-reference note in types agent pointing to consistency
for derivation/DRY concerns.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* closed-pr-review-auto-improver: Add exit states, skills integration, and Phase 5.5

- Add skills: pr-review-subagents-available, pr-review-subagents-guidelines, find-similar-patterns
- Add proper exit states at Phase 1, 2, and 4 (embedded in workflow, not separate section)
- Add Phase 5 step 2: "Find examples of the pattern" with judgment guidance
- Add Phase 5.5: Full file review & integration planning (scope fit, duplication check)
- Update output contract with detailed JSON structure and exit examples
- Add reviewer tagging to close the feedback loop

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* closed-pr-review-auto-improver: Add "keep agents standalone" guidance

Agents should be self-contained without cross-references to other agents.
This prevents coupling and ensures agents work correctly when read in isolation.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* Recover lost skills: find-similar-patterns, pr-review-subagents-*

These skills were created in the previous session but never committed.
Recovered from conversation history.

- find-similar-patterns: Methodology for finding similar code patterns
- pr-review-subagents-available: Catalog of pr-review-* agents with scope boundaries
- pr-review-subagents-guidelines: Best practices for writing/improving reviewers

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* Remove pr-review-* changes (moved to separate PR #1759)

The pr-review-consistency.md and pr-review-types.md improvements belong
in PR #1759, not this auto-improver feature branch.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* refactor: Move auto-improver to private plugin repo

Move agent and skills to inkeep/internal-cc-plugins for CI/CD-only loading:
- Removed: .claude/agents/closed-pr-review-auto-improver.md
- Removed: .agents/skills/{find-similar-patterns,pr-review-subagents-available,pr-review-subagents-guidelines}/

Updated workflow:
- Added step to clone inkeep/internal-cc-plugins
- Added --plugin-dir flag to load agent from plugin

Prerequisites before merging:
1. Create private repo: inkeep/internal-cc-plugins
2. Push plugin content to new repo
3. Add GH_PAT_PLUGINS secret to inkeep/agents

Co-Authored-By: Claude <noreply@anthropic.com>

* Switch from PAT to GitHub App for cross-repo auth

GitHub Apps provide better security and maintainability:
- 8-hour token lifetime (vs days/infinite for PATs)
- No user account dependency (survives personnel changes)
- Zero manual rotation (tokens generated fresh each run)
- Scales to N plugins without additional credentials

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* Add workflow_dispatch trigger for testing historical PRs

- Add manual trigger with pr_number input
- Add Get PR Metadata step to fetch data via API (works for both triggers)
- Update all PR references to use the new metadata outputs
- Enables testing against historical PRs like #1737

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* Address PR review feedback: robustness improvements

Fixes from Claude Code review:

1. Add merge validation for workflow_dispatch (Major)
   - Prevents analyzing unmerged PRs via manual trigger
   - Validates PR is merged before proceeding

2. Use unique HEREDOC delimiters (Major)
   - Prevents collision if PR body/comments contain "EOF"
   - Uses unique suffixes like __BODY_DELIM_7f3a9b2c__

3. Pin claude-code-action to SHA (Major)
   - Aligns with claude-code-review.yml for consistency
   - Tracks issue #892 for AJV validation bug

4. Add concurrency control (Minor)
   - Prevents race conditions on concurrent runs
   - Groups by PR number, doesn't cancel in-progress

5. Add shell error handling (Minor)
   - set -eo pipefail in all shell blocks
   - Fail fast on command errors

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* Add debug artifact upload for troubleshooting

Uploads execution logs when workflow fails, matching pattern
from claude-code-review.yml. 7-day retention.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
@nick-inkeep nick-inkeep marked this pull request as ready for review February 6, 2026 10:54
@nick-inkeep nick-inkeep merged commit 15f7442 into main Feb 6, 2026
7 of 8 checks passed
@nick-inkeep nick-inkeep deleted the pr-review-improvements/pr-1737-learnings branch February 6, 2026 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant