feat(gcp-to-aws): decision report — shared exec core rendered at gate A (stacked on #185) - #186
feat(gcp-to-aws): decision report — shared exec core rendered at gate A (stacked on #185)#186herosjourney wants to merge 17 commits into
Conversation
… product, Generate is opt-in - estimate.md: workshop offer becomes a 3-option Decision gate (done for now / what-ifs / generate). Verdict, cost (with baseline-quality label), timeline band, and deferred items presented from existing estimate artifacts. Option A ends the run decision-complete; C opts into Generate. - SKILL.md: 'Generate is opt-in' hard rule (gate C, resume acceptance, or explicit Terraform request — never auto-chain); generate state-machine row now requires consent; new decide-complete row with resume offer; ordered evaluation respects consent; feedback checkpoint re-anchored to fire after gate resolution (decision-check lands right after the actual decision). - workshop: exits return to the Decision gate, not Generate (workshop-assemble, workshop.md branch + decline, sheet action label); canonical workshop-invariants exit rule generalized (skill may define a post-Estimate gate; heroku/vercel behavior unchanged) + vendored resync. - schema-phase-status.md: optional run_mode field (decide | decide_and_execute); decide-complete documented as terminal-unless-asked (generate stays pending — no 'skipped' status).
1. State-machine workshop row: 'workshop offer' -> Decision gate (stale wording would regress agents to the old Enter/Proceed prompt) 2. Estimate scope boundary: carve out the gate's one-line timeline band; full schedules stay Generate-only 3. Every Execute path (gate C, resume Yes, explicit ask) sets run_mode: decide_and_execute BEFORE loading generate.md, so a death mid-Generate resumes as an Execute run 4. Consent guard at the top of generate.md — refuses and re-presents the gate when loaded cold without consent 5. Canonical invariants exit rule tightened: skills with a gate NEVER auto-advance to Generate (vendored resynced) 6. handoff-gates.md: decide-complete documented as a valid terminal state, not a failure to repair 7. Soft: after gate A, feedback folds into the closing line as a one-line Yes/No instead of a separate phase-like prompt
…d at gate A (stacked on decision gate) - Extract the report's executive summary (Sections 0-8: verdict, TCO, cost comparison with baseline-quality/not-comparable rules, timeline, risks, assumptions) into references/shared/report-decision-core.md — single source of truth rendered in two modes: decision (gate A) and full (Generate Step 1 loads it; do-not-restate rule) - estimate.md gate choice A now writes decision-report.html + DECISION.md from the core (no appendices, CTA footer, pre-Generate fallbacks for timeline/diagram), validated before presenting - Replace-never-patch: Execute renders migration-report.html fresh from artifacts; never splices the decision report's HTML - validate-migration-report.py --mode decision: exec required set + decision-cta, appendix-* sections forbidden; full-mode contract and REPORT_OK format byte-identical (4 new tests; 36 pass) - New golden fixture gcp-decision-gate/after-decide-complete locking the decide-complete terminal state (current_phase complete + run_mode decide + generate pending) and validator-passing decision pack (7 asserters pass)
…ss call mise run build failed at security:bandit (B404/B603, low severity) on the new fixture asserter. Applied the house pattern from run-asserters.py: inline nosec with justification (list args, no shell, committed validator path via sys.executable only). Full mise run build now exits 0.
…round 2)
The preamble declared decision fallbacks while the moved body still
commanded Generate-first sources — under-specified mode overrides, not a
verbatim move. Fixes:
- Inline '_Decision mode:_' overrides next to each affected rule, declared
authoritative in the preamble: decision-summary item 4 + Section 6
timeline (timeline_hint -> complexity band, labeled 'if you execute',
never invent weeks, omit effort hours), Section 2b (diagram from
aws-design.json clusters only, omit cluster_order), Section 7 risks
(ordered decision sources: recommendation conditions/would_flip_if ->
deferred rows -> material defaulted assumptions; never leave exec-risks
empty when those exist)
- Decision-inappropriate Generate copy split by mode: cost footnote ('if
you generate the execution pack...'), security teaser Appendix G
pointers, startup-program validate script marked full-mode-only
- Anti-stub / don't-truncate-appendices prose marked full-mode-only for
the appendix clauses
- estimate.md gate A validate command uses absolute $PLUGIN_ROOT /
$MIGRATION_DIR paths (cwd not load-bearing)
- Golden asserter now locks content, not just structure: verdict-headline
(awslabs#173), the not-comparable sentence (awslabs#175), and the 'if you execute'
timeline label
|
Round-2 fixes landed in 1ba9ffb — main ask and all four smaller items, plus the description correction: Fallback problem (main ask): conceded fully — "under-specified mode overrides" is the right diagnosis, and the preamble-only approach was the mistake. Fixed per your suggested patch, inline next to each rule:
Smaller items: (1) cost footnote, security teaser, and the startup-program script line split by mode ("If you generate the execution pack, its Terraform will align with the Balanced scenario…"); (2) anti-stub and don't-truncate-appendices prose marked full-mode-only for the appendix clauses; (3) golden asserter upgraded from state-lock to content-lock — asserts "Verbatim" overclaim: agreed and corrected in the PR description — full-mode rules relocated unchanged; decision-mode overrides are new normative text, now inline per section. Re-verified: full |
Make the richer card-based shell a validated contract so full and decision reports cannot regress to sparse, unstructured output. Co-authored-by: Cursor <cursoragent@cursor.com>
Keep recommendations and supported savings visually distinct, and attach the official Activate application link directly to the executive benefit. Co-authored-by: Cursor <cursoragent@cursor.com>
Name recurring cloud charges accurately, enforce a scannable glossary, and prevent AI coexistence from inflating infrastructure effort. Co-authored-by: Cursor <cursoragent@cursor.com>
… metric hierarchy, data-first, risk matrix, nav aids Validated on a real generated report (previewed against run 0724-0602, user-approved) before speccing. Re-applied on top of the visual-contract commits (f4bc5bf..e1a7758) that landed on this branch in parallel; three collisions resolved in favor of the approved preview: - Tables: horizontal rules only (never vertical cell borders — grid + zebra + hover is three redundant separation cues) + thead 2px anchor; new td.num/th.num right-align + tabular-nums, REQUIRED on every currency/hours/weeks/percentage column (readability convention 11) - Metric grid: .metric-hero for the 1-2 primary decision metrics (run rate, timeline) so the reader never ranks the numbers themselves; max ~5 cards; Activate is NEVER a metric card (call-to-action, not a measurement) — always the callout, apply link inside (supersedes the parallel commits' Activate-card guidance; keeps their link-required validator check satisfied via the callout inside decision-summary) - Data first, explanation adjacent (convention 10): tier reading guide becomes details.reading-guide AFTER the table; mandatory not-comparable caveat renders as a .chip-warn pill on the GCP metric, not a paragraph between heading and numbers - Corrections in disclosures (convention 12): corrected value + one clause visible; how-we-got-here narrative in details.why - Top Risks: Impact / Likelihood as separate badge columns, never 'Critical impact (low probability)' prose - Nav aids: nav.toc id=toc + per-h2 'up contents' toplink (print-hidden) - Golden fixture demonstrates all conventions it locks (42 tests, 7 asserters, full mise build exit 0)
|
Added: report readability conventions (3bd3292) — previewed against a real generated report (run 0724-0602) and user-approved before speccing. Seven conventions, all applying to both render modes via the shared core:
Note for reviewers — this commit reconciles with the parallel visual-contract commits (f4bc5bf…e1a7758) that landed on this branch while it was being written. Two of their choices were superseded in favor of the user-approved preview:
Everything else from those commits (visual-contract validator checks, card-based section styling, responsive rules) is preserved and the new conventions build on it. Verified: 42 tests, 7 asserters, dprint/markdownlint clean, full |
…ation principle Extracted from Anthropic's frontend-design skill where its principles fit a validator-enforced credibility document (its distinctiveness/aesthetic-risk core deliberately rejected — every report should look like the same trustworthy product): - The hero is the thesis: page opens title -> decision-summary (verdict + hero metrics) -> TOC -> rest. Previously an 18-link two-column menu sat between the title and the verdict — the first screenful was navigation chrome, not the decision. Encoded in the core's TOC placement rule, the HTML sketch, and the decision-mode shell; fixture reordered to match. - Deliberate five-step type scale (~1.25 ratio) documented in the CSS contract typography block — new elements pick the nearest step, no invented in-between sizes. - 'Structure is information, not decoration' added as the governing principle above the readability conventions — the test future devices must pass, with the existing rules named as its applications.
|
Added (4ff2531): hero-first opening + type scale + governing principle — extracted from Anthropic's frontend-design skill, taking only what fits a validator-enforced credibility document and explicitly rejecting its distinctiveness/aesthetic-risk core (every report should look like the same trustworthy product, not "a visual identity that could not be mistaken for anyone else's").
Verified: 42 tests, 7 asserters, dprint/markdownlint clean, full |
… in the visual contract Two guardrail lines (no visible change to current renders — they convert passing-by-luck into passing-by-rule, same move as the type scale): - Contrast floor: every text/background pair meets 4.5:1 (3:1 large display text), incl. muted captions and badge text on tinted backgrounds; never trade contrast for a softer look - Interaction discipline: cursor:pointer + 150-300ms hover/focus transitions on interactive elements; entrance/scroll/ambient animations explicitly banned (AI-slop tell in a document; no-op in print)
|
Added (c88d424): two guardrail lines in the visual contract, extracted from a second frontend-design skill review — the 4.5:1 WCAG AA contrast floor (3:1 for large display text; current palette passes, now it's a rule instead of a coincidence) and interaction discipline ( |
… gates, active-voice and denominator rules Encode writing-style rules into the report pipeline: - Spec: five new readability conventions (13-17) — no vague intensifiers, ISO dates only, color never the sole signal, active voice with named actor, denominators on progress metrics - Validator: enforce the two mechanically checkable rules (intensifiers, N/N/YYYY slash dates) as readability gates - Tests: 4 new cases incl. discovery/recovery/every substring false-positive guard
…e + duration drivers
|
5caccbd adds the timeline/effort honesty work to this PR (per review with Grok — it lands here rather than on main because this branch owns the exec timeline rules in What changed:
Checks: dprint clean, lint:md 0 errors (806 files), frontmatter OK, shared:check OK (3 trees), 51 pytest (was 36 — includes the 4 decision-mode tests, all updated), 7 asserters PASS including the decide-complete golden. Acceptance greps: zero Grok's six review amendments are all in: verdict formula (approach + binding driver, no weeks/hours/bare tier), |
…ncretely change vs Balanced
|
One more fold-in (same honesty thread as the timeline commit): scenario_deltas — the tier table now itemizes what Premium and Optimized concretely change vs Balanced, per stack. Problem: the tier labels answer "are these three different builds?" (no — same architecture, three pricing scenarios) but not the decision question: what would I buy at Premium / give up at Optimized? The generic subtitle "reservations, Spot, or storage trade-offs assumed" hides real architectural consequences — e.g. on a live test stack, the Optimized total silently drops the NAT Gateway (~a third of the Balanced total), which is a network-posture change, not just savings. Readers quote the lowest number to their board without knowing it embeds a security change and a 1-year commitment. Changes:
Checks: 51 pytest, 7 asserters PASS, dprint/lint/shared:check clean. Scope note: this is the third concern on this PR (decision report core + timeline honesty + scenario deltas) — happy to split if reviewers prefer, but all three edit the same tier-table rules in |
…emotes the track, never the stack
|
Fourth fold-in (user-reported flaw in a live report render): track-scoped outcomes. Problem, plain English: Changes (66567cc):
Checks: 51 pytest, 7 asserters PASS, dprint/lint/shared:check clean. This is concern 4 on this PR — same offer stands to split (it edits the same verdict rules in report-decision-core.md, which is why it stacks). |
leon1418
left a comment
There was a problem hiding this comment.
[🤖 AI review 🤖]
Review Summary — PR #186
Scope: Four stacked concerns on the decision-report shared core: (1) report-decision-core.md extraction + decision gate rendering, (2) timeline/effort honesty (remove hours, demote weeks to structure+drivers), (3) scenario deltas (itemize tier differences), (4) track-scoped outcomes. 35 files, +1677/−555.
Empirical verification:
- Validator:
REPORT_OK | mode=decision | sections=7/7✅ - Validator (full mode on reference fixture):
REPORT_OK | structure=complete | sections=10/10✅ - pytest: 46/46 passed ✅
- Decision-gate asserter (
check_expected_decide.py): PASS ✅ - Acceptance greps: zero stray
effort_hours/total_weeksoutside legacy-handling; zero bareTCOin gcp-to-aws exec scope ✅
CI: All checks green (build, gitleaks, bandit, semgrep, checkov) as of 2026-08-04.
Design: The extraction approach (shared core, two render depths via mode flag) is the right call — it avoids two drifting copies of the #175 baseline-quality rules and #173 verdict typography, uses the same validator script for both, and keeps the canonical-vendored discipline. The "Generate is opt-in" hard rule is a genuine product improvement: the decision IS the product for many users; execution artifacts are a second explicit product.
Findings: Clean. No blocking issues. Two nits and one FYI below.
Merge recommendation: Merge #185 first (as noted), then this. No conflicts with #180–#184 that aren't covered by their BEHIND/MERGEABLE status. The vendored workshop-invariants changes (3 copies) correctly use the "skills that DEFINE a decision gate" conditional wording, which makes the heroku skill future-compatible without requiring it to implement the gate now.
| ( | ||
| r"Rubric:", | ||
| 'internal scoring trace ("Rubric:") — drop it or gate behind a ' | ||
| '<details> "Why this mapping?" block', |
There was a problem hiding this comment.
[🤖 AI review 🤖]
Nit: The bare-week-range lookbehind uses two fixed-width alternatives ((?<!uncalibrated\):\s)(?<!uncalibrated\): )) to approximate "preceded by the uncalibrated label". This handles exactly one space or one \s, but double-space or tab after the colon would be a false positive. Unlikely in practice since the label is prescribed text, but a single-pass approach like first stripping all labeled instances before scanning would be more robust.
# Alternative: pre-filter labeled instances
cleaned = re.sub(r'uncalibrated\):\s+\d+\s*(?:–|-|to)\s*\d+\+?\s*weeks', '', html)
# Then scan cleaned for bare rangesNot blocking — the current approach works for all plausible inputs.
|
|
||
| > ⚠️ **Specialist engagement required:** [service name] does not have an automated AWS mapping from this plugin. Engage your AWS account team and/or a data analytics migration partner to evaluate the best AWS analytics path. This does **not** block phased migration of other services; exclude [service name] from the combined estimated AWS monthly run rate until the target architecture is defined. | ||
|
|
||
| **Startup credits callout (decision summary / verdict):** |
There was a problem hiding this comment.
[🤖 AI review 🤖]
Nit: The numbering under Section 0 content uses 1a. twice (Recommendation callout and Per-track disposition line both labeled 1a.). The second should be 1b. (or the current 1b. Confidence pointer becomes 1c.). This is an authoring-reference label only (not rendered), but could confuse future editors of this spec.
| ## What REPORT_OK means | ||
|
|
||
| `REPORT_OK | structure=complete` means required sections, TOC links, appendix depth, readability rules, and artifact-driven cost/TCO checks passed. It does **not** verify that every dollar figure in the HTML matches the JSON — verify numerics manually or in a future accuracy gate before executive sign-off. | ||
| `REPORT_OK | structure=complete` means required sections, TOC links, appendix depth, readability rules, and artifact-driven cloud-service run-rate checks passed. It does **not** verify that every dollar figure in the HTML matches the JSON — verify numerics manually or in a future accuracy gate before executive sign-off. |
There was a problem hiding this comment.
[🤖 AI review 🤖]
FYI: The top-level README.md (line 40) still mentions "combined TCO" in the feature table. That's outside this PR's scope (different file, describes the whole repo), but worth a follow-up cleanup to stay consistent with the terminology change landed here.
Summary
Stacked on #185 (decision gate) — merge that first; this branch contains its commits. For this PR's isolated diff: compare
feat/decision-gate...feat/decision-report-core.This PR now carries four stacked concerns that all rewrite the same executive-section rules in the new shared renderer (
report-decision-core.md), which is why they land together rather than as sequential conflicting PRs: (1) the decision report itself, (2) timeline/effort honesty, (3) scenario deltas, (4) track-scoped outcomes. Each has its own plain-English problem statement below.1. Decision report — shared exec core rendered at gate A
The problem, in plain English: #185 gives the founder a decision moment, but the decision pack lives only in chat — close the window and the verdict, costs, and assumptions are gone. The durable report today is built inside Generate, so the only way to get a shareable artifact is to accept 20 Terraform files you didn't ask for. This gives gate choice A ("done for now") a real deliverable:
decision-report.html— the thing a founder emails their co-founder — plus a Slack-friendlyDECISION.md, without a second report engine to drift.references/shared/report-decision-core.md(new, extracted not written): the report's entire executive summary — verdict typography (feat(gcp-to-aws): decision outcomes (go/conditional/defer/stay) + executive trust panel #173), cost comparison with the baseline-quality badge and not-comparable rules (feat(gcp-to-aws): Design summary card + cost baseline-quality labels #175), combined run rate, migration shape, risks, assumptions panel, Activate wording — moved out ofgenerate-artifacts-report.mdStep 1 into a shared renderer spec with two modes. Full-mode rules relocated unchanged; decision-mode overrides are new normative text written inline next to each affected rule. Decision mode (gate A): exec sections only, pre-Generate fallbacks, a requireddecision-ctasection, and theDECISION.mdtwin. Full mode (Generate Step 1): loads the same file — Step 1 is now a pointer with a do-not-restate rule, the canonical-vendored discipline this repo adopted after the drift bug.generate-artifacts-report.mdshrinks 769 → 568 lines.estimate.mdgate A renders the pack, validates it, then sets the decide-complete state — the closing message names both files.migration-report.htmlrenders fresh from the JSON artifacts; splicing or extendingdecision-report.htmlis explicitly forbidden, and the artifacts catalog notes the full report supersedes it.validate-migration-report.py --mode decision: required set = exec sections +decision-cta; anyappendix-*section is a hard failure. Full mode's required set andREPORT_OKoutput format are byte-identical.gcp-decision-gate/after-decide-complete): locks the decide-complete terminal state from feat(gcp-to-aws): post-Estimate Decision gate — Generate becomes opt-in #185 and a validator-passing decision pack. Registered inrun-asserters.py.2. Timeline/effort honesty — remove hours, demote weeks to structure + drivers (
5caccbd)The problem, in plain English: the report labels every dollar figure with its provenance and accuracy band — but presented "480–720 engineering hours" and "12–16 weeks" with no label at all, as if measured. They aren't: the hours were weeks × 40 × an assumed utilization factor, the weeks came from authored tier buckets with no calibration data, and nothing downstream computed with the hours — they existed purely for display. These are the numbers most likely to be pasted into a budget or board deck, and the least defensible in the report.
generate-infra.md,generate-billing.md,generate-ai.md, report core, fixtures): no hour fields in any artifact schema, no effort metric cards. Legacy artifacts' hour fields are dropped outright — never rendered, even with a label.migration_plan.total_weeks→duration_drivers[]+tier_bound_by; stage templates keep their order (ordinal = real information) but lose week numbers;migration-complexity.mdgets a Provenance section and its Timeline/Effort tables become Approach + Duration Drivers tables. Operational time policies (24–48h watch, observation windows, maintenance windows) are explicitly kept — procedure, not estimates.timeline_hintrenamedduration_hintwith path-shape phrasing — renamed rather than semantically aliased; report core accepts the legacy key.total_weeksrender only with the visible label "Legacy planning heuristic (uncalibrated): N weeks."total_weekskept nullable with an owner note).3. Scenario deltas — itemize what Premium/Optimized concretely change vs Balanced (
80cace8)The problem, in plain English: the tier labels answer "are these three different builds?" (no — same architecture, three pricing scenarios) but not the decision question: what would I buy at Premium, and what would I give up at Optimized? The generic subtitle "reservations, Spot, or storage trade-offs assumed" hides real architectural consequences — on a live test stack, the Optimized total silently dropped the NAT Gateway (~a third of the Balanced total), a network-posture change, not just savings. Readers quote the lowest number to their board without knowing it embeds a security change and a 1-year commitment.
schema-estimate-infra.md:projected_costs.scenario_deltas(required with three tiers) — per non-Balanced scenario, each concrete difference vs Balanced with its monthly delta; architectural deltas must state their operational consequence, commitment savings must name the commitment. Absent field (pre-extension artifacts) → column omitted, never invented.estimate-infra.md: the engine records the deltas while computing the tier totals (it already priced them); rule: never present an Optimized total whose embedded architectural changes are not itemized. Present Summary tier table lists them.report-decision-core.md: tier table gains a "vs Balanced, for this stack" column rendered verbatim from the artifact, with an explicit guard against expanding generic tier definitions instead.estimation-infra-reference.jsonmodels the field;migration-report-reference.htmlrenders the column; the decision golden deliberately exercises the absent-field path.4. Track-scoped outcomes — a failing secondary track demotes the track, never the stack (
66567cc)The problem, in plain English:
stay_ifis a flat, stack-scoped list, but a hybrid migration has separable tracks — compute+DB, AI text, AI image, analytics — with different answers. A real report render surfaced the failure: "Stay if: Imagen quality cannot be matched after the Stability AI evaluation" presented a single AI workload's quality eval as a reason to keep Postgres and compute on GCP. The plugin already rejects exactly this pattern for BigQuery ("deferral is not a reason to stay unless analytics must cut over in the same window") but never generalized the principle.schema-estimate-infra.md:recommendation.track_outcomes[]({track, outcome, note}, same outcome enum; required for multi-track stacks) plus a scope rule — whole-stackstay_ifis reserved for stay-entirely factors; track-scoped factors go inconditions[]with a track prefix and must never render as stack-level stay reasons.estimate-infra.md: triggers apply per track; a failing/unproven secondary track demotes that track only (e.g. failed image eval → that track keeps the current provider via the adapter; compute+DB stay go). The stack outcome must not be worse than the compute/database track's outcome on account of a secondary track's condition. Chat summary gains the per-track line.report-decision-core.md: required per-track disposition line under the verdict whentrack_outcomesexists; silent omission for single-track and pre-extension artifacts.Why
Completes the Decide/Execute split designed with #185 — one pipeline, two exit ramps, each with a durable artifact — and applies the same provenance discipline dollars already had to the two remaining unlabeled number families (time/effort, tier deltas). The extraction approach (shared core, two render depths) was chosen over a sibling
generate-decision-report.mdspecifically to avoid two copies of the #175 rules drifting apart; concerns 2 and 3 then land here because they rewrite the extracted rules.Test plan
dprint check— cleanmise run lint:md— 0 errors (806 files);lint:frontmatter— OK (both skills);shared:check— OK, 3 treespytest tests/— 51 passed (36 pre-existing + 4 decision-mode + validator-gate updates; includes reference-fixture-passes and stub-fails against the extended validator)run-asserters.py— PASS, 7 asserters (decide-complete golden + both workshop goldens)effort_hours/total_weeksoutside legacy-handling text; zero bare hour/week ranges in any fixture HTMLREPORT_OK | sections=10/10under the extended validator, including the not-comparable baseline callout and the assumptions paneldecision-report.html+DECISION.mdwritten, validator passes in decision mode, state lands decide-completemigration-report.htmlrendered fresh (decision report untouched), full-mode validator passesOut of scope
what-if-scenariostriggertotal_weekswritten as null with an owner note; deferred)Review note
Reviewers who prefer this split: concerns 2–4 are cleanly separable as commits (
5caccbd,80cace8,66567cc) stacked on the report-core extraction — say the word and we'll split. All three edit the same tier-table/exec-section rules inreport-decision-core.md, which is why they stack here.Revision history
4ff2531…27aa5dd— decision report core + style/visual/a11y conventions (review rounds 1–2)5caccbd— timeline/effort honesty (drafted, reviewed by a second model, amended: verdict formula,duration_hintrename, tiered validator gates, labeled legacy fallback)80cace8— scenario deltas66567cc— track-scoped outcomes (user-reported flaw in a live report render)