Improve plan parser completeness and support optional() type defaults

[Tyron2k]

Two parser fixes that unblock our Terraform codebases (tested against a production Azure environment with 90+ resources across nested modules), plus a --show-delta flag that builds on the plan-parser work.

Problem

Plan parser: empty estimates from plan JSON

The parser read only resource_changes and skipped no-op actions. In a typical plan where 95% of resources are unchanged, the estimate came back near-zero. Additionally, provider_config.expressions used a strict struct that panicked on Terraform 4.x array-valued expressions (azurerm emits features as [{}]).

HCL parser: crash on optional() type constraints

variable "vm" {
  type = object({
    os_disk = optional(object({
      disk_size_gb = optional(number, 64)
    }), {})
  })
}

When the caller omits optional keys, the parser stored a source-range string as the value. Catalog expressions like default(os_disk_disk_size_gb, 0) > 0 then failed with type mismatches.

Fix

Plan parser rewrite (internal/parser/plan)

• Walk planned_values for the complete post-apply resource set
• Annotate each resource with its PlanAction from resource_changes
• Exclude resources scheduled only for deletion
• Use json.RawMessage for provider expressions (handles arrays gracefully)
• Allocate region pointer once per tree walk, not per-resource
• Fallback to resource_changes for older plan formats

HCL optional() defaults (internal/parser/terraform)

• New applyOptionalDefaults(): parses the type-constraint AST, finds optional(type, default) calls, merges defaults into caller-supplied variables — matching Terraform's runtime behavior
• Recurses to arbitrary depth via applyChildDefaults / synthesizeFromChildren
• Unresolvable attributes store nil instead of a string placeholder, so default(x, fallback) expressions fall through correctly

Delta rendering (internal/render, cmd/c3x)

• domain.Cost carries PlanAction from the source resource
• RenderTextDelta(): shows only changed resources with +/~/- markers, summarizes unchanged in a footer
• c3x estimate --show-delta activates the delta view
• Falls back to standard renderer for formats without a delta-specific implementation

Validation

$ go test -race ./...   # all 19 packages pass
$ golangci-lint run     # 0 issues

End-to-end against a production Azure environment (AKS, PostgreSQL Flexible Server, App Gateway WAFv2, File Shares, Redis, Container Registry, FTP VMs):

Before: crash / $0
After:  $1,527.23/mo with 10 non-critical warnings
        (all for unresolvable data-source refs in diagnostic settings)

Delta mode example

$ c3x estimate --path ./plan.json --show-delta

── c3x plan changes · USD ─────────────────────────────────────────

  + azurerm_linux_virtual_machine.module.ftp_vm_003.vm
    Virtual machine
      730 hours × $0.178 = $129.94/mo
    azurerm_linux_virtual_machine.module.ftp_vm_003.vm subtotal: $129.94/mo

  ~ azurerm_postgresql_flexible_server.module.flex_postgresql.this
    Database compute (D4ads_v5, 4 vCore)
      2920 vCore-hours × $0.178 = $519.76/mo
    azurerm_postgresql_flexible_server.module.flex_postgresql.this subtotal: $519.76/mo

  ... 85 unchanged resources: $1,397.29/mo

  ────────────────────────────────────────────────────────────
  PROJECT TOTAL: $2,047.00/mo

Commits

#	Summary
1	fix(plan): rewrite plan parser to read planned_values with action annotations
2	fix(terraform): resolve optional() defaults from module variable type constraints
3	feat(render): add --show-delta flag with plan-aware delta rendering

[andreacappelletti97]

Thanks for this — it's a substantial and genuinely useful contribution, and it shows you've run it against real infrastructure. I went through it in depth.

I'm requesting changes on two crash bugs that block merge, since c3x parses untrusted plan/HCL on a hosted backend and a panic there is an availability issue:

1. plan.go — nil-pointer panic on a null child_modules element (the inner recursive walk has no mod == nil guard).
2. optional_defaults.go:112 — index-out-of-range panic on a zero-arg object() type constraint.

One behavior change needs an explicit call: estimate plan.json (no --show-delta) now prices the whole post-apply fleet instead of only the changes — that can move --budget gates and the JSON payload for existing users, so let's document it.

A few correctness items (over-estimating null optional(object) synthesis; the old-format fallback still dropping no-op resources; applyOptionalDefaults not covering the root module) and some --show-delta polish (misleading output on plain .tf, dead delete-marker, full-fleet total in delta view) are inline.

Really nice work overall — happy to help land it once the two panics are handled. Details on specific lines below.

[andreacappelletti97]

collectPlannedResourcesInner dereferences mod (mod.Resources, mod.ChildModules) with no nil check, and the recursive call below can pass a nil child. A plan JSON with a null element in child_modules (e.g. "child_modules":[null]) decodes to a nil *plannedModule and panics here. Since we parse untrusted plan JSON on the backend, this needs a if mod == nil { return } guard — the exported collectPlannedResources guards it, but this inner recursion doesn't.

[Tyron2k]

Fixed — since your #52 already has the nil-guard on the exported collectPlanned, and the function is no longer split into inner/outer in this PR, this is covered.

[andreacappelletti97]

nestedCall.Args[0] has no arg-count guard, so a type constraint like object({ x = object() }) (zero-arg object()) panics with index out of range. Line 136 guards the same pattern with && len(innerCall.Args) == 1; the same check is needed here. With no recover() on the parse path this crashes the process on untrusted HCL.

[Tyron2k]

Added len(nestedCall.Args) == 1 guard — good catch, matches the pattern on line 136.

[andreacappelletti97]

Semantics change worth calling out: walking planned_values means c3x estimate plan.json (without --show-delta) now prices the entire post-apply fleet rather than only the changed resources. That's a defensible definition of 'estimate', but it silently changes PROJECT TOTAL, can flip existing --budget CI gates, and changes the --format json payload for anyone feeding plan JSON today. Can we note this in the changelog/README?

[Tyron2k]

Agreed — this is now documented in your #52 commit message and I added a CHANGELOG entry on this PR as well. The --show-delta flag gives users who want the old change-only view a path forward.

[andreacappelletti97]

When optional(object({...})) has no explicit second-arg default, Terraform leaves the attribute null, but here we synthesize a fully-populated object from the inner defaults (synthesizeFromChildren). That fabricates values that don't exist (e.g. a phantom disk) and over-estimates cost. Only the optional(object({...}), {}) form (explicit {}) should get the synthesized object; the no-default form should stay null.

[Tyron2k]

You're right — removed synthesizeFromChildren entirely. Now only optional(object({...}), {}) with an explicit 2nd arg produces a populated object. No-default stays null, no phantom costs.

[andreacappelletti97]

The older-format fallback still runs resources through shouldSkip, which filters no-op (and delete) actions. Since the primary path now intentionally keeps no-op resources, an all-unchanged plan in the old format returns 0 resources here — inconsistent with the new behavior. Worth aligning the fallback with the primary walk.

[Tyron2k]

Aligned — the fallback now only excludes isDeleteOnly (consistent with the primary path from #52). No-op resources are kept.

[andreacappelletti97]

applyOptionalDefaults is only called for child-module vars. Root-module variables never get their optional(type, default) defaults applied, so a root resource referencing var.x.<optional-attr> still resolves to nil. Should the root module get the same treatment?

[Tyron2k]

Good point — added applyOptionalDefaults(sources, variables) as Stage 2b in parser.go, right after CLI vars are applied. Root module variables now get the same treatment.

[andreacappelletti97]

.tf resources carry PlanActionNone, which this treats as 'unchanged', so c3x estimate main.tf --show-delta on real infra prints 'No cost-affecting changes' and collapses every resource into the footer. That's misleading when there's no plan. Consider warning/disabling --show-delta for non-plan inputs, or treating PlanActionNone as shown when there is no plan context.

[Tyron2k]

Fixed — writeRendered now checks estimateHasPlanContext() (any Cost with a non-empty Action). If there's no plan context it prints a warning to stderr and falls back to the standard renderer. Flag description also updated to say "plan JSON only".

[andreacappelletti97]

This - (delete) marker is effectively dead code: pure-delete resources are excluded upstream (buildDeletedSet on the planned_values path, shouldSkip on the fallback), so PlanActionDelete never reaches a rendered Cost. The --show-delta help advertises 'create/update/delete', so either surface deletions (with their cost removed from the total) or drop the claim. Related: the delta PROJECT TOTAL (line 105) prints the full-fleet est.ProjectTotal, not the delta cost, which is confusing in the delta framing.

[Tyron2k]

Both addressed: deleted resources are now appended to the output with their change.before attributes and PlanActionDelete, so the - marker is reachable. The footer shows a DELTA: +/-$X/mo line (net of creates minus deletes) followed by PROJECT TOTAL for full-fleet context.

[andreacappelletti97]

On the two panics — both are one-liners if it helps:

• internal/parser/plan/plan.go: early return on a nil *plannedModule so a null element in child_modules can't panic the recursive walk (the exported entry guards it; the inner recursion doesn't).
• internal/parser/terraform/optional_defaults.go: len(nestedCall.Args) == 1 before indexing Args[0], matching the guard already a few lines below.

Both want a small regression test. Happy to help land them.

[andreacappelletti97]

Heads up: I landed the full-fleet plan estimate in #52 (the #51 decision), so estimate <plan.json> now walks planned_values on main with the null-child_modules guard and a keep-unchanged fallback.

That lets this PR rescope to the parts that are uniquely yours and still very much wanted, now on a consistent base:

• the --show-delta change view (with a proper delta total, per the review notes), and
• the optional() type-default resolution.

The two panic guards from the review are one-liners (described inline). Thanks again for surfacing all of this.

[Tyron2k]

Hey Andrea, thanks for the thorough review — really appreciated that you went through this line by line.

Rebased on current main (your #52 covers the planned_values foundation nicely, so I built on top of it instead of duplicating). All your comments are addressed:

Crash fixes:

• Zero-arg object() guard added in extractObjectDefaults
• json.RawMessage for provider expressions (azurerm features array no longer blows up)

Behavior corrections:

• Dropped synthesizeFromChildren — you were right, no-default optional(object(...)) should stay null, not fabricate phantom resources
• applyOptionalDefaults now covers root-module variables too
• Fallback path aligned: keeps no-op resources like the primary walk

--show-delta polish:

• Deleted resources surface with - marker (using change.before attrs)
• Added a DELTA: +$X/mo line so the diff framing actually shows the diff cost
• Warns and falls back gracefully on .tf input where there's no plan context

CHANGELOG added to document the new flag and the fixes. Let me know if anything else needs attention!

[andreacappelletti97]

Re-reviewed end to end — really nice work. All eight items from the last round are fixed and verified: build + go test -race pass on every touched package, and I exercised --show-delta on a plan and on plain .tf. The panic guards, the synthesizeFromChildren removal, root-module optional() defaults, the no-op fallback alignment, and the delete marker + DELTA line all check out.

One regression to fix before merge, in the core (non-delta) path — details inline. Once that's handled I'll squash-merge.

[andreacappelletti97]

Appending pure-delete resources into the same out slice the calculator prices means they land in PROJECT TOTAL — which regresses the #52 semantics: estimate plan.json without --show-delta now counts resources that won't exist post-apply.

Repro with a plan of {create t3.small, no-op t3.micro, delete t3.medium}: PROJECT TOTAL prints $53.14, but the post-apply fleet is $22.77 (the two survivors). --show-delta shows the same inflated $53.14.

Deletes shouldn't contribute to the post-apply total, and shouldn't appear at all in the non-delta view. Suggest surfacing them to the delta renderer via a separate field (or excluding PlanActionDelete from the priced total) so the - marker and the DELTA line keep working while PROJECT TOTAL stays the post-apply fleet.

[Tyron2k]

Good catch — you're right, appending deletes into out broke the #52 semantics. Fixed in 63b6d0f:

• NewEstimate now skips PlanActionDelete when summing ProjectTotal
• Standard renderer hides deleted resources entirely (they only surface in --show-delta)
• Action markers (+/~/-) scoped to delta mode only — normal view has no prefixes

Verified with your repro scenario: estimate plan.json shows only the two survivors, --show-delta shows all three with correct markers and a separate DELTA line.

[andreacappelletti97]

Merged. Thanks for the thorough back-and-forth on this — the delete-total fix is exactly right, and the whole change (delta view, root + child optional() defaults, and the two panic guards) is a genuinely valuable addition tested against real infrastructure. Really appreciate the careful work across the review rounds. 🙏