Skip to content

Retrospective (Codex): tighten PR review workflow skill + add respond-to-reviewer skill #441

@cmungall

Description

@cmungall

Context

Retrospective from Codex on review/iteration workflow while finishing merged PR #398 ("Add Glutaric Aciduria (GA1) disorder entry").

I (Codex) needed several rounds of back-and-forth with reviewer comments before converging. The core problem was not only content quality, but process: triage, closure tracking, and schema-aware reviewer response discipline.

What Went Wrong (from Codex perspective)

  1. I did not build a complete unresolved-comment checklist up front and execute in one closure pass.
  2. I made a "validation-passing" fix (ellipsis snippets) that created reviewer friction and then had to be undone.
  3. I missed a slot/ontology mismatch (GO:0004361 MF term placed under biological_processes) until reviewer feedback.
  4. I missed canonical term-label casing consistency (GO:0009063 preferred term case mismatch) until review caught it.
  5. I treated reviewer feedback as incremental edits rather than a structured response workflow with explicit "resolved/not resolved" state.

Proposal A: Strengthen .claude/skills/dismech-pr-review/SKILL.md

Add a mandatory review-response workflow section with concrete steps:

  1. Collect & Classify All Reviewer Items First
  • Pull all unresolved comments from the target PR.
  • Group by severity: Critical, Medium, Minor.
  • Group by type: schema/ontology, evidence snippet, modeling, wording.
  1. Create a Resolution Table Before Editing
  • Columns: comment_id, severity, request, planned_action, accept/pushback, status.
  • Require explicit rationale for any pushback.
  1. Hard Guards (do not skip)
  • Verify GO namespace placement (MF/BP/CC) against the schema slot semantics.
  • Verify preferred_term exactly matches canonical ontology label (including case).
  • For URL-based evidence: ensure snippet matches cached source text exactly (including markup tokens when relevant).
  • Avoid introducing ... snippet edits unless explicitly justified; prefer exact source substrings.
  1. Validation Gate Before Commit
  • just validate <file>
  • just validate-terms <file>
  • just validate-references <file>
  • Include a short pass/fail summary in response to reviewer.
  1. Reviewer-Reply Template
    For each critical comment:
  • Comment
  • Action taken
  • File:line
  • Validation result
  • If not accepted: rationale

Proposal B: Add New Skill: respond-to-reviewer

Create a dedicated skill focused on end-to-end reviewer comment closure.

Goal

Turn reviewer comments into a deterministic execution pipeline with explicit closure evidence.

Trigger

Use when user asks to:

  • address PR comments/reviews,
  • check unresolved comments,
  • respond to Claude/Copilot reviewer feedback,
  • determine if review concerns are fully addressed.

Workflow

  1. Fetch comments + unresolved threads.
  2. Build machine-readable checklist (severity, owner, acceptance decision).
  3. Apply edits in priority order.
  4. Run validation/test gates.
  5. Generate structured reply draft per comment with file/line refs.
  6. Re-check unresolved comments after push.

Required Outputs

  • review_response.md artifact (or terminal summary) containing:
    • unresolved count before/after,
    • per-comment disposition,
    • validation commands + outcomes,
    • explicit remaining risks/open questions.

Exit Criteria

  • No unresolved critical comments, or
  • explicit, documented pushback for remaining comments,
  • validation gates pass for touched files.

Why this matters

This would reduce churn, reduce reviewer fatigue, and improve first-pass closure rate on curation-heavy PRs like #398.

Metadata

Metadata

Assignees

No one assigned

    Labels

    documentationImprovements or additions to documentationenhancementNew feature or request

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions