Repository navigation
feat(aws-startup-advisor): parse RDfA reports into Discover (azure-to-aws) - #458
Conversation
Adds discover-rdfa.md: parses an RDfA (Resource Discovery for Azure) report archive into azure-resource-inventory.json's measured_utilization and rdfa_metadata, per the APPROVED design (Revision 5). Plain fragment, no Part A/Part B split (reading a handed-over archive is not interactive). - discover-live.md: row-4 rename table also renames the captured array's own key nodePools -> node_pools, so RDfA and live az agree on config.node_pools[] - discover.md: wire rdfa into _fragments[], promote the Status table row - discover-assemble.md: note measured_utilization as the measured tier's real producer - estimate-infra.md: rung-2 flips No-deferred to Partial-usage-quantity-only - SKILL.md: RDfA no longer listed as missing - schema-discover-azure.md: new rdfa_report_unreadable warning code New fixtures (fixtures/azure-rdfa-report*/) and tests/test_azure_rdfa_discover_contracts.py (18 tests: prose contracts + fixture-shape assertions for measured_utilization extraction, MetricError handling, corrupt-ZIP handling, and multi-ZIP tie-break). Verified: mise -C plugins/aws-startup-advisor run ci, fully green. 409 tests passed (up from 391 baseline), lint:md/model-id/model-staleness/ artifacts/vendored-parity/skill-refs all pass, test:heroku-source-review unaffected. Full evidence recorded in .agents/tasks/azure-rdfa-discovery/verification.md (startups-pr repo).
leon1418
left a comment
leon1418
left a comment
There was a problem hiding this comment.
[🤖 AI review 🤖]
Requesting changes at 0e535702e1aec9e064066e6a0101cd15ea70fe64: three P1 and six P2 findings remain after the combined review.
The consolidated correction list is:
- Wire RDfA into Discover entry/completion/assembly and make absent-report guidance reachable.
- Parse the real Metrics envelopes and handle missing window provenance consistently through estimation.
- Run archive extraction/projection in a stage with the required execution capability.
- Exclude successful zero-sample responses from utilization evidence.
- Supply the required AI profile for retained Azure ML workspaces.
- Preserve report identity across extraction-cache reuse and re-entry.
- Normalize fixed AKS pool bounds before generating scaling configuration.
- Populate the canonical nested VM OS-disk fields.
- Register
rdfa_metadatain the discovery artifact schema.
The inline comments include failure cases and verification criteria. Local validation passed: 449 Python tests, the two Heroku source-review suites (13 and 44 tests), Markdown lint, and artifact/reference/model/consent checks. GitHub checks were green at the inspected head. Focused source-contract probes exposed the issues above; the new metadata also fails the actual artifact validator. This review did not run RDfA against an Azure tenant or execute a complete LLM migration.
Move executable report preparation into the main window, admit report-only discovery, parse producer metric envelopes without invented windows, gate measured sizing on samples and known windows, add RDfA AI provenance, source-cache identity, AKS bound normalization, canonical VM OS disks, and the authoritative rdfa_metadata contract. Verification: - mise exec -- uv run --no-project --with pytest python -m pytest plugins/aws-startup-advisor/tests/test_azure_rdfa_discover_contracts.py plugins/aws-startup-advisor/tests/test_azure_discover_ordering.py plugins/aws-startup-advisor/tests/test_ai_workload_profile_contracts.py plugins/aws-startup-advisor/tests/test_validate_artifacts.py -q -p no:cacheprovider (104 passed) - python3 plugins/aws-startup-advisor/scripts/validate-artifacts.py --self-check (pass) - python3 plugins/aws-startup-advisor/scripts/validate-artifacts.py --fixtures (81 checked, 0 errors) - python3 plugins/aws-startup-advisor/tools/check-skill-refs.py --check (1289 checked, 0 missing) - python3 plugins/aws-startup-advisor/tools/sync-vendored.py --check (49 byte-identical) - python3 plugins/aws-startup-advisor/scripts/validate-artifacts.py --run-dir plugins/aws-startup-advisor/fixtures/azure-rdfa-final/after-discover --skill azure-to-aws --no-baseline --json (1 checked, 0 errors) - mise -C plugins/aws-startup-advisor run ci (446 passed; all lint checks passed) - mise run build (all root lint, validation, parity, and security checks passed)
Seed valid external report sources from the prior manifest on re-entry, move unreadable-report handling into main-window preparation, and preserve preparation warnings for assembly without a success manifest. Verification: - Targeted Azure tests: 108 passed - Artifact self-check and fixtures: passed (81 checked, 0 errors) - Final RDfA fixture validator: passed (1 checked, 0 errors) - Skill refs: passed (1,289 checked, 0 missing) - Vendored parity: passed (49 files) - Startup advisor CI: passed (450 Python tests; all lints/checks passed) - Root build: passed (Markdown, manifests/spec, parity, gitleaks) - git diff --check: passed
leon1418
left a comment
leon1418
left a comment
There was a problem hiding this comment.
[🤖 AI review 🤖]
Follow-up at 94cf8f0e25f72e2dfa1eb76fb9f3ee060aa105ba: eight original root causes are corrected. Two P2 blockers remain:
- The existing fixed-AKS-pool concern still fails for the producer’s explicit-false autoscaling shape; evidence is in the original conversation.
- Moving RDfA preparation into the main window introduced an initialization-order problem: it uses the migration cache before the later live action selects and initializes the run.
The correction list is bounded to producer-compatible fixed-pool normalization and a shared initialization step before both pre-dispatch actions. Keep the fixed/autoscaled controls, and verify cold start plus resume/fresh selection before any RDfA cache access. The original admission and execution discussions stay open because their complete startup acceptance depends on the new ordering fix.
Validation: 450 Python tests, both Heroku suites (13 and 44 tests), Markdown lint, and artifact/reference/model/consent checks passed. Focused review tests and actual inventory/AI validators passed. The AKS counterexample and initialization issue are supported by source contracts and local probes; no Azure tenant or complete LLM migration was executed.
Initialize Discover before RDfA/live pre-actions and normalize the producer's impossible AKS autoscale sentinel while preserving valid zero-min autoscaling. Verification: - mise exec -- uv run --no-project --with pytest python -m pytest plugins/aws-startup-advisor/tests/test_azure_discover_ordering.py plugins/aws-startup-advisor/tests/test_azure_rdfa_discover_contracts.py -q -p no:cacheprovider: PASS, 23 tests - mise -C plugins/aws-startup-advisor run ci: PASS, 464 Python tests plus 13 and 44 Node tests; artifact, vendored parity, references, Markdown, model, pricing, and consent checks passed - mise run build: PASS; root Markdown, manifests, spec, skill parity, and gitleaks passed - git grep opening/closing conflict markers: PASS, none found - git diff --check: PASS
leon1418
left a comment
leon1418
left a comment
There was a problem hiding this comment.
[🤖 AI review 🤖]
Follow-up at b88166eaf137156ab46516792d1dd1a74f49b732: the previously reported positive-count AKS case and shared initialization ordering are corrected. One P2 case remains that I missed in the previous review: Azure permits a user pool with zero nodes, but the mapper emits min=0, desired=0, max=0, and EKS requires maxSize >= 1.
The current normalizer produces that shape; the actual EKS SDK parameter validator rejects it. Preserve the zero desired capacity with a valid positive maximum (or explicitly defer that group), and cover the zero-node case alongside the passing fixed 3/3/3 and autoscaled 0/1/2 controls. This behavior also existed at the prior reviewed head; it was not introduced by the latest fix. Details and source-contract links are in the existing AKS conversation rather than a duplicate thread.
Validation passed: 464 Python tests, both Heroku suites (13 and 44 tests), 81 artifact fixtures, and all current Markdown/model/pricing/reference/consent checks. The independent general review found no candidates; the additional zero-node boundary was verified in the parent review. No Azure/EKS service call, PowerShell producer, or full LLM migration was executed.
leon1418
left a comment
leon1418
left a comment
There was a problem hiding this comment.
[🤖 AI review 🤖]
Reverified at fd0b2759198fc7aa00fe006977845d28a1fa2a1d against base ff72bd2d30bd7a166ed7a25f26ddb45509d442af. The existing zero-node AKS user-pool issue remains unchanged: its normalized maxSize: 0 fails the EKS API minimum of 1. Both review perspectives confirmed it; the detailed evidence is already in the AKS conversation. No additional finding is being added.
Shared run initialization remains fixed, so the pending setup conversation can be resolved on the existing verified-fix reply. The positive fixed-pool and genuine autoscaling controls still pass. All 464 Python tests, both Heroku suites (13 and 44 tests), 81 artifact fixtures and current plugin lints passed. Validation used source contracts, test helpers and SDK parameter validation; no service deployment or full LLM migration was run.
The recommendation remains request changes for the existing zero-node case.
…inimum of 1 Addresses the remaining open review finding on the RDfA AKS node-pool normalizer. Azure permits a user pool with zero nodes; the absent-bounds rule normalized that to min/desired/max = 0/0/0, but the EKS API rejects maxSize 0 (Capacity has a minimum of 1 — "Invalid value for parameter maxSize, value: 0, valid min value: 1"). Keep desired_size and min_size at the real 0 and floor max_size to 1 so the downstream EKS scaling config is valid. Document the rule in discover-rdfa.md (invariant is now min <= desired <= max AND max >= 1) and add a zero-node contract test. Existing fixed (3/3/3) and autoscaled (0/1/2) cases unchanged. Full startup-advisor CI passes (465 Python tests, +1; lint:md, skill-refs, vendored-parity all green).
aws#458 (parse RDfA reports into Discover) merged into main while this branch's prior 'Azure's active skill guidance now matches Discover' commit still described RDfA as an unsupported, halting input. Both descriptions can no longer be true at once. Reconcile by keeping this branch's real fixes (billing exports correctly described as not implemented, Bicep/ARM halt behavior, the tightened Input Security framing, and the delegated-LLM initiated_by note folded into aws#458's new shared Step 0) while restoring RDfA as a supported, non-halting Discover input everywhere this branch had marked it unsupported: - azure-to-aws/SKILL.md: frontmatter description, build-status note, the live-first-discovery and discovery-budget-risk bullets, Input Security (RDfA moves back to the untrusted-but-supported input list; drops the RDfA-halts-if-only-source and RDfA-plus-other-source sentences), the .gitignore note, and the Sizing/Migration mode defaults. - azure-to-aws/references/phases/discover/discover.md: merge conflict resolved by keeping aws#458's refactored 'Shared Step 0' (used by both the RDfA and live-az pre-dispatch actions) and relocating this branch's initiated_by: LLM_TO_BEDROCK note into it. - Root plugins/aws-startup-advisor/README.md: two capability rows that listed RDfA alongside Bicep/ARM/billing-exports as unsupported. - tests/test_decision_gate_wiring.py: updated test_azure_skill_describes_only_implemented_discovery_sources to assert RDfA is present and described as supported, instead of absent/unsupported. Full suite: 475 passed. lint:vendored-parity (54 files), lint:skill-refs (1318 checked, 0 missing), lint:artifacts (81 checked, 0 errors), lint:consent-disclosure, and lint:md (SKILL.md, discover.md, README.md) all pass. Co-authored-by: Cursor <cursoragent@cursor.com>
…s#474) * docs(aws-startup-advisor): name RDfA as an Azure discovery source aws#458 made a customer-supplied RDfA report a real Discover producer and left billing-export discovery unimplemented, but the skill description and plugin README still listed billing exports as a source and never mentioned RDfA. * fix(aws-startup-advisor): trim azure-to-aws SKILL.md description under the 1024 limit The RDfA naming edit pushed the azure-to-aws SKILL.md frontmatter description to 1126 chars, failing the repo-root `build` check (`lint:spec`: description over the 1024 limit). Trim to 1015 chars by dropping two redundant trigger phrases ("move off Azure", "move Azure AI or agentic workloads to AWS") while preserving the RDfA discovery-source mention (this PR's purpose) and the exact "billing-export discovery is not implemented" wording the PR's own test_azure_discovery_source_guidance.py pins. Root build and plugin CI (467 tests) pass. --------- Co-authored-by: Logan Kleier <lkleier@amazon.com>
Problem
Azure migration assessments cannot currently use a customer-provided Resource Discovery for Azure (RDfA) report, even though the Azure skill already identifies RDfA as a missing Discover source. Customers who have already collected roughly 31 days of Azure Monitor history therefore cannot contribute that measured utilization to the assessment and must rely on the other supported discovery sources.
This also leaves downstream estimation without RDfA's usage-quantity evidence and leaves the live Azure and proposed RDfA paths with different names for AKS node-pool configuration.
Fix
Add an opt-in, non-interactive RDfA Discover fragment that reads a ZIP archive or extracted report directory supplied by the customer. The skill never runs RDfA or accesses the customer's tenant; when no report or live
azmanifest is present, it explains what RDfA is, identifies the four Azure roles the customer needs to run it, and clarifies that RDfA is a one-time collection over already-retained history rather than a 31-day waiting period.The fragment selects the newest report when multiple ZIPs are present, skips corrupt or unreadable inputs with an
rdfa_report_unreadablewarning, and joins report inventory and metrics to existing IaC/live inventory entries by Azure resource ID. It records per-resourcemeasured_utilizationfromMetricPercentile, rejects rows marked withMetricError, and computes each row's window from its own timestamps. It also addsrdfa_metadatafor report provenance and deferred data.Align live and RDfA AKS output on
config.node_pools[], wire the fragment into Discover assembly, and mark usage-quantity baseline support as partial for estimation.Scope
This change affects only the
azure-to-awsskill's Discover and Estimate contracts, shared Azure discovery schema, RDfA fixtures, and RDfA contract tests. It adds mappings for 17 of RDfA's 57 collectors. Other collectors fall back to an ARM-ID-derivedazure_typewithazure_type_provenance: "derived_uncorroborated", and their module names are retained inrdfa_metadata.unmapped_collectors[].Dollar-cost derivation from RDfA consumption data remains out of scope. RDfA consumption CSVs contain quantities rather than prices, obfuscation can prevent a reliable inventory join, and the skill does not ship an Azure rate card. The fragment records consumption-file presence as unparsed in
rdfa_metadata.consumption_csv; it does not make the rate-card baseline rung reachable or claim dollar-cost support.The fragment reads a report the customer has already supplied, so it does not add an interactive consent gate. Corrupt or unreadable individual inputs are warned and skipped rather than aborting the full Discover phase.
This PR is self-contained and does not require another PR to land first. It overlaps the Azure unsupported-input guidance changed by open PR #459: if #459 lands first, the RDfA guidance must be reconciled so this implemented source is not still described as unsupported.
In code, for reviewers
discover-rdfa.mdimplements archive selection, inventory normalization, ARM-type mapping and fallback derivation, metric extraction, AKS node-pool grouping, warnings, and metadata output.discover.mdregisters the fragment;discover-assemble.mdidentifies it as a measured-utilization producer;estimate-infra.mdmarks the RDfA baseline rung as partial.discover-live.mdrenamesnodePoolstonode_poolsso live and RDfA output share one contract.SKILL.mdmoves RDfA from missing to supported with the documented boundaries, andschema-discover-azure.mdadds the unreadable-report warning code.Validation
Validation recorded for current head
e3bade5184a792de37fc288ec849d2f196b2aa13:mise -C plugins/aws-startup-advisor run cipassed.The tests validate the skill and artifact contracts against representative fixtures; they do not run RDfA in a customer tenant. Consumption-quantity parsing, Azure rate-card lookup, and explicit mappings for collectors outside the 17-row table remain unimplemented as described above.
Type of Change
This adds an Azure RDfA discovery capability to an existing skill.
Startup Advisor review checklist
By submitting this PR, I confirm that:
mise -C plugins/aws-startup-advisor run ciand all checks pass.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.