Skip to content

refactor: decompose mapping.go's three highest-complexity functions#195

Merged
yvonnedevlinrh merged 1 commit into
unbound-force:mainfrom
yvonnedevlinrh:opsx/crapload-decompose-pr2b
Jul 17, 2026
Merged

refactor: decompose mapping.go's three highest-complexity functions#195
yvonnedevlinrh merged 1 commit into
unbound-force:mainfrom
yvonnedevlinrh:opsx/crapload-decompose-pr2b

Conversation

@yvonnedevlinrh

Copy link
Copy Markdown
Contributor

Summary

Phase 2b of CRAPload fragility reduction (#166). Decomposes the three highest-complexity functions in internal/quality/mapping.go — the assertion mapping engine — into 9 independently testable helpers.

Changes

Decomposition

Function Before After Helpers extracted
matchContainerUnwrap 50 8 collectTrackedVars, traceForwardDataFlow, matchTrackedInExpr
isTransformationCall 26 5 isByteLikeParam, isPointerDestParam, resolveCallSignature, findParamIndex
matchAssertionToEffect 25 5 matchDirect, matchIndirectRoot
Combined 101 18 9 helpers

Test gap coverage

  • +25 new unit tests using synthetic AST via parseAndTypeCheck
  • Filled 3 isTransformationCall gaps: io.Reader parameter, empty interface destination, mixed parameter ordering
  • matchAssertionToEffect now has direct unit tests (previously zero)
  • matchContainerUnwrap helpers now have unit tests (previously integration only)

Test infrastructure cleanup

  • Refactored parseAndTypeCheck to delegate to parseAndTypeCheckWithFset (DRY)
  • Added comment documenting intentionally ignored type-check errors

Files changed (3 source + 5 spec + 2 learnings)

AGENTS.md                                          (+1)  — Recent Changes entry
internal/quality/mapping.go                        (+408/-168) — 9 helpers extracted
internal/quality/container_unwrap_internal_test.go (+653/-12) — 25 new tests, DRY fix

Verification

go build ./...                              # PASS
go test -race -count=1 -short ./...         # 16/16 packages PASS
golangci-lint run                           # 0 issues

# Complexity targets
gocyclo internal/quality/mapping.go | grep -E 'matchContainerUnwrap|isTransformationCall|matchAssertionToEffect'
# 8 matchContainerUnwrap, 5 matchAssertionToEffect, 5 isTransformationCall

# Mapping accuracy ratchet
go test -race -count=1 -run TestSC003_MappingAccuracy ./internal/quality/...
# 84.7% (floor: 84.0%) — PASS

Note on traceForwardDataFlow complexity

traceForwardDataFlow landed at complexity 32 — higher than the design's ~12 estimate. This is the forward data-flow tracing loop moved intact from matchContainerUnwrap. The parent function dropped from 50 to 8 because the concerns are now isolated, but the extracted core loop retains inherent complexity from nested ast.Inspect closures, multi-iteration convergence, and transformation call detection. Further decomposition of traceForwardDataFlow is a candidate for Phase 2c.

Review council

  • Spec review: 3/5 APPROVE, 2/5 REQUEST CHANGES → fixes applied → all APPROVE
  • Code review: 4/4 APPROVE (Architect 9/10, Testing, Guard, Adversary)

Spec artifacts

Full OpenSpec change at openspec/changes/crapload-decompose-pr2b/:

  • proposal.md — motivation + constitution alignment
  • design.md — 5 design decisions with alternatives rejected
  • specs/decompose-mapping.md — 7 ADDED, 3 MODIFIED requirements with 24 scenarios
  • tasks.md — 11 tasks in 5 groups, all checked off

Extract 9 helpers from the three highest-complexity functions in
internal/quality/mapping.go:

- matchContainerUnwrap (50 → 8): collectTrackedVars,
  traceForwardDataFlow, matchTrackedInExpr
- isTransformationCall (26 → 5): isByteLikeParam, isPointerDestParam,
  resolveCallSignature, findParamIndex
- matchAssertionToEffect (25 → 5): matchDirect, matchIndirectRoot

Combined complexity reduced from 101 to 18.

+25 new unit tests using synthetic AST via parseAndTypeCheck. Filled 3
test gaps in isTransformationCall (io.Reader, empty interface, mixed
parameter ordering). TestSC003_MappingAccuracy ratchet (85.0%) passes
unchanged — no behavioral changes.

Refactored parseAndTypeCheck to delegate to parseAndTypeCheckWithFset
to eliminate DRY violation. Added comment documenting intentionally
ignored type-check errors in test helper.

Phase 2b of issue unbound-force#166.

Assisted-by: OpenCode (claude-opus-4-6)
Signed-off-by: Yvonne Devlin <ydevlin@redhat.com>
@yvonnedevlinrh yvonnedevlinrh self-assigned this Jul 17, 2026
@yvonnedevlinrh yvonnedevlinrh added the refactor Improvements or cleanup of code label Jul 17, 2026
@yvonnedevlinrh yvonnedevlinrh moved this to In Review 👀 in Unbound Force Planning Jul 17, 2026
@yvonnedevlinrh
yvonnedevlinrh requested a review from a team July 17, 2026 09:13
@yvonnedevlinrh
yvonnedevlinrh merged commit 1eb68bd into unbound-force:main Jul 17, 2026
5 checks passed
@yvonnedevlinrh
yvonnedevlinrh deleted the opsx/crapload-decompose-pr2b branch July 17, 2026 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor Improvements or cleanup of code

Projects

Status: In Review 👀

Development

Successfully merging this pull request may close these issues.

2 participants