refactor: decompose 5 high-complexity functions (CC reduction)

[yvonnedevlinrh]

Summary

Decomposes 5 high-complexity functions to reduce cyclomatic complexity (CC) and CRAPload, continuing the work from #166. Closes #200.

Function	Before	After	Target
isPointerArgStore	CC 13	CC 3	≤5
detectASTReceiverMutations	CC 24	CC 11	≤15
runCrap	CC 19	CC 14	≤15
runQuality	CC 32	CC 14	≤15
writeOneResult	CC 32	CC 4	≤15

Approach:

• isPointerArgStore: Removed structurally unreachable branches — tracesToParam already walks FieldAddr/IndexAddr/UnOp chains internally
• detectASTReceiverMutations: Extracted 3 per-node-type handlers (handleReceiverAssignStmt, handleReceiverIncDecStmt, handleReceiverCallExpr)
• runCrap: Extracted resolveBaselineAndCompare, writeCrapOutputAndSummary, evaluateCrapGates (D7 gate ordering preserved)
• runQuality: Extracted loadQualityConfig, setupQualityDeps, runQualityPerPackage, handleQualityEmptyResults, writeQualityReport, writeQualityEmptyResults
• writeOneResult: Extracted writeEffectRows, buildEffectsTable, writeTierSummary, writeVerboseSignals

~35 new unit tests added. No behavioral changes — all existing tests pass without modification.

How to Test

# Run all tests
go test -race -count=1 -short ./...

# Verify CC targets (install gocyclo: go install github.com/fzipp/gocyclo/cmd/gocyclo@latest)
gocyclo -over 5 internal/analysis/mutation.go | grep isPointerArgStore    # should return nothing
gocyclo -over 15 internal/analysis/mutation.go | grep detectASTReceiverMutations  # nothing
gocyclo -over 15 cmd/gaze/main.go | grep 'runCrap '    # nothing
gocyclo -over 15 cmd/gaze/main.go | grep 'runQuality '  # nothing
gocyclo -over 15 internal/report/text.go | grep writeOneResult  # nothing

# Lint
golangci-lint run

How to Demo

Pure refactoring — no user-visible behavioural changes. Run gaze crap ./... and gaze quality ./... to confirm identical output. Compare baseline JSON before and after if desired.

Key Files Changed

cmd/gaze/

• main.go — decomposed runCrap (3 helpers) and runQuality (6 helpers)
• main_test.go — 16 new tests for extracted helpers
  internal/analysis/
• mutation.go — simplified isPointerArgStore, extracted 3 handlers from detectASTReceiverMutations
• mutation_test.go — 9 new tests (2 SSA subtests + 7 AST handler tests)
• export_test.go — 3 export wrappers for handler tests
• testdata/src/mutation/mutation.go — 2 new test fixtures
  internal/report/
• text.go — decomposed writeOneResult into 4 helpers
• report_test.go — 7 new tests for report helpers
  openspec/changes/decompose-high-complexity-functions/
• Spec artifacts: proposal, design, delta spec, tasks

This PR was generated by /uf.finale (AI-assisted).

[em-redhat]

PR #236 Review: refactor: decompose 5 high-complexity functions (CC reduction)

Walkthrough

File	Change	Focus
AGENTS.md	Added recent-changes entry for decomposition	documentation
cmd/gaze/main.go	Extracted 8 helpers from runCrap and runQuality	standard
cmd/gaze/main_test.go	~29 new tests for extracted helpers	test-quality
internal/analysis/export_test.go	3 exported test wrappers for handler functions	test-quality
internal/analysis/mutation.go	Simplified isPointerArgStore, extracted 3 handlers from detectASTReceiverMutations	standard
internal/analysis/mutation_test.go	12 new tests for handlers + pointer arg store	test-quality
internal/analysis/testdata/src/mutation/mutation.go	2 new test fixtures	test-quality
internal/report/report_test.go	~13 new tests for writeEffectRows and writeVerboseSignals	test-quality
internal/report/text.go	Extracted 4 helpers from writeOneResult (CC 32→4)	standard
openspec/changes/decompose-high-complexity-functions/*	5 OpenSpec artifacts	documentation

---

Constitution Alignment

Principle	Status	Notes
I. Accuracy	N/A	Pure refactoring, no detection logic changes
II. Minimal Assumptions	N/A	No new assumptions introduced
III. Actionable Output	N/A	Output formatting preserved exactly
IV. Testability	PASS	~52 new tests for extracted helpers; every helper is independently testable

---

Findings

PASS: isPointerArgStore simplification is correct

The removal of UnOp, FieldAddr, and IndexAddr branches from isPointerArgStore is the boldest change in this PR. I verified that tracesToParamVisited already handles all three types recursively in its type switch. The outer function's manual unwrapping was structurally unreachable — tracesToParam(addr, param) at the top of the loop would already walk through any FieldAddr->UnOp->param chain. The two new test cases (SetDirect and NestedFieldAddrChain) confirm the simplified version still catches both direct and nested pointer arg stores.

PASS: detectASTReceiverMutations decomposition preserves semantics

The three extracted handlers (handleReceiverAssignStmt, handleReceiverIncDecStmt, handleReceiverCallExpr) faithfully replicate the inline logic. The return !found change (replacing return true) is correct — when a handler sets found=true, the AST inspection should stop, which is exactly what return false (via !found) achieves.

PASS: writeOneResult decomposition eliminates code duplication

The showClassify and else branches had heavily duplicated row-building and table construction logic. The extraction into writeEffectRows and buildEffectsTable eliminates this duplication. The additional writeTierSummary extraction goes beyond what the design doc specified (D3) but is well-justified by DRY principles (CS-004).

PASS: runCrap and runQuality decompositions

runCrap extracted 3 helpers. runQuality extracted 5 helpers (design D2 specified 2). The additional extractions (loadQualityConfig, setupQualityDeps, handleQualityEmptyResults, writeQualityReport) are well-motivated — each encapsulates a coherent responsibility. The D7 gate ordering (baseline before thresholds) is explicitly preserved in evaluateCrapGates.

PASS: Test quality

~52 new tests across 3 packages. All use standard library assertions (TC-001, TC-002). Test naming follows TestXxx_Description pattern (TC-003). Tests use DI patterns consistent with existing codebase. Tests verify specific expected values, not just err == nil (TC-009).

LOW: writeQualityEmptyResults naming

The function name could be confused with "writing an empty results file." A name like writeQualityNoMappings might be clearer. Non-blocking cosmetic observation.

---

Issue Alignment

Issue #200 acceptance criteria — all 6 items satisfied:

• ✅ Each function's CC reduced to ≤15
• ✅ Extracted helpers have unit tests (~52 new tests)
• ✅ All existing tests pass without modification
• ✅ go test -race -count=1 -short ./... passes
• ✅ golangci-lint run reports zero issues
• ✅ CRAPload does not increase

---

Verdict

APPROVE — Clean, well-tested refactoring that achieves all stated CC reduction targets. The isPointerArgStore simplification is correct (verified against tracesToParamVisited chain walking). No behavioral changes, no API surface changes, no security concerns. All 5 CI checks pass.