[herosjourney]

Problem

When heroku-to-aws is run against a large Heroku Enterprise estate, three things the customer and account team need are missing, so the SA ends up rebuilding them by hand from the skill's JSON:

• Which apps share a database. Apps attached to the same Postgres/Redis/Kafka add-on cannot be cut over one at a time without cross-cloud access to that store. The skill's flat model has no view of this.
• A spreadsheet. Customers review estates in Excel, not HTML. The inventory has everything needed for one.
• The contract. Enterprise customers have an order form with committed spend that list-price discovery cannot see (support, credits, headroom). Without it the Heroku baseline looks too low and the AWS saving too high, and the customer rightly distrusts the number.

Fix

Shared data stores (cohorts)

• discover-live.md: record attached_apps per add-on from addons.json attachments[]; limitation noted when the capture has no attachments.
• discover-assemble.md rule 11 (rule 10 is the end-of-life stack rule from feat(aws-startup-advisor): flag end-of-life Heroku stacks in heroku-to-aws discovery #610, already on main) + schema: shared_data_stores.cohorts (connected components over owning app + attached apps, data-store add-ons only), apps_in_cohorts. Explicitly a reporting label: no edges, no ordering, no cluster_id on resources. The Philosophy "Flat resource model" bullet in SKILL.md gains one sentence saying so.
• generate-report.md: conditional shared-data-stores section (table + one sentence on what a cohort means for cutover; no sequencing recommendation).

Estate workbook — scripts/emit-estate-workbook.py (+ test_emit_estate_workbook.py, 10 tests: tabs, exclusions, optional inputs, fail-open, stale-artifact exclusion, XML-illegal characters stripped and every worksheet part parses, NaN/Infinity → empty cell, .tmp removed on write failure)

• Stdlib-only .xlsx writer (zip + inline-string XML), so it runs anywhere the skill runs.
• Tabs: Read Me, Summary, Apps, Formations, Postgres, Redis, Kafka, Other add-ons, Shared data stores, Cost estimate (when the Estimate ran). Each resource row carries AWS status (Mapped / Deferred + reason / Design not run) and the proposed target and sizing from aws-design.json.
• Copies only; computes nothing. Follower databases are listed with their role and excluded from the leaders-only data total.
• Never includes config var names/values, hostnames (counts only), app/resource UUIDs, VPC/subnet ids — a test asserts this.
• Fail-open (ESTATE_OK / ESTATE_SKIP, always exit 0), same pattern as emit-plan-json.py. Wired at the end of Discover (so the customer can review what was found before any design exists) and refreshed after Estimate and Generate.

Contract reconciliation

• clarify-interview.md Q16 (asked when billing_profile is absent, null, or has available != true; in Batch 3): order-form line totals and term dates into preferences.commercial.heroku_contract. No unit prices, quantities or counterparty recorded.
• estimate-cost-engine.md Part 1: current_costs.contract with monthly run-rate, per-category contract vs discovered vs gap, a gap_explanation that must name mechanisms (negotiated vs list, headroom, apps outside CLI view, support/Connect invisible to discovery), with discovered add-on spend bucketed under addon_credits so per-line discovered_usd sums to discovered_monthly. Part 3 gains heroku_monthly_like_for_like (= full baseline − current_costs.deferred_resource_cost, an additional field shown beside the unchanged full baseline) and heroku_monthly_contract. preferences.commercial.heroku_contract / sources.Q16 are in the clarify-assemble.md template (machine contract).
• generate-report.md: reconciliation table under exec-costs with a fixed caveat; "contract minus AWS" is never the headline saving.

Ported from awslabs/startups#365, which is closed in favour of this PR. Best landed after #608 (environment-tier columns and the report tier line read fields #608 produces; they are blank otherwise).

Scope

plugins/aws-startup-advisor/skills/heroku-to-aws/ only: SKILL.md (one sentence), references/phases/clarify/clarify-interview.md, references/phases/discover/discover-live.md, references/phases/discover/discover-assemble.md, references/phases/estimate/estimate-cost-engine.md, references/phases/estimate/estimate-assemble.md, references/phases/generate/generate-assemble.md, references/phases/generate/generate-report.md, references/shared/schema-discover-heroku.md, and two new files under scripts/. No root or .github changes.

Known limitations: the new report sections are conditional and not enforced by validate-heroku-migration-report.py (the required section set is unchanged). Q16 is skipped when invoice data is present, so a customer with both an invoice and a contract is not asked about the contract. pg_role on the workbook's Postgres tab, the Apps tab's Environment tier / Tier source columns, the Summary's tier breakdown and the report's apps_by_environment_tier line are populated only once #608 lands; without it those columns are blank, the data total includes all databases, and the report omits the tier line (fail-soft, "when present"). No live run against a customer account is included.

Type of Change

Select at least one.

• New skill
• Bug fix
• Documentation update
• CI/CD change
• Other

Startup Advisor review checklist

By submitting this PR, I confirm that:

• I have reviewed the full diff and understand all changes I am submitting.
• I have tested my changes and confirmed they work as intended.
• I have run mise -C plugins/aws-startup-advisor run ci and all checks pass.
• I have disclosed any known issues, limitations, or untested scenarios.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[leon1418]

[🤖 AI review 🤖]

Reviewed c6b9b23. Requesting changes for five P2 issues:

1. Run the Discover workbook command in the main-window handoff; the dispatched rw assembler has no shell.
2. Preserve Fargate plus ALB mappings and copy the supported Fargate/EKS sizing fields into the workbook.
3. Include Q16 in the full interview batch and presentation so an order form can reach the estimate.
4. Keep pending downstream mappings and costs out of a rediscovery workbook while preserving the old artifacts.
5. Derive and display the separate scope-matched comparison across the estimate, report and workbook; retain the full-spend comparison and its caveats.

The inline comments give the triggers and corrections. There is also one lower-priority P3 input-decoding issue in the writer's fail-open handling.

Validation: 39 focused tests passed on original source, git diff --check passed, and 17 synthetic report-validator probes on the audited source copy behaved as expected. Those probes confirm that the new optional report content is not enforced by the validator. No live Heroku migration or Excel UI run was performed.

[leon1418]

[🤖 AI review 🤖]

Follow-up review of 803af2f completed with OCR, an independent reviewer, and the convergence evidence gate. The Discover handoff, workbook target/sizing, pending-phase filtering and UTF-8 handling are verified fixed; those four conversations are resolved. Q16's batch wiring is also fixed.

Two P2 concerns remain in their existing threads:

• Q16 must treat an absent/null billing profile as no invoice. I missed this producer/condition mismatch in the earlier review.
• The matched comparison must use the same priced deferred contribution for its baseline subtraction and excluded-amount disclosure; two definitions still request list prices.

Validation: 41 focused tests passed on original source, eight original-writer cases passed, and git diff --check passed. The independent reviewer also ran the seven authored workbook tests and additional producer-shape probes on audited copies. No live Heroku migration or Excel UI run was performed. The existing changes-requested review remains outstanding.

[herosjourney]

Reviewed: head c6b9b23 (base 00840a0), 11 files (+741/−1). Skill touched: heroku-to-aws. Tree: plugins/aws-startup-advisor only.

Ran:

• pytest scripts/test_emit_estate_workbook.py: 5 passed.
• emit-estate-workbook.py on 14 constructed run directories, each output opened with openpyxl 3.1.5 and every XML part parsed. Inputs covered: a realistic inventory shaped by the schema; inventory only and the full inventory + design + estimate-with-contract chain; an empty estate, {} and []; formula-leading, unicode and comma names; control characters, NaN/Infinity and a lone surrogate; duplicate names, a 40k-character cell and wrong-typed fields.
• Two mutation probes on a copy of the script.
• A contract trace of every field the script reads or the diff writes, against the schema, the phase files and scripts/artifact-contracts.json.
• An _exec tier trace against INTERPRETER.md.
• A producer sweep for new fields across main, this head and the other open heroku-to-aws PRs (#608, #610, #611, #612, #614, #615).

GitHub CI is green at head (startup-advisor runs mise run ci; CodeQL, check-frontmatter and the author checklist also pass). Merges cleanly into main per git merge-tree.

Did not run: mise run ci locally; any live Heroku CLI call; an end-to-end Discover → Generate run; opening the workbooks in Excel or LibreOffice; checking the heroku addons --all --json output shape against a primary source.

Cross-plugin parity: n/a. This repo has a single copy of skills/heroku-to-aws/, and no vendored files were touched. Outside skills/: nothing changed.

Change narrative: Three things get rebuilt by hand today in large Enterprise estates: which apps share a data store, a spreadsheet the customer will review, and the committed contract spend. The PR adds attached_apps and cohort grouping in Discover; a standard-library .xlsx writer run at Discover, Estimate and Generate; and a Clarify question (Q16) feeding a contract reconciliation in Estimate and new report sections. Body vs diff: it matches, except "Independent of #608". Only the pg_role dependency is disclosed; the environment_tier and apps_by_environment_tier dependencies are not (Should-fix 4).

Axis	Stance	One line
Parsimony	advisory	Additive and in the repo's idiom (a fail-open writer copying emit-plan-json.py). The Discover-time run is the one wiring point that costs more than it returns (Alternatives 1).
Completeness	finding	Q16 can't be reached (Blocking 1). Discover's dispatched worker is told to run python3 with no shell (Should-fix 1).
Security	advisory	No config var names or values, hostnames or UUIDs reach the workbook (test plus probe). Formula-leading text is stored as inline strings with no formula element. Order forms are not named in Input Security (Nit 3).
Alternatives	advisory	Two below.
Contradictions	finding	The like-for-like rule conflicts with "Keep the full baseline" (Should-fix 3). The schema cites "rule 9" for what is rule 10 (Nit 1).
Overlap	advisory	The like-for-like step re-derives Part 1b's deferred_resource_cost (folded into Should-fix 3).
Cross-plugin parity	clear	Single tree; nothing to mirror.
Propagation & data	finding	shared_data_stores / attached_apps: schema updated first, every consumer swept. preferences.commercial: its home clarify-assemble.md was not updated (Blocking 1). environment_tier, environment_tier_source and apps_by_environment_tier have no producer on main or at this head; only #608 has one (Should-fix 4).

The contract between the script and the phase files was checked and matches:

• Invocation: the command string is identical in discover-assemble.md:65, estimate-assemble.md:386 and generate-assemble.md:124, and matches argparse.
• Inputs: heroku-resource-inventory.json, aws-design.json and estimation-infra.json match the _produces of their phases.
• Status lines: ESTATE_OK | … / ESTATE_SKIP | reason=… match all three phase files.
• Design join: deferred[] is matched on (addon_name, addon_plan), consistent with design-mapping.md § 2F.

Blocking

1. [Completeness] Q16 is never asked and its answer has nowhere to land, so contract reconciliation can't run on any path. clarify/clarify-interview.md:159

| Q16 — Heroku contract | \billing_profile.available == false` (no invoice data) | Billing data present, or user says "none" |`

Consequence:

• The batch table puts Batch 3 at "Q11, Q12b, Q12–Q15" (:230), the batch rule says "Q12–Q15 always fire" (:234), and the Batch 3 template presents "[Present active questions Q11–Q15]" (:287). Q16 is active but in no batch, and questions are only presented by batch.
• Even if asked, clarify-assemble.md Step 4 (the declared preferences.json shape per scripts/artifact-contracts.json) has no commercial key, so preferences.commercial.heroku_contract is never written.
• As a result, estimate-cost-engine.md:151 never fires, current_costs.contract is never produced, and the report table, caveat and workbook block are dead on every run.
• The trigger also reads available == false, while runs with no billing file carry billing_profile: null or no section (fixtures heroku-nonweb-scaling, heroku-workshop).

Fix:

• Batch 3 cell (:230) → Q11, Q12b, Q12–Q16; rule at :234 → "Q16 fires when no invoice data"; :287 → [Present active questions Q11–Q16].
• Trigger → "billing_profile absent, null, or available == false".
• In clarify-assemble.md Step 4, add "commercial": { "heroku_contract": { … } } (omitted when Q16 is answered none or defaulted) and "Q16" to sources.

Regression: a contract test asserting every question in the Applicability table appears in exactly one batch row (or the "Not in any batch" list), and that clarify-assemble.md names commercial.

Should-fix

1. [Completeness] Discover's workbook step tells a shell-less worker to run python3. discover/discover-assemble.md:64–65. discover.md declares _exec: _agent: rw, and INTERPRETER.md:307/314 gives that tier Read/Grep/Glob/Write/Edit only ("rw deliberately excludes shell/Bash"). On dispatching hosts the Discover-time workbook, sold in the body as "review what was found before any AWS design exists", is never written, and the step invites a fabricated ESTATE_OK. Fix: remove step 11 from the assembler and add a main-window step to discover.md after HANDOFF_OK, mirroring generate-assemble.md:123. Generate (rwx) and Estimate (inline) are fine as written.

2. [Generated artifacts] The writer prints ESTATE_OK for a workbook that won't open. scripts/emit-estate-workbook.py:44–47, :75–76. Reproduced at head:

• confidence_note: "bad\x01char\x0bhere" gives ESTATE_OK, then sheet2.xml is not well-formed and openpyxl raises ParseError.
• monthly_price_usd: NaN (accepted by json.load) gives ESTATE_OK, then openpyxl reports "Unable to read workbook … invalid XML".
• A lone surrogate in an app name gives ESTATE_SKIP, but leaves heroku-estate.xlsx.tmp in $MIGRATION_DIR (:123–131).

Free-text fields copied verbatim are the realistic carriers (confidence_note, gap_explanation, order-form notes pasted from Word, where \x0b is a soft line break). Mutation probe: replacing escape() with return text still passes all 5 tests, so escaping has no coverage. Fix: strip [\x00-\x08\x0b\x0c\x0e-\x1f\ufffe\uffff\ud800-\udfff] in escape(), emit an empty cell for non-finite floats, and tmp.unlink(missing_ok=True) on any exception during the zip write. Regression: a test with &<>", \x0b and NaN that asserts ESTATE_OK and that every worksheet part parses.

3. [Contradictions] The like-for-like rule pulls against "Keep the full baseline", and the figure has two names. estimate/estimate-cost-engine.md:182–184 says the like-for-like figure is "the only Heroku figure that is scope-matched … Part 3 uses it". Part 3 (:433) says "Keep the full baseline — do not subtract or hide the deferred resource's cost." Part 3 also stores the figure as heroku_monthly_like_for_like (:402), not discovered_like_for_like_monthly, and step 5 re-derives Part 1b's deferred_resource_cost by a different method. Fix: "Write cost_comparison.heroku_monthly_like_for_like = Part 1 baseline − current_costs.deferred_resource_cost. Show it beside, never instead of, heroku_monthly_baseline; every monthly_difference stays against the full baseline." The same goes for step 2 (:158–161): the discovered add-on bucket has no contract category to land on, so per-line discovered_usd won't sum to discovered_monthly. Map add-ons to addon_credits.

4. [Propagation / PR honesty] Consumers of #608-only fields ship without #608, and the body says "Independent of #608". generate/generate-report.md:64 (metadata.apps_by_environment_tier) and emit-estate-workbook.py:202–203, 218 (environment_tier, environment_tier_source) have no producer on main or at this head; only #608's discover-assemble.md rule 5a writes them. If this lands first, the Apps tab carries two always-blank columns and Summary shows every app as "(not inferred)". Fix: state the dependency and land #608 → #609, or move these consumers into #608.

Nits

1. references/shared/schema-discover-heroku.md:73: "written by assemble rule 9" should read "rule 10" (discover-assemble.md:46).
2. scripts/emit-estate-workbook.py:331–340: the Cost estimate tab omits heroku_monthly_like_for_like and the per-line discovered_usd/gap_usd, so the workbook and report show the contract differently.
3. SKILL.md:145 Input Security lists "billing exports" but not the Enterprise order form that Q16 now invites. Add it to the untrusted-file list.

Heads-up (out of diff or pre-existing, not blocking)

1. The new test file is not run by CI. The mise run ci test task runs pytest tests -q from the plugin root, so skills/heroku-to-aws/scripts/test_*.py is never collected (the same is true of the existing test_validate_heroku_migration_report*.py). Green CI says nothing about this script.
2. Unknown whether cohorts can be detected. Whether heroku addons --all --json emits attachments[] was not verified. The replay fixture fixtures/heroku-live-capture/live-capture/addons.json has no attachments key, so no fixture exercises cohorts. Please link a captured sample or the CLI source.
3. A 40,012-character string is written unchecked; Excel's cell limit is 32,767. openpyxl reads it; Excel's behaviour is not verified.
4. Landing order: #611 is stacked on this PR. Conflicts with #610 in discover-assemble.md and generate-report.md, and with #612 in generate-report.md; shares files with #608 and #614 without conflict. Suggested order: #608 → #609 → #611, then rebase #610 and #612.

Alternatives (max 2)

1. Write the workbook at Estimate and Generate only. This removes the shell-less wiring point (Should-fix 1); the cost is no workbook before Design. Keep the Discover run only if it moves to the main window.
2. Compute cohorts in code, not prose. A small helper for the connected components over attached_apps, shared by the workbook and report and covered by a test. Follow-up.

Verdict

Comment. Blocking 1 would set Request changes from a non-author identity.
Commenting rather than approving: author == reviewer identity.

[herosjourney]

Reviewed: head c6b9b23 (base 00840a0), 11 files (+741/−1). Skill touched: heroku-to-aws. Tree: plugins/aws-startup-advisor only.

Ran:

• pytest scripts/test_emit_estate_workbook.py: 5 passed.
• emit-estate-workbook.py on 14 constructed run directories, each output opened with openpyxl 3.1.5 and every XML part parsed. Inputs covered: a realistic inventory shaped by the schema; inventory only and the full inventory + design + estimate-with-contract chain; an empty estate, {} and []; formula-leading, unicode and comma names; control characters, NaN/Infinity and a lone surrogate; duplicate names, a 40k-character cell and wrong-typed fields.
• Two mutation probes on a copy of the script.
• A contract trace of every field the script reads or the diff writes, against the schema, the phase files and scripts/artifact-contracts.json.
• An _exec tier trace against INTERPRETER.md.
• A producer sweep for new fields across main, this head and the other open heroku-to-aws PRs (#608, #610, #611, #612, #614, #615).

GitHub CI is green at head (startup-advisor runs mise run ci; CodeQL, check-frontmatter and the author checklist also pass). Merges cleanly into main per git merge-tree.

Did not run: mise run ci locally; any live Heroku CLI call; an end-to-end Discover → Generate run; opening the workbooks in Excel or LibreOffice; checking the heroku addons --all --json output shape against a primary source.

Cross-plugin parity: n/a. This repo has a single copy of skills/heroku-to-aws/, and no vendored files were touched. Outside skills/: nothing changed.

Change narrative: Three things get rebuilt by hand today in large Enterprise estates: which apps share a data store, a spreadsheet the customer will review, and the committed contract spend. The PR adds attached_apps and cohort grouping in Discover; a standard-library .xlsx writer run at Discover, Estimate and Generate; and a Clarify question (Q16) feeding a contract reconciliation in Estimate and new report sections. Body vs diff: it matches, except "Independent of #608". Only the pg_role dependency is disclosed; the environment_tier and apps_by_environment_tier dependencies are not (Should-fix 4).

Axis	Stance	One line
Parsimony	advisory	Additive and in the repo's idiom (a fail-open writer copying emit-plan-json.py). The Discover-time run is the one wiring point that costs more than it returns (Alternatives 1).
Completeness	finding	Q16 can't be reached (Blocking 1). Discover's dispatched worker is told to run python3 with no shell (Should-fix 1).
Security	advisory	No config var names or values, hostnames or UUIDs reach the workbook (test plus probe). Formula-leading text is stored as inline strings with no formula element. Order forms are not named in Input Security (Nit 3).
Alternatives	advisory	Two below.
Contradictions	finding	The like-for-like rule conflicts with "Keep the full baseline" (Should-fix 3). The schema cites "rule 9" for what is rule 10 (Nit 1).
Overlap	advisory	The like-for-like step re-derives Part 1b's deferred_resource_cost (folded into Should-fix 3).
Cross-plugin parity	clear	Single tree; nothing to mirror.
Propagation & data	finding	shared_data_stores / attached_apps: schema updated first, every consumer swept. preferences.commercial: its home clarify-assemble.md was not updated (Blocking 1). environment_tier, environment_tier_source and apps_by_environment_tier have no producer on main or at this head; only #608 has one (Should-fix 4).

The contract between the script and the phase files was checked and matches:

• Invocation: the command string is identical in discover-assemble.md:65, estimate-assemble.md:386 and generate-assemble.md:124, and matches argparse.
• Inputs: heroku-resource-inventory.json, aws-design.json and estimation-infra.json match the _produces of their phases.
• Status lines: ESTATE_OK | … / ESTATE_SKIP | reason=… match all three phase files.
• Design join: deferred[] is matched on (addon_name, addon_plan), consistent with design-mapping.md § 2F.

Blocking

1. [Completeness] Q16 is never asked and its answer has nowhere to land, so contract reconciliation can't run on any path. clarify/clarify-interview.md:159

| Q16 — Heroku contract | \billing_profile.available == false` (no invoice data) | Billing data present, or user says "none" |`

Consequence:

• The batch table puts Batch 3 at "Q11, Q12b, Q12–Q15" (:230), the batch rule says "Q12–Q15 always fire" (:234), and the Batch 3 template presents "[Present active questions Q11–Q15]" (:287). Q16 is active but in no batch, and questions are only presented by batch.
• Even if asked, clarify-assemble.md Step 4 (the declared preferences.json shape per scripts/artifact-contracts.json) has no commercial key, so preferences.commercial.heroku_contract is never written.
• As a result, estimate-cost-engine.md:151 never fires, current_costs.contract is never produced, and the report table, caveat and workbook block are dead on every run.
• The trigger also reads available == false, while runs with no billing file carry billing_profile: null or no section (fixtures heroku-nonweb-scaling, heroku-workshop).

Fix:

• Batch 3 cell (:230) → Q11, Q12b, Q12–Q16; rule at :234 → "Q16 fires when no invoice data"; :287 → [Present active questions Q11–Q16].
• Trigger → "billing_profile absent, null, or available == false".
• In clarify-assemble.md Step 4, add "commercial": { "heroku_contract": { … } } (omitted when Q16 is answered none or defaulted) and "Q16" to sources.

Regression: a contract test asserting every question in the Applicability table appears in exactly one batch row (or the "Not in any batch" list), and that clarify-assemble.md names commercial.

Should-fix

1. [Completeness] Discover's workbook step tells a shell-less worker to run python3. discover/discover-assemble.md:64–65. discover.md declares _exec: _agent: rw, and INTERPRETER.md:307/314 gives that tier Read/Grep/Glob/Write/Edit only ("rw deliberately excludes shell/Bash"). On dispatching hosts the Discover-time workbook, sold in the body as "review what was found before any AWS design exists", is never written, and the step invites a fabricated ESTATE_OK. Fix: remove step 11 from the assembler and add a main-window step to discover.md after HANDOFF_OK, mirroring generate-assemble.md:123. Generate (rwx) and Estimate (inline) are fine as written.

2. [Generated artifacts] The writer prints ESTATE_OK for a workbook that won't open. scripts/emit-estate-workbook.py:44–47, :75–76. Reproduced at head:

• confidence_note: "bad\x01char\x0bhere" gives ESTATE_OK, then sheet2.xml is not well-formed and openpyxl raises ParseError.
• monthly_price_usd: NaN (accepted by json.load) gives ESTATE_OK, then openpyxl reports "Unable to read workbook … invalid XML".
• A lone surrogate in an app name gives ESTATE_SKIP, but leaves heroku-estate.xlsx.tmp in $MIGRATION_DIR (:123–131).

Free-text fields copied verbatim are the realistic carriers (confidence_note, gap_explanation, order-form notes pasted from Word, where \x0b is a soft line break). Mutation probe: replacing escape() with return text still passes all 5 tests, so escaping has no coverage. Fix: strip [\x00-\x08\x0b\x0c\x0e-\x1f\ufffe\uffff\ud800-\udfff] in escape(), emit an empty cell for non-finite floats, and tmp.unlink(missing_ok=True) on any exception during the zip write. Regression: a test with &<>", \x0b and NaN that asserts ESTATE_OK and that every worksheet part parses.

3. [Contradictions] The like-for-like rule pulls against "Keep the full baseline", and the figure has two names. estimate/estimate-cost-engine.md:182–184 says the like-for-like figure is "the only Heroku figure that is scope-matched … Part 3 uses it". Part 3 (:433) says "Keep the full baseline — do not subtract or hide the deferred resource's cost." Part 3 also stores the figure as heroku_monthly_like_for_like (:402), not discovered_like_for_like_monthly, and step 5 re-derives Part 1b's deferred_resource_cost by a different method. Fix: "Write cost_comparison.heroku_monthly_like_for_like = Part 1 baseline − current_costs.deferred_resource_cost. Show it beside, never instead of, heroku_monthly_baseline; every monthly_difference stays against the full baseline." The same goes for step 2 (:158–161): the discovered add-on bucket has no contract category to land on, so per-line discovered_usd won't sum to discovered_monthly. Map add-ons to addon_credits.

4. [Propagation / PR honesty] Consumers of #608-only fields ship without #608, and the body says "Independent of #608". generate/generate-report.md:64 (metadata.apps_by_environment_tier) and emit-estate-workbook.py:202–203, 218 (environment_tier, environment_tier_source) have no producer on main or at this head; only #608's discover-assemble.md rule 5a writes them. If this lands first, the Apps tab carries two always-blank columns and Summary shows every app as "(not inferred)". Fix: state the dependency and land #608 → #609, or move these consumers into #608.

Nits

1. references/shared/schema-discover-heroku.md:73: "written by assemble rule 9" should read "rule 10" (discover-assemble.md:46).
2. scripts/emit-estate-workbook.py:331–340: the Cost estimate tab omits heroku_monthly_like_for_like and the per-line discovered_usd/gap_usd, so the workbook and report show the contract differently.
3. SKILL.md:145 Input Security lists "billing exports" but not the Enterprise order form that Q16 now invites. Add it to the untrusted-file list.

Heads-up (out of diff or pre-existing, not blocking)

1. The new test file is not run by CI. The mise run ci test task runs pytest tests -q from the plugin root, so skills/heroku-to-aws/scripts/test_*.py is never collected (the same is true of the existing test_validate_heroku_migration_report*.py). Green CI says nothing about this script.
2. Unknown whether cohorts can be detected. Whether heroku addons --all --json emits attachments[] was not verified. The replay fixture fixtures/heroku-live-capture/live-capture/addons.json has no attachments key, so no fixture exercises cohorts. Please link a captured sample or the CLI source.
3. A 40,012-character string is written unchecked; Excel's cell limit is 32,767. openpyxl reads it; Excel's behaviour is not verified.
4. Landing order: #611 is stacked on this PR. Conflicts with #610 in discover-assemble.md and generate-report.md, and with #612 in generate-report.md; shares files with #608 and #614 without conflict. Suggested order: #608 → #609 → #611, then rebase #610 and #612.

Alternatives (max 2)

1. Write the workbook at Estimate and Generate only. This removes the shell-less wiring point (Should-fix 1); the cost is no workbook before Design. Keep the Discover run only if it moves to the main window.
2. Compute cohorts in code, not prose. A small helper for the connected components over attached_apps, shared by the workbook and report and covered by a test. Follow-up.

Verdict

Comment. Blocking 1 would set Request changes from a non-author identity.
Commenting rather than approving: author == reviewer identity.

[leon1418]

[🤖 AI review 🤖]

Reviewed 31f171c. The two previously reported issues are fixed: Q16 now handles absent billing profiles, and matched-cost fields use the same selected-baseline contribution.

Two additional P2 issues remain:

1. The report promises to cross-check the new Heroku comparison anchor, but the validator skips that key. Checking the already-emitted anchor does not require new mandatory sections.
2. A valid estate can have a zero matched baseline; the percentage formula and report need an explicit unavailable-percentage branch while keeping the absolute difference.

Both issues were present at the previously reviewed head, 803af2f. I missed them in that review; they were not introduced by these latest fixes. The inline comments give reproductions and bounded corrections.

Validation: 44 focused original-source tests passed; eight original-writer cases, five preference-contract cases and XML boundary probes passed. Six full report-validator CLI checks reproduced the missing Heroku cross-check with AWS mismatch controls. The zero-baseline input passes the original machine inventory contract; its arithmetic witness is not a live migration execution.

[herosjourney]

The branch is now merged up to current main (head 080e8fde). The conflict was in the Discover assemble rules and the report section list: both sides kept, and this PR's shared-data-stores rule became rule 11 because main's end-of-life stack rule took 10. Full CI passed on the conflict-resolution commit. Earlier review threads are replied to and resolved. Requesting a fresh review of 080e8fde. Merge still needs one code-owner approval from someone who did not push.