Review Feedback Schema#

Purpose#

Structured format for logging code review outcomes. This data enables:

1. Identifying rules that produce false positives
2. Tracking skill accuracy over time
3. Automated skill improvement via pattern analysis

Schema#

csv

date,file,line,rule_source,category,severity,issue,verdict,rationale

FieldTypeDescriptionExample ValuesdateISO dateWhen review occurred2025-12-23``filepathRelative file pathamelia/agents/developer.py``linestringLine number(s)128, 190-191``rule_sourcestringSkill and rule that triggered issuepython-code-review/common-mistakes:unused-variables, pydantic-ai-common-pitfalls:tool-decorator``categoryenumIssue taxonomytype-safety, async, error-handling, style, patterns, testing, security``severityenumAs flagged by reviewercritical, major, minor``issuestringBrief descriptionReturn type list[Any] loses type safety``verdictenumHuman decisionACCEPT, REJECT, DEFER, ACKNOWLEDGE``rationalestringWhy verdict was chosenpydantic-ai docs explicitly support this pattern

Gates (feedback log rows)#

Run in order before appending a row. Do not skip ahead while a gate fails.

1. Evidence bound to code
   • Pass when: file is a repo-relative path that exists (or existed at review time), and line identifies line number(s) you actually opened—not only a paraphrased summary.
2. Rule source attributable
   • Pass when: rule_source matches skill-name[/section]:rule-id (see Rule Source Format). If the trigger is unknown, set a best-effort source and state the gap in rationale instead of inventing a rule id.
3. Verdict backed by artifact
   • Pass when: For REJECT, rationale cites something checkable (command + output, doc URL, or quoted code). For ACCEPT, it states the fix or points to the change. For DEFER/ ACKNOWLEDGE, it names a tracker, timeline, or documented intent per Verdict Types.
4. Row shape valid
   • Pass when: The line has nine comma-separated fields matching the header row; fields that contain commas or newlines are CSV-quoted so a standard parser preserves columns.

Verdict Types#

VerdictMeaningActionACCEPTIssue is valid, will fixCode change madeREJECTIssue is invalid/wrongNo change; may improve skillDEFERValid but not fixing nowTracked for laterACKNOWLEDGEValid but intentionalDocument why it's intentional

When to Use Each#

ACCEPT: The reviewer correctly identified a real issue.

csv

2025-12-27,amelia/agents/developer.py,128,python-code-review:type-safety,type-safety,major,Return type list[Any] loses type safety,ACCEPT,Changed to list[AgentMessage]

REJECT: The reviewer was wrong - the code is correct.

csv

2025-12-23,amelia/drivers/api/openai.py,102,python-code-review:line-length,style,minor,Line too long (104 > 100),REJECT,ruff check passes - no E501 violation exists

DEFER: Valid issue but out of scope for current work.

csv

2025-12-22,api/handlers.py,45,fastapi-code-review:error-handling,error-handling,minor,Missing specific exception type,DEFER,Refactoring planned for Q1

ACKNOWLEDGE: Intentional design decision.

csv

2025-12-21,core/cache.py,89,python-code-review:optimization,patterns,minor,Using dict instead of dataclass,ACKNOWLEDGE,Performance-critical path - intentional

Rule Source Format#

Format: skill-name/section:rule-id or skill-name:rule-id

Examples:

• python-code-review/common-mistakes:unused-variables
• pydantic-ai-common-pitfalls:tool-decorator
• fastapi-code-review:dependency-injection
• pytest-code-review:fixture-scope

Use the skill folder name and identify the specific rule or section that triggered the issue.

Category Taxonomy#

CategoryDescriptionExamplestype-safetyType annotation issuesMissing types, incorrect types, Any usageasyncAsync/await issuesBlocking in async, missing awaiterror-handlingException handlingBare except, missing error handlingstyleCode style/formattingLine length, naming conventionspatternsDesign patternsAnti-patterns, framework misusetestingTest qualityMissing coverage, flaky testssecuritySecurity issuesInjection, secrets exposure

Writing Good Rationales#

For ACCEPT#

Explain what you fixed:

• "Changed Exception to (FileNotFoundError, OSError)"
• "Fixed using model_copy(update={...})"
• "Removed unused Any import"

For REJECT#

Explain why the issue is invalid:

• "ruff check passes - no E501 violation exists" (linter authoritative)
• "pydantic-ai docs explicitly support this pattern" (framework idiom)
• "Intentional optimization documented in code comment" (documented decision)

For DEFER#

Explain when/why it will be addressed:

• "Tracked in issue #123"
• "Refactoring planned for Q1"
• "Blocked on dependency upgrade"

For ACKNOWLEDGE#

Explain why it's intentional:

• "Performance-critical path per project conventions (e.g. AGENTS.md or CLAUDE.md)"
• "Legacy API compatibility requirement"
• "Matches upstream library pattern"

Example Log#

csv

date,file,line,rule_source,category,severity,issue,verdict,rationale
2025-12-20,tests/integration/test_cli_flows.py,407,pytest-code-review:parametrization,testing,minor,Unused extra_args parameter in parametrization,ACCEPT,Fixed - removed dead parameter
2025-12-20,tests/integration/test_cli_flows.py,237-242,pytest-code-review:coverage,testing,major,Missing review --local in git repo error test,REJECT,Not applicable - review uses different error path
2025-12-21,amelia/server/orchestrator/service.py,1702,python-code-review:immutability,patterns,critical,Direct mutation of frozen ExecutionState,ACCEPT,Fixed using model_copy(update={...})
2025-12-23,amelia/drivers/api/tools.py,48-53,pydantic-ai-common-pitfalls:tool-decorator,patterns,major,Misleading RunContext pattern - should use decorators,REJECT,pydantic-ai docs explicitly support passing raw functions with RunContext to Agent(tools=[])
2025-12-23,amelia/drivers/api/openai.py,102,python-code-review:line-length,style,minor,Line too long (104 > 100),REJECT,ruff check passes - no E501 violation exists
2025-12-27,amelia/core/orchestrator.py,190-191,python-code-review:exception-handling,error-handling,major,Generic exception handling in get_code_changes_for_review,ACCEPT,Changed Exception to (FileNotFoundError OSError)
2025-12-27,amelia/agents/developer.py,128,python-code-review:type-safety,type-safety,major,Return type list[Any] loses type safety,ACCEPT,Changed to list[AgentMessage] and removed unused Any import

Pre-Review Verification Checklist#

Before reporting ANY finding, reviewers MUST verify:

Verification Steps#

1. Confirm the issue exists: Read the actual code, don't infer from context
2. Check surrounding code: The issue may be handled elsewhere (guards, earlier checks)
3. Trace state/variable usage: Search for all references before claiming "unused"
4. Verify assertions: If claiming "X is missing", confirm X isn't present
5. Check framework handling: Many frameworks handle validation/errors automatically
6. Validate syntax understanding: Verify against current docs (Tailwind v4, TS 5.x, etc.)

Common False Positive Patterns#

PatternRoot CausePrevention"Unused variable"Variable used elsewhereSearch all references"Missing validation"Framework validatesCheck Pydantic/Zod/etc."Type assertion"Actually annotationConfirm as vs :"Memory leak"Cleanup existsCheck effect returns"Wrong syntax"New framework versionVerify against current docs"Style issue"Preference not ruleBoth approaches valid

Signals of False Positive Risk#

If you're about to flag any of these, double-check:

• "This variable appears unused" → Search for ALL references first
• "Missing error handling" → Check parent/framework handling
• "Should use X instead of Y" → Both may be valid
• "This syntax looks wrong" → Verify against current version docs

Reference: review-verification-protocol for full verification workflow.

How This Feeds Into Skill Improvement#

1. Aggregate by rule_source: Identify which rules have high REJECT rates
2. Analyze rationales: Find common themes in rejections
3. Update skills: Add exceptions, clarifications, or verification steps
4. Track impact: Measure if changes reduce rejection rate

See the review-skill-improver skill for the full analysis workflow.

Improvement Signals#

PatternSkill Improvement"linter passes" rejectionsAdd linter verification step before flagging style issues"docs support this" rejectionsAdd exception for documented framework patterns"intentional" rejectionsAdd codebase context check before flagging"wrong code path" rejectionsAdd code tracing step before claiming gaps