mirror of https://github.com/garrytan/gstack.git
feat: integrate design review lite into /review and /ship
Add generateDesignReviewLite() resolver, insert {{DESIGN_REVIEW_LITE}}
partial in review Step 4.5 and ship Step 3.5. Update dashboard to
recognize design-review-lite entries. Ship pre-flight uses
gstack-diff-scope for smarter design review recommendations.
This commit is contained in:
parent
885640c489
commit
faf80a7a9d
|
|
@ -75,6 +75,14 @@ Follow the output format specified in the checklist. Respect the suppressions
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
## Step 4.5: Design Review (conditional)
|
||||||
|
|
||||||
|
{{DESIGN_REVIEW_LITE}}
|
||||||
|
|
||||||
|
Include any design findings alongside the findings from Step 4. They follow the same Fix-First flow in Step 5 — AUTO-FIX for mechanical CSS fixes, ASK for everything else.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
## Step 5: Fix-First Review
|
## Step 5: Fix-First Review
|
||||||
|
|
||||||
**Every finding gets action — not just critical ones.**
|
**Every finding gets action — not just critical ones.**
|
||||||
|
|
|
||||||
|
|
@ -519,6 +519,45 @@ Minimum 0 per category.
|
||||||
11. **Show screenshots to the user.** After every \`$B screenshot\`, \`$B snapshot -a -o\`, or \`$B responsive\` command, use the Read tool on the output file(s) so the user can see them inline. For \`responsive\` (3 files), Read all three. This is critical — without it, screenshots are invisible to the user.`;
|
11. **Show screenshots to the user.** After every \`$B screenshot\`, \`$B snapshot -a -o\`, or \`$B responsive\` command, use the Read tool on the output file(s) so the user can see them inline. For \`responsive\` (3 files), Read all three. This is critical — without it, screenshots are invisible to the user.`;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function generateDesignReviewLite(): string {
|
||||||
|
return `## Design Review (conditional, diff-scoped)
|
||||||
|
|
||||||
|
Check if the diff touches frontend files using \`gstack-diff-scope\`:
|
||||||
|
|
||||||
|
\`\`\`bash
|
||||||
|
eval $(~/.claude/skills/gstack/bin/gstack-diff-scope <base> 2>/dev/null)
|
||||||
|
\`\`\`
|
||||||
|
|
||||||
|
**If \`SCOPE_FRONTEND=false\`:** Skip design review silently. No output.
|
||||||
|
|
||||||
|
**If \`SCOPE_FRONTEND=true\`:**
|
||||||
|
|
||||||
|
1. **Check for DESIGN.md.** If \`DESIGN.md\` or \`design-system.md\` exists in the repo root, read it. All design findings are calibrated against it — patterns blessed in DESIGN.md are not flagged. If not found, use universal design principles.
|
||||||
|
|
||||||
|
2. **Read \`.claude/skills/review/design-checklist.md\`.** If the file cannot be read, skip design review with a note: "Design checklist not found — skipping design review."
|
||||||
|
|
||||||
|
3. **Read each changed frontend file** (full file, not just diff hunks). Frontend files are identified by the patterns listed in the checklist.
|
||||||
|
|
||||||
|
4. **Apply the design checklist** against the changed files. For each item:
|
||||||
|
- **[HIGH] mechanical CSS fix** (\`outline: none\`, \`!important\`, \`font-size < 16px\`): classify as AUTO-FIX
|
||||||
|
- **[HIGH/MEDIUM] design judgment needed**: classify as ASK
|
||||||
|
- **[LOW] intent-based detection**: present as "Possible — verify visually or run /qa-design-review"
|
||||||
|
|
||||||
|
5. **Include findings** in the review output under a "Design Review" header, following the output format in the checklist. Design findings merge with code review findings into the same Fix-First flow.
|
||||||
|
|
||||||
|
6. **Log the result** for the Review Readiness Dashboard:
|
||||||
|
|
||||||
|
\`\`\`bash
|
||||||
|
eval $(~/.claude/skills/gstack/bin/gstack-slug 2>/dev/null)
|
||||||
|
mkdir -p ~/.gstack/projects/$SLUG
|
||||||
|
echo '{"skill":"design-review-lite","timestamp":"TIMESTAMP","status":"STATUS","findings":N,"auto_fixed":M}' >> ~/.gstack/projects/$SLUG/$BRANCH-reviews.jsonl
|
||||||
|
\`\`\`
|
||||||
|
|
||||||
|
Substitute: TIMESTAMP = ISO 8601 datetime, STATUS = "clean" if 0 findings or "issues_found", N = total findings, M = auto-fixed count.`;
|
||||||
|
}
|
||||||
|
|
||||||
|
// NOTE: design-checklist.md is a subset of this methodology for code-level detection.
|
||||||
|
// When adding items here, also update review/design-checklist.md, and vice versa.
|
||||||
function generateDesignMethodology(): string {
|
function generateDesignMethodology(): string {
|
||||||
return `## Modes
|
return `## Modes
|
||||||
|
|
||||||
|
|
@ -865,7 +904,7 @@ echo "---CONFIG---"
|
||||||
~/.claude/skills/gstack/bin/gstack-config get skip_eng_review 2>/dev/null || echo "false"
|
~/.claude/skills/gstack/bin/gstack-config get skip_eng_review 2>/dev/null || echo "false"
|
||||||
\`\`\`
|
\`\`\`
|
||||||
|
|
||||||
Parse the output. Find the most recent entry for each skill (plan-ceo-review, plan-eng-review, plan-design-review). Ignore entries with timestamps older than 7 days. Display:
|
Parse the output. Find the most recent entry for each skill (plan-ceo-review, plan-eng-review, plan-design-review, design-review-lite). Ignore entries with timestamps older than 7 days. For Design Review, show whichever is more recent between \`plan-design-review\` (full visual audit) and \`design-review-lite\` (code-level check). Append "(FULL)" or "(LITE)" to the status to distinguish. Display:
|
||||||
|
|
||||||
\`\`\`
|
\`\`\`
|
||||||
+====================================================================+
|
+====================================================================+
|
||||||
|
|
@ -1056,6 +1095,7 @@ const RESOLVERS: Record<string, () => string> = {
|
||||||
BASE_BRANCH_DETECT: generateBaseBranchDetect,
|
BASE_BRANCH_DETECT: generateBaseBranchDetect,
|
||||||
QA_METHODOLOGY: generateQAMethodology,
|
QA_METHODOLOGY: generateQAMethodology,
|
||||||
DESIGN_METHODOLOGY: generateDesignMethodology,
|
DESIGN_METHODOLOGY: generateDesignMethodology,
|
||||||
|
DESIGN_REVIEW_LITE: generateDesignReviewLite,
|
||||||
REVIEW_DASHBOARD: generateReviewDashboard,
|
REVIEW_DASHBOARD: generateReviewDashboard,
|
||||||
TEST_BOOTSTRAP: generateTestBootstrap,
|
TEST_BOOTSTRAP: generateTestBootstrap,
|
||||||
};
|
};
|
||||||
|
|
|
||||||
|
|
@ -69,7 +69,8 @@ If the Eng Review is NOT "CLEAR":
|
||||||
- Show that Eng Review is missing or has open issues
|
- Show that Eng Review is missing or has open issues
|
||||||
- RECOMMENDATION: Choose C if the change is obviously trivial (< 20 lines, typo fix, config-only); Choose B for larger changes
|
- RECOMMENDATION: Choose C if the change is obviously trivial (< 20 lines, typo fix, config-only); Choose B for larger changes
|
||||||
- Options: A) Ship anyway B) Abort — run /plan-eng-review first C) Change is too small to need eng review
|
- Options: A) Ship anyway B) Abort — run /plan-eng-review first C) Change is too small to need eng review
|
||||||
- If CEO/Design reviews are missing, mention them as informational ("CEO Review not run — recommended for product changes") but do NOT block or recommend aborting for them
|
- If CEO Review is missing, mention as informational ("CEO Review not run — recommended for product changes") but do NOT block
|
||||||
|
- For Design Review: run `eval $(~/.claude/skills/gstack/bin/gstack-diff-scope <base> 2>/dev/null)`. If `SCOPE_FRONTEND=true` and no design review (plan-design-review or design-review-lite) exists in the dashboard, mention: "Design Review not run — this PR changes frontend code. The lite design check will run automatically in Step 3.5, but consider running /plan-design-review for a full visual audit." Still never block.
|
||||||
|
|
||||||
3. **If the user chooses A or C,** persist the decision so future `/ship` runs on this branch skip the gate:
|
3. **If the user chooses A or C,** persist the decision so future `/ship` runs on this branch skip the gate:
|
||||||
```bash
|
```bash
|
||||||
|
|
@ -334,6 +335,10 @@ Review the diff for structural issues that tests don't catch.
|
||||||
- **Pass 1 (CRITICAL):** SQL & Data Safety, LLM Output Trust Boundary
|
- **Pass 1 (CRITICAL):** SQL & Data Safety, LLM Output Trust Boundary
|
||||||
- **Pass 2 (INFORMATIONAL):** All remaining categories
|
- **Pass 2 (INFORMATIONAL):** All remaining categories
|
||||||
|
|
||||||
|
{{DESIGN_REVIEW_LITE}}
|
||||||
|
|
||||||
|
Include any design findings alongside the code review findings. They follow the same Fix-First flow below.
|
||||||
|
|
||||||
4. **Classify each finding as AUTO-FIX or ASK** per the Fix-First Heuristic in
|
4. **Classify each finding as AUTO-FIX or ASK** per the Fix-First Heuristic in
|
||||||
checklist.md. Critical findings lean toward ASK; informational lean toward AUTO-FIX.
|
checklist.md. Critical findings lean toward ASK; informational lean toward AUTO-FIX.
|
||||||
|
|
||||||
|
|
@ -555,7 +560,11 @@ gh pr create --base <base> --title "<type>: <summary>" --body "$(cat <<'EOF'
|
||||||
<If Step 3.4 ran: "Tests: {before} → {after} (+{delta} new)">
|
<If Step 3.4 ran: "Tests: {before} → {after} (+{delta} new)">
|
||||||
|
|
||||||
## Pre-Landing Review
|
## Pre-Landing Review
|
||||||
<findings from Step 3.5, or "No issues found.">
|
<findings from Step 3.5 code review, or "No issues found.">
|
||||||
|
|
||||||
|
## Design Review
|
||||||
|
<If design review ran: "Design Review (lite): N findings — M auto-fixed, K skipped. AI Slop: clean/N issues.">
|
||||||
|
<If no frontend files changed: "No frontend files changed — design review skipped.">
|
||||||
|
|
||||||
## Eval Results
|
## Eval Results
|
||||||
<If evals ran: suite names, pass/fail counts, cost dashboard summary. If skipped: "No prompt-related files changed — evals skipped.">
|
<If evals ran: suite names, pass/fail counts, cost dashboard summary. If skipped: "No prompt-related files changed — evals skipped.">
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue