Skip to content

Add x_RecommendationCategory and x_RecommendationSubcategory enrichments to v1_4 Recommendations#2194

Draft
flanakin wants to merge 6 commits into
flanakin/focus14-phase7-tests-changelogfrom
flanakin/focus14-phase9-rec-category
Draft

Add x_RecommendationCategory and x_RecommendationSubcategory enrichments to v1_4 Recommendations#2194
flanakin wants to merge 6 commits into
flanakin/focus14-phase7-tests-changelogfrom
flanakin/focus14-phase9-rec-category

Conversation

@flanakin

@flanakin flanakin commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator

🛠️ Description

Adds two hub-toolkit enrichment columns to the Recommendations dataset in the FOCUS 1.4 hub schema (v1_4): x_RecommendationCategory and x_RecommendationSubcategory, normalized to a fixed, documented hub taxonomy so reports can group recommendations consistently across providers.

  • Source category values (Azure Advisor PascalCase, Cost Management reservation recommendations) are normalized on ingestion; HighAvailabilityReliability.
  • All 22 single-purpose hub-native Resource Graph queries now declare a real subcategory in x_RecommendationDetails (see mapping table below) instead of falling back to Other.
  • A coherence rule guarantees the (category, subcategory) pair is always valid: unknown categories are backfilled from a valid subcategory's parent, a subcategory only passes through when its parent matches the resolved category, and both columns default to Other when unmapped (symmetric defaults).
  • The canonical taxonomy is documented in data-model.md (with a stability statement, source-value mapping table, and cross-category tie-breakers) and enforced by tests that validate the KQL datatable, the docs, and every query file against a single canonical list.

Query → subcategory mapping

All hub-native queries hardcode x_RecommendationCategory = 'Cost'. Subcategories:

Subcategory Queries
Idle Resources BackendlessAppGateways, BackendlessLoadBalancers, EmptyAppServicePlans, EmptyNSGs, EmptySQLElasticPools, IdleVNetGateways, OrphanedNATGateways, StoppedVMs, UnassociatedDDoSPlans, UnattachedDisks, UnattachedNICs, UnattachedPublicIPs, UnprovisionedExpressRouteCircuits
SKU Modernization BasicLoadBalancers, BasicPublicIPs, ClassicAppGateways, LegacyStorageAccounts, UnmanagedDisks
License Optimization SQLVMsWithoutAHB, VMsWithoutAHB
Spot Eligibility NonSpotAKSClusters
Storage Tiering PremiumSnapshots
(per-row, from Advisor) AdvisorCost — heterogeneous per row; no single subcategory applies, so rows fall back to Other until a per-type mapping (for example, keyed on x_RecommendationTypeId) is added

Reservation recommendations from Cost Management exports default to Commitment Discount Coverage.

Decisions

  • Low Utilization = running but underused (resize/consolidate); Idle Resources = doing no useful work, including stopped, unattached, and orphaned resources (stop/deallocate/delete). Unused commitment capacity is explicitly routed to Commitment Discount Utilization rather than overloading Idle Resources.
  • Added License Optimization and Storage Tiering under Cost — both are used by hub-native queries today. Other suggested subcategories (data transfer, cost allocation, marketplace) are noted in the docs as future candidates; nothing in the repo produces them yet.
  • Renamed Identity & AccessIdentity and Access (the only ampersand in the list) and Negotiated Discount EnablementNegotiated Discounts.
  • Other is now an allowed category fallback too, so both columns always have a value.
  • Fixed the High AvailaibilityHigh Availability typo from the design notes.

Files changed

  • .../Analytics/scripts/IngestionSetup_v1_4_Datasets.kql — taxonomy normalization in Recommendations_transform_v1_4() (a SubcategoryTaxonomy datatable of (subcategory, parent) pairs + coherence rules) and both columns in Recommendations_final_v1_4 (the Recommendations transform moved here with the D9 file split)
  • .../Analytics/scripts/HubSetup_v1_4.kql — both columns in the Recommendations_v1_4() projection
  • .../Recommendations/queries/*.json — 22 query files declare x_RecommendationSubcategory in x_RecommendationDetails
  • docs-mslearn/toolkit/hubs/data-model.md — canonical taxonomy, stability statement, source mapping table, normalization rules, tie-breakers
  • docs-mslearn/toolkit/hubs/configure-recommendations.md — subcategory guidance for custom query authors
  • src/powershell/Tests/Unit/HubsFocusSchemas.Tests.ps1Recommendations taxonomy (W9) context (42 tests) validating KQL, docs, and query files against one canonical taxonomy
  • docs-mslearn/toolkit/changelog.md — entry under FinOps hubs v15.0.0

Verification

  • Full unit suite: Invoke-Pester ./src/powershell/Tests/Unit/*2097 total, 2093 passed, 0 failed, 4 skipped.
  • Build-Toolkit finops-hub completes with exit code 0; both v1_4 ingestion setup scripts are under the Bicep 131K loadTextContent() limit (91,445 and 40,494 chars including these changes).
  • KQL functions not yet compiled in Data Explorer / Fabric after merge (pending deployment).
  • Recommendations() returning reservation rows with category Cost / subcategory Commitment Discount Coverage, Advisor HighAvailability rows as Reliability, and hub-native query rows with their mapped subcategories — not yet verified against a live deployment.

Stacked on #2136 (flanakin/focus14-phase7-tests-changelog); merges up the stack toward flanakin/focus14.

📋 Checklist

🔬 How did you test this change?

  • 🤏 Lint tests
  • 🤞 PS -WhatIf / az validate
  • 👍 Manually deployed + verified
  • 💪 Unit tests
  • 🙌 Integration tests

📦 Deploy to test?

  • Hubs + ADX (managed)
  • Hubs + Fabric (manual) — URI:
  • Hubs (manual)
  • Hubs (no data)
  • Workbooks
  • Alerts

🙋‍♀️ Do any of the following that apply?

  • 🚨 This is a breaking change.
  • 🤏 The change is less than 20 lines of code.

🤖 Generated with Claude Code

Adds two hub-toolkit enrichment columns to the Recommendations dataset in
the FOCUS 1.4 hub schema, normalized to a fixed hub taxonomy so reports
can group recommendations consistently across providers.

- IngestionSetup_v1_4.kql: normalize x_RecommendationCategory into the
  5 top-level values (Cost, Operational Excellence, Performance,
  Reliability, Security); map Azure Advisor's HighAvailability -> Reliability;
  derive x_RecommendationSubcategory from the documented taxonomy with
  'Other' as the fallback. Add both columns to Recommendations_final_v1_4.
- HubSetup_v1_4.kql: include both columns in the Recommendations_v1_4()
  project list.
- data-model.md: document the allowed values for both columns and the
  normalization behavior.
- HubsFocusSchemas.Tests.ps1: assert columns exist in the final table,
  the transform projects them, and the case() expression covers all
  5 top-level categories.
- changelog.md: note the new enrichment columns under FinOps hubs v15.0.0.

Co-Authored-By: Michael Flanakin <flanakin@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@flanakin

Copy link
Copy Markdown
Collaborator Author

Taxonomy review — "Pick one" resolutions (Low Utilization, Idle Resources, Unused Capacity)

The PR resolved both of the user's open "pick one" questions and the standalone-Unused-Capacity question. The picks are reasonable but the boundary between Low Utilization and Idle Resources is the single biggest ambiguity in the entire Cost taxonomy and deserves an explicit definition, not just a label list.

Low Utilization (over Underused Resources) — defensible. Low Utilization is closer to the FinOps Framework's "Workload Optimization" capability vocabulary and matches Azure Advisor's own "Low utilization virtual machines" recommendation string. Underused Resources reads as a generic noun phrase that overlaps Right-Sizing semantically. Good pick.

Idle Resources (over Unused Resources) — defensible. Idle Resources is the more common industry term (Azure Advisor "idle public IP address", AWS Trusted Advisor "Idle Load Balancers", FOCUS-aligned terminology). The parenthetical "(includes orphaned)" in the docs is helpful.

Folding Unused Capacity into Idle Resources — this is where I'd push back. The folding rationale ("a separate bucket would have produced overlap") is reasonable in spirit but the resulting label is now overloaded across three distinct cost levers:

  1. Orphaned resources (unattached disks, unassociated public IPs) — a hygiene/cleanup action
  2. Idle running compute (VM at 2% CPU for 30 days) — a stop/deallocate action
  3. Unused reserved/committed capacity (paid-for RI hours not consumed, unused savings plan commitment) — a commitment-management action

Item 3 is fundamentally different from 1 and 2 — the user has already paid for it, the resource isn't actually idle from the provider's POV, and the remediation lives in commitment portfolio management, not resource cleanup. Lumping unused commitment hours under Idle Resources will make reports misleading and conflict with how FinOps practitioners think about rate vs. usage optimization.

Recommendation: Either (a) keep Idle Resources covering #1 and #2 only and route "unused commitment hours" into Commitment Discount Utilization (which already exists and is the correct semantic home), or (b) document explicitly in data-model.md that "unused capacity" here means unattached/orphaned provisioned capacity, not unused committed capacity. Today's phrasing "includes unused capacity and orphaned resources" reads as if all three cases land in Idle Resources, which would degrade signal quality.

🤖 [AI][Claude Code] Taxonomy review

@flanakin

Copy link
Copy Markdown
Collaborator Author

Taxonomy review — Cross-category overlap and routing ambiguity

A single subcategory list per parent category is clean, but several subcategories sit on category boundaries where ingestion will have to make a judgment call. These need either (a) tie-breaker rules documented next to the taxonomy, or (b) renames that make the boundary obvious.

Right-Sizing (Cost) vs Resource Sizing (Performance) — biggest concern. The same recommendation ("upgrade to a larger SKU because the workload is throttling") could land in either. In practice, Azure Advisor surfaces sizing recs under both Cost (downsize to save) and Performance (upsize to relieve pressure). The current labels don't make the direction explicit. Suggest renaming Performance's Resource Sizing to Performance Sizing or Capacity Sizing to signal "size up for SLO" vs "size down for cost".

Scaling and Capacity (Performance) vs Autoscaling (Cost) vs Capacity (Reliability) — three scaling buckets across three categories. The intended split is presumably: Autoscaling = cost lever (scale down when idle), Scaling and Capacity = perf lever (scale out for throughput), Capacity = reliability lever (headroom for failover/spike). That's defensible but invisible to a reader of the label list. At minimum, rename Reliability's Capacity to Capacity Headroom or Capacity Reservation to disambiguate from Performance's Scaling and Capacity.

Quotas and Limits (OpEx) vs Capacity (Reliability) — quota exhaustion is simultaneously an operational hygiene issue (raise the quota) and a reliability issue (failover region runs out of vCPUs). Suggest documenting that quota recs always land in OpEx, and Capacity (Reliability) is reserved for consumed-vs-available capacity recs.

Backup Configuration (Reliability) vs Data Protection (Security) — overlap is real. Backup encryption, immutability, ransomware vaults, and cross-region backup copies all read as both. Suggest documenting that backup-availability recs (RPO, retention, geo-redundancy) land under Reliability, while backup-confidentiality recs (encryption, access policies on backup vaults) land under Security.

Resource Configuration (Performance) vs Resource Consistency (OpEx) — these labels are nearly synonymous to a non-expert reader. Consider renaming Performance's Resource Configuration to Performance Configuration or Tuning Configuration. As written, an Advisor rec like "enable accelerated networking" could land in either.

Service Retirement (Reliability) vs Operational Hygiene (OpEx) — service retirement (e.g., Classic VMs going EOL) is operational debt as much as a reliability risk. Defensible to keep under Reliability since the consequence is downtime, but worth noting in the docs.

Without disambiguation rules, two Hub deployments ingesting the same Advisor stream could classify identical recommendations differently depending on which case() branch fires first. That kills cross-tenant report comparability — the explicit goal of having a fixed taxonomy.

🤖 [AI][Claude Code] Taxonomy review

@flanakin

Copy link
Copy Markdown
Collaborator Author

Taxonomy review — Missing subcategories

The Cost category is the most consequential one for a FinOps Hub, and several very common cost-recommendation types have no home in the current taxonomy and would all fall into Other. That defeats the purpose of having a fixed taxonomy for reporting.

Critical gaps (would already be hit by Azure Advisor, Cost Management, and partner tools today):

  1. License optimization — Azure Hybrid Benefit, BYOL, SQL Server license mobility, Windows Server license sync. Azure Advisor explicitly surfaces "Consider Azure Hybrid Benefit" recs. These are neither right-sizing nor commitment discounts. Suggest adding License Optimization under Cost.

  2. Storage tier / lifecycle — recommendations to move blobs to cool/archive, enable lifecycle management, delete old snapshots. This is a huge category in practice and doesn't map cleanly to Idle Resources (the data isn't idle, it's just on the wrong tier) or Right-Sizing. Suggest adding Storage Tiering or Storage Lifecycle under Cost.

  3. Data transfer / egress — recommendations to use Private Link, ExpressRoute Direct, PoP-local egress, or to relocate workloads to reduce inter-region transfer cost. Today these would fall to Region Placement or Other. Region Placement is overloaded if it's also the home for egress optimization. Suggest adding Data Transfer or Network Cost under Cost.

  4. Tagging / cost allocation hygiene — recs to apply missing tags, normalize tag values, or enforce allocation tagging. These are technically Cost (they affect allocation) but operationally OpEx (they're a hygiene action). Suggest adding Cost Allocation under Cost, or explicitly documenting that tagging recs land under Governance and Policy (OpEx).

Important gaps:

  1. Marketplace / third-party SaaS — Azure Marketplace spend is a growing category; recs around marketplace subscription cleanup, plan downgrades, BYOL on marketplace SKUs have no home. Suggest Marketplace Optimization under Cost.

  2. Free tier / trial expiration — particularly relevant for sandbox/dev workloads moving off free credits. Could fit under Operational Hygiene (OpEx) but worth calling out.

  3. Currency / billing-currency hedging — niche but real for enterprises with multi-currency MCA. Probably acceptable to leave in Other.

Cross-cloud / multi-cloud allocation — the user's question implied this is in scope. If the hub will normalize AWS Cost Explorer / GCP Recommender recs (per the FOCUS framing), categories like "Reserved Instance modification" (AWS) and "Committed Use Discount" (GCP) already map cleanly to existing Cost subcategories — that's fine. But "Savings Plans coverage gap", "Compute Optimizer rightsizing", and "GCP Recommender idle VM" need test coverage to confirm the mapping holds. Worth adding test cases.

The taxonomy currently captures the FinOps Framework's "Workload Optimization" and "Rate Optimization" capabilities reasonably well but misses most of "Architecting for Cloud" (license, tier, transfer) and parts of "Allocation".

🤖 [AI][Claude Code] Taxonomy review

@flanakin

Copy link
Copy Markdown
Collaborator Author

Taxonomy review — Naming consistency and label hygiene

The taxonomy is mostly noun-phrased and Title-Cased, but a few inconsistencies will create friction for downstream consumers (filters, dropdowns, KQL in (…) lists).

Hyphen vs space inconsistency:

  • Right-Sizing (Cost) is hyphenated.
  • Resource Sizing (Performance) is not.
  • SKU Modernization, Resource Configuration, Resource Consistency are space-separated.

Either keep the hyphen for Right-Sizing (common industry term, defensible) or normalize to Right Sizing. Today's mix means an end user filtering by "sizing" recs has to remember which one is hyphenated. If you keep the hyphen, document why (it's a single compound term) so future additions follow the same rule.

& vs and:

  • Identity & Access (Security) uses an ampersand.
  • Every other multi-word label uses and (Automation and Process, Governance and Policy, Quotas and Limits, Scaling and Capacity, Throughput and Latency, Service Selection and Architecture).

This is the most jarring inconsistency. Suggest renaming to Identity and Access for consistency. Bonus: it matches the more common framework name "Identity and Access Management (IAM)".

Verb vs noun phrasing: mostly noun-phrased, which is good. The two outliers worth checking:

  • Threat Detection (Security) — noun phrase, fine.
  • Negotiated Discount Enablement (Cost) — wordy and ambiguous. "Enablement" implies the rec is to enable a negotiated discount, but in practice these recs detect that a customer is not benefiting from a negotiated discount they already have. Suggest Negotiated Discount or Negotiated Pricing — drop Enablement.

CapitalizationTitle Casing is used throughout, which is correct for a closed enum. Confirm the KQL case() expression is case-sensitive (it appears to be — in ('Cost', ...) not =~). That means a producer writing cost instead of Cost would fall through to the default. That's probably intentional (forces normalization on ingestion) but worth documenting.

"Other" placementOther appears in the in-list of valid subcategories and is the explicit fallback. That's fine but means a source system could legitimately set x_RecommendationSubcategory = 'Other' and have it pass through, which is indistinguishable from "we didn't have a mapping". Consider documenting that Other from an upstream source is treated identically to the default fallback (no information loss, but no provenance either).

High Availaibility typo — fixed in this PR. Good.

🤖 [AI][Claude Code] Taxonomy review

@flanakin

Copy link
Copy Markdown
Collaborator Author

Taxonomy review — KQL mapping logic and default behavior

The case() expression in Recommendations_transform_v1_4() does the right thing structurally but has a few semantic issues worth tightening before this becomes the canonical normalization path.

Asymmetric defaults — category vs subcategory:

  • x_RecommendationCategory defaults to empty string ('') for unknown values.
  • x_RecommendationSubcategory defaults to 'Other' for unknown values.

This asymmetry is documented in the PR description, but it has a usability consequence: a row can legitimately have x_RecommendationCategory = '' and x_RecommendationSubcategory = 'Commitment Discount Coverage' (if x_SourceType == 'ReservationRecommendations' is false but the upstream value happened to be in the allow-list). That's an inconsistent state that downstream reports won't expect.

Suggest one of:

  • (a) Always populate both. If category is unknown, force subcategory to Other (don't let an allow-listed subcategory leak through under an empty category).
  • (b) Validate the (category, subcategory) pair against the documented hierarchy. Today a row could have Category = Cost, Subcategory = Threat Detection if the upstream details object lies about both. Worth a parent-child validation pass.

HighAvailabilityReliability mapping is correct — confirmed against Azure Advisor's documented Category enum (Cost, HighAvailability, OperationalExcellence, Performance, Security). Good.

Reservation default to Cost — correct. Reservation recommendations from Microsoft.Consumption/reservationRecommendations don't carry a Category, and they're definitionally cost recs.

Hard-coded reservation subcategory — the fallback x_SourceType == 'ReservationRecommendations'Commitment Discount Coverage is correct for Azure RIs but assumes the source is always a coverage rec, never a utilization rec. Cost Management does surface both:

  • reservationRecommendations API → coverage (which SKU to buy)
  • reservationDetails / utilization API → utilization (what % of purchased RIs are being consumed)

If the hub ever ingests reservation utilization recs under the same x_SourceType, they would be miscategorized as Coverage when they should be Commitment Discount Utilization. Worth adding a discriminator (e.g., based on x_RecommendationDetails shape) or splitting x_SourceType values.

=~ (case-insensitive equality) for Advisor values — fine, but redundant since the documented Advisor enum is fixed PascalCase. If you want to harden against unexpected casing from undocumented sources, =~ is a small cost. If you want strict provenance, == would be more honest. Either is defensible; just be consistent.

No mapping for Microsoft Defender for Cloud / Microsoft Sentinel recs — these flow through a different API and category schema (Microsoft.Security/assessments). If the hub ever ingests them, the case() would drop them all to empty-category. Worth a follow-up to add a MicrosoftDefenderForCloudSecurity branch when that source is wired up.

Subcategory case() ordering — the allow-list check fires first, then the reservation fallback, then Other. That means a reservation rec whose upstream details happen to set x_RecommendationSubcategory to a different valid value (e.g., Idle Resources) would pass through as Idle Resources, not Commitment Discount Coverage. Probably intentional (trust the upstream) but worth a comment in the KQL to make the precedence explicit.

🤖 [AI][Claude Code] Taxonomy review

@flanakin

Copy link
Copy Markdown
Collaborator Author

Taxonomy review — Documentation completeness and canonical-vs-open status

The taxonomy is documented in data-model.md as a bulleted list, which is fine for human reading but insufficient for a fixed taxonomy meant to drive cross-tenant reporting.

Is this a closed enum or an open vocabulary? The PR description says "fixed hub taxonomy" but the docs don't explicitly say "this list is canonical and will not change without a schema version bump". Without that statement:

  • A consumer building a Power BI report can't know whether to hard-code a slicer against these values.
  • A partner ingesting recs into the hub can't know whether to invest in mapping to this list vs. waiting for it to stabilize.
  • A future contributor doesn't know whether adding a new subcategory is a breaking change.

Suggest adding to data-model.md:

The lists below are canonical for the FOCUS 1.4 hub schema. New values may be added in a future schema version; existing values will not be renamed or removed without a schema version bump.

No queryable form — the taxonomy lives in (a) the KQL case() expression, (b) the docs markdown, and (c) the Pester test. These three sources are not linked; if one drifts from another, no automation catches it. Worth considering:

  • A Recommendations_taxonomy_v1_4() KQL function that returns the (category, subcategory) pairs as rows. Consumers could join against it to validate or to populate UI dropdowns.
  • Or an open-data CSV under src/open-data/ (the repo already has this pattern for regions and services). Then docs and KQL both derive from a single source.

Today, a Power BI author building a Recommendations report has to either (a) re-type the list from the docs, or (b) summarize by x_RecommendationSubcategory against live data, which won't surface taxonomy values that haven't been seen yet.

Mapping table to source values is missing — the docs say "Azure Advisor's HighAvailabilityReliability" as an example but don't document the full mapping. A consumer who sees x_RecommendationCategory = 'Reliability' and wants to know which Advisor categories that includes has to read the KQL. Worth adding a small table in data-model.md:

Source Source value Hub category
Azure Advisor Cost Cost
Azure Advisor HighAvailability Reliability
Azure Advisor OperationalExcellence Operational Excellence
Azure Advisor Performance Performance
Azure Advisor Security Security
Cost Management ReservationRecommendations Cost

This becomes essential when you add a second source (Defender for Cloud, AWS Trusted Advisor, GCP Recommender).

Changelog entry — the entry calls out the columns but not the taxonomy decisions (Low Utilization vs Underused, Idle Resources covers unused capacity, etc.). Those decisions are the load-bearing part of this PR and deserve a one-line callout so a v15 consumer doesn't have to read the PR to understand the breaking semantic.

🤖 [AI][Claude Code] Taxonomy review

flanakin and others added 3 commits July 15, 2026 14:02
Resolves conflicts from the upstream IngestionSetup_v1_4.kql split (D9):
- IngestionSetup_v1_4.kql: take the phase7 split (Recommendations and other
  supplemental datasets moved to IngestionSetup_v1_4_Datasets.kql)
- IngestionSetup_v1_4_Datasets.kql: re-apply this PR's Recommendations
  category/subcategory normalization where the transform now lives
- HubsFocusSchemas.Tests.ps1: keep the phase7 rewrite; taxonomy assertions
  auto-merged as an additional context
- changelog.md: keep both sides' entries; ms.date set to today

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…and enforce category coherence

Addresses the W9 review feedback on the recommendation taxonomy:

- Declare x_RecommendationSubcategory in all 22 single-purpose hub-native
  Resource Graph queries (Idle Resources, SKU Modernization, Spot
  Eligibility, License Optimization, Storage Tiering) so they no longer
  fall back to 'Other'. AdvisorCost is per-row heterogeneous and keeps
  the transform fallback.
- Replace the flat subcategory allow-list in Recommendations_transform_v1_4
  with a SubcategoryTaxonomy datatable of (subcategory, parent) pairs, and
  enforce coherence: unknown categories are backfilled from a valid
  subcategory's parent, and a subcategory only passes through when its
  parent matches the resolved category.
- Make defaults symmetric and deliberate: both columns always have a value
  and unmapped values become 'Other' for both.
- Taxonomy hygiene: add License Optimization and Storage Tiering (both used
  by hub queries today); rename Identity & Access -> Identity and Access
  and Negotiated Discount Enablement -> Negotiated Discounts; keep unused
  commitment capacity under Commitment Discount Utilization instead of
  overloading Idle Resources.
- Document the canonical taxonomy contract, source-value mapping table,
  normalization rules, and cross-category tie-breakers in data-model.md;
  document subcategory guidance for custom queries in
  configure-recommendations.md.
- Rework the taxonomy test context: canonical taxonomy as test source of
  truth validated against both the KQL datatable and the docs, coherence
  rule assertions, and per-query subcategory validation for all 22 hub
  query files.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@flanakin
flanakin changed the base branch from flanakin/focus14 to flanakin/focus14-phase7-tests-changelog July 15, 2026 21:17
@flanakin

Copy link
Copy Markdown
Collaborator Author

W9 fix pass — responses to the 6 taxonomy review comments

All 6 review comments are addressed below. Merged flanakin/focus14-phase7-tests-changelog (D9 file split + test harness rewrite); the Recommendations transform edits now live in IngestionSetup_v1_4_Datasets.kql. Verification: full unit suite 2097 total / 2093 passed / 0 failed / 4 skipped; Build-Toolkit finops-hub exit 0; v1_4 ingestion scripts are 91,445 + 40,494 chars — both well under the 131,072 Bicep loadTextContent() limit, so the BCP184 break can't recur.

1. Low Utilization / Idle Resources / Unused Capacity overlap — addressed (option a + b)

Took the recommendation: Idle Resources covers orphaned/unattached resources and idle running compute only; unused commitment capacity is routed to Commitment Discount Utilization. data-model.md now carries crisp one-line definitions for both (Low Utilization = running but underused → resize/consolidate; Idle Resources = no useful work → stop/deallocate/delete) and the misleading "includes unused capacity" phrasing is gone.

2. Cross-category routing ambiguity — addressed via documented tie-breakers; renames deliberately deferred

Tie-breaker rules are documented inline in the taxonomy list in data-model.md: Right-Sizing (down for cost) vs Resource Sizing (up for performance), Autoscaling vs Scaling and Capacity, Capacity = headroom for failover/spikes, Quotas and Limits always wins for quota recs, Backup Configuration (availability) vs Data Protection (confidentiality). The suggested renames (Performance Sizing, Capacity Headroom, etc.) were deliberately not taken: no source in the repo emits any Performance/Reliability subcategory today, so the docs disambiguation achieves the comparability goal with zero churn, and the labels can still be adjusted before v1_4 ships if reviewers prefer.

3. Missing subcategories — partially addressed, grounded in what the repo produces today

Added License Optimization (used by VMsWithoutAHB, SQLVMsWithoutAHB) and Storage Tiering (used by PremiumSnapshots). Data transfer, cost allocation/tagging, and marketplace subcategories were not added — nothing in the toolkit produces them yet — and are documented in data-model.md as future candidates. Beyond that, all 22 single-purpose hub-native queries now declare a real subcategory (13× Idle Resources, 5× SKU Modernization, 2× License Optimization, 1× Spot Eligibility, 1× Storage Tiering; full table in the PR body). Judgment calls for owner review: retired-SKU cleanups (BasicLoadBalancers, BasicPublicIPs, ClassicAppGateways, LegacyStorageAccounts, UnmanagedDisks) → SKU Modernization rather than Reliability's Service Retirement, since the queries classify them as Cost; PremiumSnapshots → Storage Tiering rather than Right-Sizing. AdvisorCost stays per-row (heterogeneous); a follow-up could map well-known Advisor recommendationTypeId values.

4. Naming hygiene — addressed

Identity & AccessIdentity and Access; Negotiated Discount EnablementNegotiated Discounts. Kept Right-Sizing hyphenated as a compound industry term (matching Advisor's "right-size"); kept Title Casing to match FOCUS enum conventions (e.g., On-Demand). Case-insensitive matching on ingestion with exact-cased storage is now documented, as is the "source Other is indistinguishable from unmapped" caveat.

5. KQL defaults and coherence — addressed

The asymmetry is gone. Decision (reflected in data-model.md, the deciding doc): both columns always have a value and both default to OtherOther is now an allowed category fallback. Rationale: report slicers get no blank buckets, and the rule is one sentence. Coherence is enforced structurally: the transform uses a SubcategoryTaxonomy datatable of (subcategory, parent) pairs; an unknown category is backfilled from a valid subcategory's parent, and a subcategory only survives when its parent matches the resolved category (so Cost + Threat Detection is impossible — the parent-child validation suggested in the comment). Subcategory precedence (valid source value > reservation default > Other) is commented in the KQL. The reservation coverage-vs-utilization discriminator is a fair point but the hub only ingests reservationRecommendations (coverage) today; noted for when a utilization source is wired up. =~ is kept, and now documented as deliberate hardening.

6. Canonical-enum documentation — addressed; queryable form deferred

data-model.md now states the lists are canonical for the FOCUS 1.4 hub schema (additive changes only; renames/removals require a schema version bump), includes the full source → hub category mapping table (Advisor × 5, Cost Management reservations, hub queries), and the changelog entry calls out the load-bearing taxonomy decisions. The drift problem is solved with automation rather than a fourth artifact: the Pester context Recommendations taxonomy (W9) holds one canonical (subcategory → parent) list and validates the KQL datatable, data-model.md, and all 22 query files against it (42 tests; verified they fail on mutation). A Recommendations_taxonomy_v1_4() KQL function / open-data CSV is deferred — the datatable in the transform is trivially promotable to one if a consumer asks for it.

🤖 [AI][Claude Code] W9 fix pass

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@flanakin

Copy link
Copy Markdown
Collaborator Author

Synced with phase 7, completing the dev sync through the full FOCUS 1.4 stack. Clean merge, no conflicts (changelog entries auto-merged).

This pulls in from upstream: the origin/dev merge (PR deploy workflows, v15 dev cycle, ingestion OOM fix #2151, price lookup max() fix #2189), the ports of those KQL fixes into the v1_4 transforms, and the Q4 ContractCommitments spec alignment (30 spec-prefixed FOCUS columns) with its test and changelog updates.

Verified: Build-Toolkit finops-hub exit 0; full unit suite green (2165 passed, 0 failed, 4 skipped); no conflict markers.

@github-actions

Copy link
Copy Markdown
Contributor

Hubs + ADX (managed) deployed to pr-2194-adx.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs: Review 👀 PR that is ready to be reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants