perf(common): extend ComputeDetails with VCPU + MemoryGB by cristim · Pull Request #97 · LeanerCloud/CUDly

cristim · 2026-04-27T00:13:43Z

Summary

Adds VCPU int + MemoryGB float64 (both omitempty) to
common.ComputeDetails so per-provider catalogue lookups can enrich compute
recommendations with sizing data. Foundation work for the deferred Azure /
AWS / GCP compute SKU enrichment tracked in #82.

GetDetailDescription now appends " (N vCPU / M GB)" when both values
are populated; the existing "platform/tenancy" form is preserved when
either is zero (so today's call sites stay unchanged).

Scope decision — schema-only foundation

This PR intentionally lands only the shared-type schema + matcher
formatting + tests. Per-provider catalogue wiring is out of scope:

Azure: PR perf(azure): batched SKU catalogue lookup eliminates N+1 in recommendation converters #81 (perf/azure-batched-sku-lookup) introduces the
cachedSKULookup helper for cache/cosmos/database. Compute was scoped
back from perf(azure): batched SKU catalogue lookup eliminates N+1 in recommendation converters #81 because these struct fields didn't exist yet. With the
schema in place, a follow-up PR can wire compute on top of perf(azure): batched SKU catalogue lookup eliminates N+1 in recommendation converters #81 once it
merges. Filing the wiring against an unmerged PR here would create a
rebase/merge dependency that's better tracked as a separate issue.
AWS: ce.EC2InstanceDetails (Cost-Explorer) has no vCPU/Memory
fields. Populating would require a new ec2:DescribeInstanceTypes
caller + region-scoped catalogue cache + mocks/tests — substantial
net-new code, separate follow-up.
GCP: no compute recommendation converter exists today. N/A until one
is added.
Frontend: api.Recommendation (TS) has no details field and the
detail drawer renders a hard-coded list. Surfacing the new fields needs
separate API serializer + UI changes; tracked as a follow-up once at
least one provider populates the fields.

The schema change is the prerequisite that unblocks every one of those
follow-ups.

Changes

pkg/common/types.go::ComputeDetails gains VCPU + MemoryGB
(omitempty) with a doc comment explaining the catalogue-source
contract.
ComputeDetails.GetDetailDescription() appends the size suffix when
both fields are > 0; uses %g so 16 GB renders without trailing
zeros while 0.5 GB (smallest Azure SKU) renders verbatim.
pkg/common/types_test.go adds 4 table rows covering VCPU-only,
MemoryGB-only (both fall back to base form), integer GB, and
fractional GB.

Test plan

go build ./... (root + pkg/)
go test ./... (root)
go test ./... from pkg/
Pre-commit hooks (gofmt, govet, gosec, gocyclo, etc.) all green
Existing ComputeDetails consumers (Azure converter, AWS parser,
cmd/main.go switch) continue compiling — no field is removed and
omitempty keeps the JSON payload unchanged when fields aren't
populated

Follow-up issues (to file post-merge)

Wire Azure compute to cachedSKULookup after perf(azure): batched SKU catalogue lookup eliminates N+1 in recommendation converters #81 merges
AWS EC2 catalogue lookup via DescribeInstanceTypes
Frontend: add details to api.Recommendation + render in the detail
drawer

Refs #82

Summary by CodeRabbit

New Features
- Compute details now display vCPU and memory sizing information alongside platform and tenancy data when available.

Adds optional VCPU (int) and MemoryGB (float64) fields to common.ComputeDetails so per-provider catalogue lookups can enrich compute recommendations with sizing data. Both fields use json omitempty — converters that don't yet wire a catalogue leave them at the zero value and the API payload stays clean. GetDetailDescription appends " (N vCPU / M GB)" when both values are > 0, otherwise returns the existing "platform/tenancy" form. Format uses %g so 16 GB renders as "16 GB" (not "16.000000 GB") while 0.5 GB SKUs render as "0.5 GB". Scope decision — schema-only foundation: - Azure: PR #81 (perf/azure-batched-sku-lookup) introduces the cachedSKULookup helper for cache/cosmos/database. Compute was scoped back from #81 because these fields didn't exist. With the schema in place, a follow-up PR can wire compute on top of #81 once it merges (filed as a separate issue post-merge to avoid an unmerged-PR dependency in this changeset). - AWS: ce.EC2InstanceDetails (Cost-Explorer) has no vCPU/Memory fields; populating would require a new ec2:DescribeInstanceTypes caller + cache + mocks. Substantial net-new code, tracked as a separate follow-up. - GCP: no compute recommendation converter exists today. N/A. - Frontend: api.Recommendation has no `details` field and the detail drawer renders a hard-coded list. Surfacing the new fields needs separate API + UI work; tracked as follow-up once at least one provider populates the fields. Tests: extend pkg/common/types_test.go::TestComputeDetails_GetDetail Description with 4 new table rows — VCPU-only, MemoryGB-only, integer-GB, and fractional-GB (Azure 0.5 GB SKU shape). Refs #82

cristim · 2026-04-27T00:13:48Z

@coderabbitai review

coderabbitai · 2026-04-27T00:13:50Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 48c6463e-f517-4e6d-a897-711602c55853

📥 Commits

Reviewing files that changed from the base of the PR and between 2e33c88 and de195f9.

📒 Files selected for processing (2)

pkg/common/types.go
pkg/common/types_test.go

📝 Walkthrough

Walkthrough

The ComputeDetails struct gains optional compute sizing fields (VCPU, MemoryGB) with JSON omitempty tags. The GetDetailDescription() method is updated to conditionally append a formatted size descriptor when both fields are populated and greater than zero. Comprehensive test coverage verifies the conditional rendering behavior across multiple scenarios.

Changes

Cohort / File(s)	Summary
Compute Details Enhancement `pkg/common/types.go`	Added `VCPU` (int) and `MemoryGB` (float64) fields to `ComputeDetails` struct. Updated `GetDetailDescription()` to conditionally append `(<vcpu> vCPU / <memory> GB)` suffix when both sizing fields are non-zero.
Test Coverage `pkg/common/types_test.go`	Extended `TestComputeDetails_GetDetailDescription` with table-driven test cases verifying suffix omission when either field is zero, inclusion when both are non-zero, and proper formatting for integer and fractional memory values.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related issues

perf(common): extend ComputeDetails with vCPU + MemoryGB to unlock Azure compute SKU enrichment #82 — Directly addresses the same struct and method modifications to add and populate VCPU and MemoryGB compute sizing metadata.

Poem

🐰 Hop hop, compute details grow,
vCPU and memory now on show!
With omitempty tags so clean,
The finest sizing specs you've seen. ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main change: adding VCPU and MemoryGB fields to ComputeDetails, which is the primary alteration in the changeset.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch perf/computedetails-vcpu-memory

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai · 2026-04-27T00:13:53Z

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

cristim · 2026-04-27T00:34:40Z

@coderabbitai review

coderabbitai · 2026-04-27T00:34:46Z

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

closes #148) (#229) PR #97 added VCPU + MemoryGB fields to common.ComputeDetails (with omitempty JSON tags). PR #81 introduced the lazy SKU-catalogue pattern for cache/cosmosdb/database but explicitly scoped back compute because the destination fields didn't exist. Both prerequisites are now in place; this wires Azure compute to the same pattern. Implementation mirrors cache/database verbatim for consistency: - New unexported vmSKUEntry{vCPUs, memoryGB}. - New skuCacheOnce sync.Once + skuCacheMap field on ComputeClient. - cachedSKULookup(ctx, skuName) lazily triggers the catalogue fetch on first call; subsequent calls are O(1) map lookups. ok=false on miss or fetch error. - fetchSKUCatalogue reuses the existing createResourceSKUsPager and isAvailableInRegion helpers (already used by GetValidResourceTypes), walks every page, and reduces virtualMachines SKUs into the map. Returns nil on error so the sync.Once-gated cache stays nil and converters fall back to the empty-fields path with a one-time WARN. - extractVMSKUCapabilities pulls vCPUs (Atoi) and MemoryGB (ParseFloat) from the SKU's Capabilities name/value list. Unparseable or missing capabilities → 0, treated as "unknown". - populateVMSKUMapFromPage was extracted out of fetchSKUCatalogue to stay under the project's gocyclo=10 threshold (matches the cache/database extraction pattern from PR #81). - convertAzureVMRecommendation now takes ctx (was _) and populates Details.VCPU/MemoryGB from the cache when both >0. The single caller GetRecommendations already passes ctx; no public API change. Behaviour preserved on failure: a transient ResourceSKUsClient error no longer breaks the conversion — VCPU/MemoryGB stay at 0 (omitempty hides them from API payloads), the rest of Details is populated from the recommendation payload as before, and a WARN is logged once per client lifetime. UX improvement: common.ComputeDetails.GetDetailDescription appends " (<vcpu> vCPU / <memory> GB)" when both fields are >0, so Azure VM recommendation summaries now include the size string instead of the SKU name alone. Tests added in providers/azure/services/compute/client_test.go (file-scoped vmSKUCatalogueMockPager keeps the shared mocks.MockResourceSKUsPager surface untouched, mirroring the cosmosdb / cache test convention): - TestComputeClient_ConvertAzureVMRecommendation_PopulatesVCPUAndMemoryFromSKUCache: catalogue hit populates VCPU=2, MemoryGB=8 for Standard_D2s_v3. - TestComputeClient_ConvertAzureVMRecommendation_PagerErrorFallsBack: fetch error leaves both fields at 0; conversion succeeds. - TestComputeClient_ConvertAzureVMRecommendation_NoMatchLeavesFieldsZero: catalogue miss leaves both fields at 0; conversion succeeds. - TestComputeClient_CachedSKULookup_FetchedOnce: 10 lookups trigger exactly 1 NextPage call (pins the N+1 invariant per PR #81). Out of scope (separate follow-ups, per the issue body): - AWS / GCP compute converter wiring. - Platform / Tenancy / Scope enrichment (require non-ResourceSKUs Azure data sources). Verification: gofmt clean; go vet ./... clean; gocyclo -over 10 finds no offenders in the modified file; go test github.com/LeanerCloud/CUDly/providers/azure/services/compute → 42 passing (4 new + all existing). The pre-existing providers/azure/services/search build failure (missing go.sum entry) is unrelated to this change. Closes #148

This was referenced Apr 27, 2026

perf(azure): wire ComputeDetails VCPU/MemoryGB enrichment via cachedSKULookup helper #98

Open

perf(aws): populate ComputeDetails VCPU/MemoryGB from EC2 DescribeInstanceTypes catalogue #99

Open

cristim added triaged Item has been triaged priority/p3 Polish / idea / may never ship severity/low Minor harm urgency/this-quarter Within the quarter impact/few Limited audience effort/xs Trivial / one-liner type/feat New capability labels Apr 28, 2026

cristim merged commit 3ad1ed2 into feat/multicloud-web-frontend Apr 30, 2026
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

perf(common): extend ComputeDetails with VCPU + MemoryGB#97

perf(common): extend ComputeDetails with VCPU + MemoryGB#97
cristim merged 1 commit into
feat/multicloud-web-frontendfrom
perf/computedetails-vcpu-memory

cristim commented Apr 27, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

cristim commented Apr 27, 2026

Uh oh!

coderabbitai Bot commented Apr 27, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related issues

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot commented Apr 27, 2026

Uh oh!

cristim commented Apr 27, 2026

Uh oh!

coderabbitai Bot commented Apr 27, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

cristim commented Apr 27, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Scope decision — schema-only foundation

Changes

Test plan

Follow-up issues (to file post-merge)

Summary by CodeRabbit

Uh oh!

cristim commented Apr 27, 2026

Uh oh!

coderabbitai Bot commented Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related issues

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot commented Apr 27, 2026

Uh oh!

cristim commented Apr 27, 2026

Uh oh!

coderabbitai Bot commented Apr 27, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

cristim commented Apr 27, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Apr 27, 2026 •

edited

Loading