Skip to content

feat: redesign document-review skill with persona-based review#359

Merged
tmchow merged 15 commits intomainfrom
feat/plan-review-personas
Mar 24, 2026
Merged

feat: redesign document-review skill with persona-based review#359
tmchow merged 15 commits intomainfrom
feat/plan-review-personas

Conversation

@tmchow
Copy link
Copy Markdown
Collaborator

@tmchow tmchow commented Mar 24, 2026

Summary

The existing document-review skill uses a single-voice reviewer that tries to evaluate planning documents across all dimensions simultaneously. This produces shallow, unfocused feedback -- the reviewer context-switches between product strategy, technical feasibility, design completeness, security, scope, and consistency without going deep on any of them.

This PR replaces that with a multi-persona pipeline that dispatches specialized reviewer agents in parallel, each focused on a single analytical lens:

Always-on personas (run on every document)

  • coherence-reviewer -- hunts contradictions, terminology drift, structural issues, broken internal references, and unresolved dependency gaps
  • feasibility-reviewer -- evaluates whether the plan survives contact with reality: architecture conflicts, shadow path tracing (happy/nil/empty/error for every data flow), dependency gaps, migration risks, implementability

Conditional personas (activated by content signals in the document)

  • product-lens-reviewer -- challenges problem framing with premise inversion (Munger), trajectory checking, goal-requirement alignment, and prioritization coherence
  • design-lens-reviewer -- dimensional 0-10 rating across information architecture, interaction states, user flows, responsive/a11y, and unresolved decisions; includes AI slop detection
  • security-lens-reviewer -- plan-level security architecture: attack surface inventory, auth/authz gaps, data exposure, trust boundaries, and a lightweight threat model
  • scope-guardian-reviewer -- challenges unnecessary abstractions, premature frameworks, scope-goal misalignment, and priority dependency issues

Pipeline architecture

The orchestrator skill (SKILL.md) handles the full lifecycle:

  1. Classify the document type and select conditional personas based on content signals
  2. Dispatch all selected agents in parallel using fully-qualified namespace references
  3. Synthesize findings through confidence gating, fingerprint-based dedup, cross-persona promotion of corroborated below-threshold findings, and contradiction resolution
  4. Route findings into two buckets: auto-fixes (applied inline) and everything else (presented grouped by P0-P3 severity)
  5. Offer iterative refinement or "Review complete" terminal signal

Reference files

  • findings-schema.json -- JSON schema for structured findings with 2-class autofix routing (auto/present)
  • subagent-template.md -- shared prompt template with variable slots for consistent agent dispatch
  • review-output-template.md -- markdown output format for the synthesized review

Key design decisions

  • No <examples> blocks -- these agents are always dispatched by the orchestrator, never auto-triggered, so triggering examples waste tokens
  • Terse agent prompts (~40-50 lines each) -- inspired by iterative-engineering's coherence-reviewer pattern; every line earns its place
  • Content-based activation centralized in the skill, not in agent descriptions -- keeps agents focused on analysis, not self-selection
  • 2-class autofix routing (auto/present) -- findings are either deterministic fixes the orchestrator applies without asking, or they require user judgment. No middle ground needed for document review.
  • Backward-compatible terminal signal -- "Review complete" preserved for all 4 callers (ce:plan, ce:plan-beta, ce:brainstorm, ce:work)

Test plan

  • Run bun run release:validate to verify plugin/marketplace consistency
  • Invoke document-review skill on a requirements document and verify conditional persona selection
  • Invoke document-review skill on a technical plan and verify all 6 personas activate
  • Verify findings output matches review-output-template format
  • Verify "Review complete" terminal signal is emitted

tmchow added 11 commits March 23, 2026 20:57
Define requirements for replacing document-review with a persona-based
review pipeline. 2 always-on personas (coherence, feasibility) + 4
conditional (product-lens, design-lens, security-lens, scope-guardian)
with hybrid action model (auto-fix quality issues, present strategic
questions for user decision).
Implementation plan with 4 units: create always-on agents (coherence,
feasibility), conditional agents (product-lens, design-lens,
security-lens, scope-guardian), rewrite document-review SKILL.md with
persona pipeline orchestration, and update README. Includes structured
findings schema, fingerprint-based dedup, residual concern promotion,
and per-persona confidence calibration.
… to review agents

Product-lens gains failure-mode inversion (Munger) and trajectory check;
feasibility gains systematic 4-path tracing for data flows.
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 73b8b9d610

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread plugins/compound-engineering/skills/document-review/SKILL.md
tmchow added 2 commits March 23, 2026 21:45
Drop the 4-class enum (safe_auto/gated_auto/manual/advisory) carried
for hypothetical ce:review-beta compatibility. Document review only
needs two buckets: apply automatically or present to user. Also removes
owner and requires_verification fields from the schema.
Address PR review feedback: Phase 3.1 was listing a subset of required
fields instead of referencing the schema definition, which could miss
validating autofix_class.
@tmchow tmchow changed the title feat: persona-based document review pipeline feat: redesign document-review skill with persona-based review Mar 24, 2026
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a8afb8ac5e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread plugins/compound-engineering/skills/document-review/SKILL.md
tmchow added 2 commits March 23, 2026 22:07
Address PR review feedback:
- Dedup now skips merging when findings recommend opposing actions,
  preserving both for contradiction resolution
- Auto findings without suggested_fix get demoted to present
Set coherence-reviewer to model: haiku -- its work (terminology drift,
contradiction detection, cross-reference checking) is pattern matching
that doesn't benefit from larger models. Fix converters to omit model
field when inherit instead of emitting literal string (droid, copilot).
Update OpenCode sonnet alias to claude-sonnet-4-6.
@tmchow tmchow force-pushed the feat/plan-review-personas branch from 7acdc24 to 4df757b Compare March 24, 2026 08:50
@tmchow tmchow merged commit 18d22af into main Mar 24, 2026
2 checks passed
@github-actions github-actions Bot mentioned this pull request Mar 24, 2026
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4df757b46b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +80 to 82
if (agent.model && agent.model !== "inherit") {
frontmatter.model = agent.model
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve model: inherit when converting droids

Factory droids treat model: inherit as a distinct runtime choice (use the parent session model), but this change drops the model field whenever a Claude agent is marked inherit. That means converted droids no longer explicitly inherit and can fall back to Factory defaults, causing behavior/cost drift for plugins that rely on parent-model alignment. Keep emitting model: "inherit" for this target instead of omitting it.

Useful? React with 👍 / 👎.

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