refactor(agent-personas): generalise discussion mode, fold review skills, drop Shape A/B jargon#479
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThis PR refactors the agent skill architecture from atomic, implicit multi-persona coordination to an explicit orchestrator pattern. It consolidates five specialized review skills into a unified ChangesAgent Skill Architecture Refactoring
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
@prisma-next/mongo-runtime
@prisma-next/family-mongo
@prisma-next/sql-runtime
@prisma-next/family-sql
@prisma-next/extension-arktype-json
@prisma-next/extension-cipherstash
@prisma-next/middleware-telemetry
@prisma-next/mongo
@prisma-next/extension-paradedb
@prisma-next/extension-pgvector
@prisma-next/postgres
@prisma-next/sql-orm-client
@prisma-next/sqlite
@prisma-next/target-mongo
@prisma-next/adapter-mongo
@prisma-next/driver-mongo
@prisma-next/contract
@prisma-next/utils
@prisma-next/config
@prisma-next/errors
@prisma-next/framework-components
@prisma-next/operations
@prisma-next/ts-render
@prisma-next/contract-authoring
@prisma-next/ids
@prisma-next/psl-parser
@prisma-next/psl-printer
@prisma-next/cli
@prisma-next/emitter
@prisma-next/migration-tools
prisma-next
@prisma-next/vite-plugin-contract-emit
@prisma-next/mongo-codec
@prisma-next/mongo-contract
@prisma-next/mongo-value
@prisma-next/mongo-contract-psl
@prisma-next/mongo-contract-ts
@prisma-next/mongo-emitter
@prisma-next/mongo-schema-ir
@prisma-next/mongo-query-ast
@prisma-next/mongo-orm
@prisma-next/mongo-query-builder
@prisma-next/mongo-lowering
@prisma-next/mongo-wire
@prisma-next/sql-contract
@prisma-next/sql-errors
@prisma-next/sql-operations
@prisma-next/sql-schema-ir
@prisma-next/sql-contract-psl
@prisma-next/sql-contract-ts
@prisma-next/sql-contract-emitter
@prisma-next/sql-lane-query-builder
@prisma-next/sql-relational-core
@prisma-next/sql-builder
@prisma-next/target-postgres
@prisma-next/target-sqlite
@prisma-next/adapter-postgres
@prisma-next/adapter-sqlite
@prisma-next/driver-postgres
@prisma-next/driver-sqlite
commit: |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.agents/skills/drive-discussion/SKILL.md:
- Around line 85-92: The three example blockquotes under "Example openings" (the
lines starting with "> In discussion mode with architect +
principal-engineer...", "> In discussion mode with pm...", and "> An assumption
seems to have been falsified mid-implementation...") have blank lines between
quoted lines which triggers MD028; remove the intervening blank lines so each
multi-line example is a single continuous blockquote (i.e., consecutive lines
each beginning with ">") and ensure there are no empty lines separating the ">"
lines to satisfy markdownlint.
- Around line 41-44: The SKILL.md currently contradicts itself by allowing
“silent” mid-thread persona switches in the “Cross-pollination is expected”
paragraph while also insisting persona loads be visible at every boundary (the
“Persona is the lens, not a phase…” / agent-personas library statement) — pick
one behavior and make the doc consistent: either remove or constrain the
“silent” switch language (the sentence starting with “Switch silently for the
subthread”) and replace it with an explicit visible-reload rule, or instead
update the visibility rule to allow short-lived, explicitly scoped persona
overrides (e.g., “persona overrides may be used for subthreads but must be
declared in the workflow body”) so both paragraphs align; ensure you reference
the same symbol names (`tech-lead`, “Cross-pollination is expected”, and
agent-personas) when editing so the intent is unambiguous.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 1d59f7b4-f3a6-4d74-bc1d-4f1c80e2e475
📒 Files selected for processing (9)
.agents/skills/drive-agent-personas/SKILL.md.agents/skills/drive-create-spec/SKILL.md.agents/skills/drive-design-discussion/SKILL.md.agents/skills/drive-discussion/SKILL.md.agents/skills/drive-pr-local-review/SKILL.md.agents/skills/drive-product-discussion/SKILL.md.agents/skills/review-implementation/SKILL.md.agents/skills/review-system-design/SKILL.md.agents/skills/review-walkthrough/SKILL.md
💤 Files with no reviewable changes (5)
- .agents/skills/review-system-design/SKILL.md
- .agents/skills/drive-product-discussion/SKILL.md
- .agents/skills/drive-design-discussion/SKILL.md
- .agents/skills/review-implementation/SKILL.md
- .agents/skills/review-walkthrough/SKILL.md
…witches
Removes the "switch silently for the subthread" wording, which
contradicted the visibility rule in the same section ("each persona load
is visible in the workflow body"). Subthread persona overrides must now
be declared in-line and the return to the driving lens must be explicit
too, aligning the operating rule with the rest of the agent-personas
library.
Addresses CodeRabbit review #479 thread A01.
The three Example-openings blockquotes were adjacent with blank lines
between them, which triggers markdownlint MD028 (no blanks inside
blockquote). They are alternatives, not one continuous example, so
merging them (CodeRabbit's suggestion) would conflate three distinct
framings. Instead, separate each blockquote with a short prose connector
("For an engineering design…", "Or, for a product framing:", "Or,
mid-implementation…") so the blockquotes are no longer adjacent and the
examples stay clearly distinct.
Addresses CodeRabbit review #479 thread A02.
…Shape A/B jargon Three coordinated changes to the agent-personas skill library: 1. Generalise drive-design-discussion → drive-discussion. The discussion mechanics (one-thread-at-a-time, response shape, read-only stance, explicit exit-and-document step) are the skill; the personas are inputs to it. Personas can be specified by the user or inferred from context and announced. Synthesis now requires an in-depth written summary that captures reasoning, assumptions, and rejected alternatives, plus a mandatory offer to document the outcome as a spec, plan, ADR, or amendment. Three worked examples (engineering design, product framing, mid-implementation falsified assumption) ground the configurable shape. 2. Fold the three review-* atomic sub-skills back into drive-pr-local-review. review-system-design, review-implementation, and review-walkthrough are inlined as numbered passes (§ 3 architect, § 4 principal-engineer, § 5 tech-lead) with explicit persona-load instructions at each transition. One place to find the review content trumps the decomposition. The composite still owns scope + expectations (§§ 1–2) and synthesis (§ 6). 3. Drop the Shape A / Shape B terminology. The labels were jargon that did not help readers and produced a false dichotomy. The drive-agent-personas skill now describes a single multi-persona pattern: orchestrator at top, explicit persona loads at workflow boundaries, orchestrator reloaded at synthesis. drive-discussion no longer announces its “shape”; the skill body is the contract. Also: cross-reference updates in drive-product-discussion and drive-create-spec to point at drive-discussion.
…scussion + pm persona) drive-discussion now generalises the discussion mode and accepts any persona from the library as input. Invoking it with the pm persona produces the same behaviour drive-product-discussion produced — the operating rules, response shape, and exit-and-document protocol are identical; only the lens changes. Worked example B in drive-discussion covers the product-framing case explicitly. Removing the preset keeps the surface honest: one mode skill, lens chosen at invocation time. Users who reach for "product mode" / "PM mode" are routed by drive-discussion's description (which names those triggers) and inferred-persona logic.
…witches
Removes the "switch silently for the subthread" wording, which
contradicted the visibility rule in the same section ("each persona load
is visible in the workflow body"). Subthread persona overrides must now
be declared in-line and the return to the driving lens must be explicit
too, aligning the operating rule with the rest of the agent-personas
library.
Addresses CodeRabbit review #479 thread A01.
The three Example-openings blockquotes were adjacent with blank lines
between them, which triggers markdownlint MD028 (no blanks inside
blockquote). They are alternatives, not one continuous example, so
merging them (CodeRabbit's suggestion) would conflate three distinct
framings. Instead, separate each blockquote with a short prose connector
("For an engineering design…", "Or, for a product framing:", "Or,
mid-implementation…") so the blockquotes are no longer adjacent and the
examples stay clearly distinct.
Addresses CodeRabbit review #479 thread A02.
15315ae to
5623b72
Compare
…witches
Removes the "switch silently for the subthread" wording, which
contradicted the visibility rule in the same section ("each persona load
is visible in the workflow body"). Subthread persona overrides must now
be declared in-line and the return to the driving lens must be explicit
too, aligning the operating rule with the rest of the agent-personas
library.
Addresses CodeRabbit review #479 thread A01.
…witches
Removes the "switch silently for the subthread" wording, which
contradicted the visibility rule in the same section ("each persona load
is visible in the workflow body"). Subthread persona overrides must now
be declared in-line and the return to the driving lens must be explicit
too, aligning the operating rule with the rest of the agent-personas
library.
Addresses CodeRabbit review #479 thread A01.
The three Example-openings blockquotes were adjacent with blank lines
between them, which triggers markdownlint MD028 (no blanks inside
blockquote). They are alternatives, not one continuous example, so
merging them (CodeRabbit's suggestion) would conflate three distinct
framings. Instead, separate each blockquote with a short prose connector
("For an engineering design…", "Or, for a product framing:", "Or,
mid-implementation…") so the blockquotes are no longer adjacent and the
examples stay clearly distinct.
Addresses CodeRabbit review #479 thread A02.
At a glance
Follow-up to #447. Three coordinated simplifications to the persona-library skill surface, prompted by use of the M3-landed skills and feedback from the ignite team (see the parallel ignite/#84).
A skill that wants the discussion mode now looks like this:
— and the agent stress-tests an idea through those lenses, then offers to document the outcome as a spec, plan, ADR, or amendment. Personas are inputs to the skill, not part of its identity.
The changes
1.
drive-design-discussion→drive-discussionThe discussion mechanics (one-thread-at-a-time, response shape, read-only stance, explicit exit-and-document step) are the skill; the personas are inputs. Personas can be specified by the user ("discussion mode with pm") or inferred from context and announced ("loading architect + principal-engineer based on the framing — say so if you want a different lens").
Synthesis now requires an in-depth written summary (not a one-paragraph wrap-up) that captures:
Followed by a mandatory offer to document the summary as a project spec, plan, ADR, or amendment. Discussions that produce real understanding deserve real artefacts.
Three worked examples (engineering design, product framing, mid-implementation falsified assumption) ground the configurable shape.
2. Fold the three review-* sub-skills back into
drive-pr-local-reviewreview-system-design,review-implementation, andreview-walkthrough(which #447 introduced as atomic single-persona sub-skills) are inlined as numbered passes indrive-pr-local-review:system-design-review.mdcode-review.mdwalkthrough.mdEach transition is an explicit persona-load instruction. The skill still owns scope + expectations (§§ 1–2) and synthesis (§ 6).
One place to find the review content trumps the decomposition; in practice the three passes are always run together for a meaningful PR review, and the decomposition added file-jumping friction without producing reusable surfaces.
3. Drop the Shape A / Shape B terminology
The labels were jargon that did not help readers and produced a false dichotomy.
drive-agent-personasnow describes a single multi-persona pattern:drive-discussionno longer announces its "shape"; the skill body is the contract.4. Delete
drive-product-discussiondrive-discussioninvoked with thepmpersona produces exactly whatdrive-product-discussionproduced — same operating rules, same response shape, same exit-and-document protocol; only the lens differs. Worked example B indrive-discussioncovers the product-framing case explicitly, anddrive-discussion's description names"product mode"/"pm mode"as triggers so users routing to product framing still land in the right place.Removing the preset keeps the surface honest: one mode skill, lens chosen at invocation time. Shipping per-persona presets would balloon the surface for no behavioural gain.
What this PR contains
Test plan
drive-discussionenters cleanly on each documented trigger phrase (discussion mode,tech design mode,product mode,pm mode,challenge my idea,pressure-test this).drive-discussionwith user-specified personas honours the named set exactly; with no personas named, infers and announces.drive-discussionsynthesis produces the in-depth summary (with reasoning, assumptions, rejected alternatives) and offers to document.drive-pr-local-reviewproduces all three artefacts side-by-side on a real PR, with explicit persona transitions visible in the workflow.Shape A/Shape B,drive-design-discussion,drive-product-discussion,review-system-design,review-implementation, orreview-walkthroughin.agents/skills/.Alternatives considered
drive-design-discussionanddrive-product-discussionas separate skills, each with their personas baked in. Tempting because it avoids touching the existing skills, but produces N skills competing for the same invocation surface ("discuss this" / "pressure-test this") with subtly different framings. Configurable personas-at-invocation is the load-bearing simplification — one skill, lens chosen at invocation time.drive-product-discussion,drive-architect-discussion, …) as thin wrappers. Removeddrive-product-discussionprecisely because this approach scales badly — every new persona would want a preset, and the configurable-lens shape already covers the use case.Summary by CodeRabbit