fix(tools): restore drift-report visibility lost to helper refactors and parser gaps

[juemerson-at-purestorage]

Problem

tools/lib/PfbCmdletParamTools.ps1's Get-PfbWireNameForParameter resolved a cmdlet
parameter's REST wire name by finding exactly one AST shape — an index assignment into a
hashtable named body/queryParams:

if ($Sort) { $queryParams['sort'] = $Sort }

The codebase uses at least four idioms for the same thing. A parameter matching none of
them gets WireName = $null and a non-Typed surface, and Get-PfbParameterCoverageGaps
then discards the entire endpoint into its notVerified bucket — so the drift report
silently stops analysing it. A separate bug discarded endpoints whose cmdlet declares no
inventoried parameters at all.

Two consequences, both measured:

1. PR Centralize -Filter/-Sort/-Limit/-TotalOnly query params (#32) #49 (issue Centralize -Limit/-Sort/-Filter/-TotalOnly query-param handling in Invoke-PfbApiRequest #32, already merged) regressed the report. Migrating 30 Get-Pfb*
   cmdlets onto the shared Add-PfbCommonQueryParams helper removed their literal
   $queryParams['names'] = ... lines. Those parameters are still declared and still reach
   the wire — the inventory just can't see them. notVerified went 164 → 194.
2. PR Migrate remaining Get-Pfb* cmdlets to Add-PfbCommonQueryParams (#33) #51 (issue Migrate cmdlets to centralized query-param handling #33) would take it much further — 196 cmdlets on the helper, pushing
   notVerified to 349 and dropping analysable endpoints to 132 of 481 (27%).

The dangerous part is the direction of the error: the report's headline field count falls
932 → 571, which reads as a 39% improvement when it is actually blindness. The refactors are
correct; the tooling's heuristic was incomplete.

Fix

tools/ + Tests/ only — no Public/, Private/, Data/, Reports/, .github/ or
.gitignore changes.

Idiom 2 — shared-helper delegation. Get-PfbCommonQueryParamMap is a documented mirror of
Private/Add-PfbCommonQueryParams.ps1 with two detection rules, because the helper learns its
inputs two ways: by parameter name via the caller's $PSBoundParameters
(Filter/Sort/Limit/TotalOnly) and by explicit helper argument (-Names/-Ids).
Get-PfbCommonQueryParamHelperWireName resolves against call sites, requires a literal
-Into <variable> so Get-PfbEndpointForVariable can still trace the endpoint, and requires
-BoundParameters to be literally $PSBoundParameters before trusting the by-name rule — the
by-name mapping is only true because that dictionary is the caller's own bound parameters.

Idiom 3 — hashtable-literal initializer. $queryParams = @{ 'names' = $Name }, common in
New-Pfb*. Handled at top level only.

Idiom 4 — nested single-key reference object. $body['account'] = @{ name = $Account },
resolving to the outer key (account). The capability map records top-level body
properties only, so account.name does not exist as a field; two parameters both resolving to
account is correct, not a collision. 13 pairs across 9 cmdlets. The resolver deliberately
refuses multi-key sub-objects, two levels of nesting, non-literal outer keys, and intermediate
variables — each has a test (see Known remaining gap below).

Resolution order is index-assignment → hashtable-literal → helper. Helper last is load-bearing:
per issue #32's design, cmdlets whose Name/Id-equivalents map to non-generic keys
(policy_names, file_system_names, member_names, …) deliberately kept explicit lines
after the helper call, and those must win.

Null-vs-empty conflation, tools/lib/PfbApiDriftTools.ps1. @($inventoryByCmdlet[$name])
yields @($null) — a one-element array whose element is $null — when the cmdlet declares only
inventory-exempt parameters (-Array/-Attributes), so the hashtable key is absent entirely.
$null.Surface -ne 'Typed' was then true and the endpoint was discarded. Correct semantics: a
cmdlet with no inventory rows is vacuously fully mapped, not unmappable.

A sync test parses Add-PfbCommonQueryParams's own AST and asserts the mirrored map still
matches it, so the two cannot drift silently.

Verification

Report regenerated from each tools/lib against each cmdlet tree, all output to scratch paths.

config	tree	parameterGaps	notVerified	missing fields	uncovered
baseline	main	254	194	932	117
baseline	PR #51	132	349	571	117
idioms 2+3	main	343	101	1370	117
idioms 2+3	PR #51	347	94	1380	117
all four + null fix	main	376	59	1538	117
all four + null fix	PR #51	373	59	1517	117

The acceptance criterion was that a pure mechanical refactor must be report-neutral. The
main-vs-PR#51 notVerified spread closes from 155 → 0, with identical endpoint sets on
both sides. The residual 21-field difference is a strict subset (21 fields main-only, 0
PR#51-only: filter/limit/sort on cmdlets #51 migrated) — a genuine code difference from
#51's own change, not tool blindness.

Field count rising 932 → 1538 is the intended effect: endpoints returning to analysis, not
new gaps appearing. Reviewers should expect this number to go up.

Contribution of each fix, measured in isolation (36 + 6 = 42, no overlap):

• Null-vs-empty: 36 endpoints leave notVerified, including PATCH /arrays,
  PATCH /password-policies, PATCH /support with 18 / 19 / 12 missing fields each.
• Idiom four: 6 endpoints — POST /buckets, POST /file-system-exports,
  POST /lifecycle-rules, POST /object-store-access-keys, POST /quotas/groups,
  POST /subnets.

Regression checks by endpoint-set diff, not just counts:

MAIN: baseline    -> final   194 -> 59; NEWLY notVerified: 0
PR51: baseline    -> final   349 -> 59; NEWLY notVerified: 0
FINAL: main       vs PR51     59 -> 59; identical endpoint sets

Full 1763-row inventory diffed key-by-key: 0 keys added, 0 lost, 0
Typed→non-Typed regressions, 0 wire-name or endpoint/method changes on already-Typed
rows, 0 fields removed from the gap list (strictly additive).

• Pester: 449 total, 447 passed, 0 failed, 2 skipped. Baseline re-measured in the same
  worktree rather than taken on trust: 436 / 434 / 0 / 2 — so the delta is exactly the 13 new
  tests, all passing. (An earlier-quoted 6-skipped baseline was measured without the gitignored
  tools/specs cache, where 4 tests self-skip via Set-ItResult.)
• Sync test mutation-verified — adding a fake $Into['destroyed'] key to a scratch copy of
  the helper makes it fail with the expected diff. Not a vacuous assertion.
• Determinism — two runs to different paths produced byte-identical artifacts (matching
  SHA256 for both .json and .md).
• No other helper-induced blindness exists: Add-PfbCommonQueryParams is the only shared
  helper in Private/ that mutates a caller's body/queryParams hashtable.
  Assert-PfbApiCapability and Invoke-PfbApiRequest take those hashtables but only read them.

Live-array testing: not applicable, deliberately

This repo's standing rule is that changes are live-verified against a FlashBlade before a PR.
This change has no runtime surface — it touches only build-time reporting tooling, and no
Public//Private//Data/ file. The equivalent verification is running the real generators
against the real cached specs and two real cmdlet trees, which is the table above. Calling the
exemption out explicitly rather than skipping it silently.

One behaviour deliberately reversed

Tests/PfbCmdletParamTools.Tests.ps1 previously asserted "does NOT descend into a nested
sub-object literal", and Get-PfbHashtableLiteralWireNameForParameter's .DESCRIPTION
documented that as an intentional decision. Both are inverted here.

Worth flagging rather than burying: the original rationale was that crediting the inner key
(name) would mis-name the field and make several parameters collide. Resolving to the
outer key answers that objection rather than overriding it, so the doc comment was rewritten
to say so explicitly instead of being deleted.

Known remaining gap

The idiom-four resolver does not follow intermediate variables. New-PfbFileSystem's
$nfsBody/$smbBody are not traceable to an Invoke-PfbApiRequest call, so
Update-PfbFileSystem's -NfsExportPolicy / -SmbSharePolicy / -SmbClientPolicy remain
unresolved. That is the next parser gap — named here rather than silently absorbed, and each
refusal case has a test pinning the current behaviour.

🤖 Generated with Claude Code