Skip to content

Commit 3f3c66d

Browse files
David-Noble-at-workDavid Noble
David-Noble-at-work
and
David Noble
authored
refactor(complexity): phase 7 — the engine core; the board reads zero (#319)
Final phase of the complexity plan (#312): sixteen functions under the thresholds with exact order preservation. ActionPlanner.Plan (69) becomes bindParameterSlots -> bindPresentValue -> bindResourceValue; NewMethod (59) its four labeled validators; newReceiverType (32) splits announced/derived method building. The executor extracts waitRetryDelay and the three-outcome gateRetry from dispatchWithPolicy (32), and prepareResume plus failAndUnwind — the step-21 terminal selection stated once — from Run (26). Guard validation splits into decision-node and tree-shape passes; checkPromiseTypes into a per-slot check; rearm into per-entry re-arming; parseParameterToken into sink/named parsers; assembleGraph extracts its unit table build. flow gains a shared resolveCombinatorSubgraph (Gather's nil-Graph compensator nuance preserved by an inline pre-check), classifyGatherRuns with gatherRun promoted from an anonymous type, bindKwargFrame, a shared desugarLambdaBody, and layoutDecisionTree. A plan-table omission surfaced and folded in: lint's Go (31) was in the original 61 but assigned to no phase. The repository's full lint output — every linter, uncapped — is now empty. make vet and the unmodified full make test pass; gofmt clean. Plan: docs/plans/complexity-refactors.md (complete). Co-authored-by: David Noble <[email protected]>
1 parent 2dc69c5 commit 3f3c66d

13 files changed

Lines changed: 1346 additions & 716 deletions

File tree

‎cmd/star/provider/lint/provider.go‎

Lines changed: 118 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -51,15 +51,7 @@ func (p *Provider) Go(paths []string, config string, skipModTidy bool) (GoResult
5151
paths = []string{"./..."}
5252
}
5353

54-
modTidyPassed := true
55-
modTidyDetails := ""
56-
if !skipModTidy {
57-
sessionCtx := p.RuntimeEnvironment().Context
58-
if sessionCtx == nil {
59-
sessionCtx = context.Background()
60-
}
61-
modTidyPassed, modTidyDetails = checkModTidy(sessionCtx)
62-
}
54+
modTidyPassed, modTidyDetails := p.modTidyStatus(skipModTidy)
6355

6456
if checkTool("golangci-lint") == "" {
6557
return GoResult{}, fmt.Errorf("golangci-lint not installed\n Install: go install github.com/golangci/golangci-lint/cmd/golangci-lint@latest")
@@ -74,65 +66,17 @@ func (p *Provider) Go(paths []string, config string, skipModTidy bool) (GoResult
7466
}
7567
}
7668

77-
cmdArgs := []string{"run", "--output.json.path", "stdout"}
78-
if config != "" {
79-
if !filepath.IsAbs(config) {
80-
if absConfig, err := filepath.Abs(config); err == nil {
81-
config = absConfig
82-
}
83-
}
84-
cmdArgs = append(cmdArgs, "--config="+config)
85-
}
86-
cmdArgs = append(cmdArgs, paths...)
87-
88-
cmd := exec.CommandContext(context.Background(), "golangci-lint", cmdArgs...)
89-
output, err := cmd.Output()
90-
91-
var issues []goIssueRaw
92-
if len(output) > 0 {
93-
var lintOutput goOutputRaw
94-
if jsonErr := json.Unmarshal(output, &lintOutput); jsonErr == nil {
95-
issues = lintOutput.Issues
96-
}
97-
}
98-
99-
if err != nil && len(output) == 0 {
100-
var exitErr *exec.ExitError
101-
if errors.As(err, &exitErr) {
102-
stderr := string(exitErr.Stderr)
103-
if stderr != "" {
104-
return GoResult{}, fmt.Errorf("golangci-lint failed: %s", strings.TrimSpace(stderr))
105-
}
106-
}
69+
issues, err := runGolangciLint(golangciArgs(config, paths))
70+
if err != nil {
71+
return GoResult{}, err
10772
}
10873

10974
result := GoResult{
11075
ConfigCreated: configCreated,
11176
ModTidyPassed: modTidyPassed,
11277
ModTidyDetails: modTidyDetails,
11378
}
114-
115-
for _, issue := range issues {
116-
severity := issue.Severity
117-
if severity == "" {
118-
severity = "warning"
119-
}
120-
switch severity {
121-
case "error":
122-
result.ErrorCount++
123-
default:
124-
result.WarningCount++
125-
}
126-
result.Issues = append(result.Issues, GoIssue{
127-
File: issue.Pos.Filename,
128-
Line: issue.Pos.Line,
129-
Column: issue.Pos.Column,
130-
Message: issue.Text,
131-
Linter: issue.FromLinter,
132-
Severity: severity,
133-
SourceLines: issue.SourceLines,
134-
})
135-
}
79+
appendGoIssues(&result, issues)
13680

13781
result.TotalCount = len(result.Issues)
13882
result.LintPassed = result.ErrorCount == 0 && result.WarningCount == 0
@@ -316,6 +260,119 @@ type goPosRaw struct {
316260
Column int `json:"Column"`
317261
}
318262

263+
// modTidyStatus runs the tidy check under the session context (Background for bare test
264+
// environments) unless skipped.
265+
//
266+
// Parameters:
267+
// - `skipModTidy`: when true, the check is skipped and reports passing.
268+
//
269+
// Returns:
270+
// - `bool`: whether go.mod/go.sum are tidy.
271+
// - `string`: the failure detail, or empty.
272+
func (p *Provider) modTidyStatus(skipModTidy bool) (bool, string) {
273+
274+
if skipModTidy {
275+
return true, ""
276+
}
277+
278+
sessionCtx := p.RuntimeEnvironment().Context
279+
if sessionCtx == nil {
280+
sessionCtx = context.Background()
281+
}
282+
283+
return checkModTidy(sessionCtx)
284+
}
285+
286+
// golangciArgs builds the golangci-lint argv: JSON output, the (absolutized) config when one is set,
287+
// and the package patterns.
288+
//
289+
// Parameters:
290+
// - `config`: the config path; empty adds no --config flag.
291+
// - `paths`: the package patterns.
292+
//
293+
// Returns:
294+
// - `[]string`: the argv after the binary name.
295+
func golangciArgs(config string, paths []string) []string {
296+
297+
cmdArgs := []string{"run", "--output.json.path", "stdout"}
298+
if config != "" {
299+
if !filepath.IsAbs(config) {
300+
if absConfig, err := filepath.Abs(config); err == nil {
301+
config = absConfig
302+
}
303+
}
304+
cmdArgs = append(cmdArgs, "--config="+config)
305+
}
306+
307+
return append(cmdArgs, paths...)
308+
}
309+
310+
// runGolangciLint executes golangci-lint and parses its JSON issue stream; an empty-output failure
311+
// surfaces the tool's stderr.
312+
//
313+
// Parameters:
314+
// - `cmdArgs`: the argv after the binary name.
315+
//
316+
// Returns:
317+
// - `[]goIssueRaw`: the parsed issues; empty when the run was clean.
318+
// - `error`: non-nil when the tool failed without producing output.
319+
func runGolangciLint(cmdArgs []string) ([]goIssueRaw, error) {
320+
321+
cmd := exec.CommandContext(context.Background(), "golangci-lint", cmdArgs...)
322+
output, err := cmd.Output()
323+
324+
var issues []goIssueRaw
325+
if len(output) > 0 {
326+
var lintOutput goOutputRaw
327+
if jsonErr := json.Unmarshal(output, &lintOutput); jsonErr == nil {
328+
issues = lintOutput.Issues
329+
}
330+
}
331+
332+
if err != nil && len(output) == 0 {
333+
var exitErr *exec.ExitError
334+
if errors.As(err, &exitErr) {
335+
stderr := string(exitErr.Stderr)
336+
if stderr != "" {
337+
return nil, fmt.Errorf("golangci-lint failed: %s", strings.TrimSpace(stderr))
338+
}
339+
}
340+
}
341+
342+
return issues, nil
343+
}
344+
345+
// appendGoIssues folds the raw issues into the result: severities default to warning, counts
346+
// accumulate per severity.
347+
//
348+
// Parameters:
349+
// - `result`: the result under construction.
350+
// - `issues`: the parsed raw issues.
351+
func appendGoIssues(result *GoResult, issues []goIssueRaw) {
352+
353+
for _, issue := range issues {
354+
severity := issue.Severity
355+
if severity == "" {
356+
severity = "warning"
357+
}
358+
switch severity {
359+
case "error":
360+
result.ErrorCount++
361+
default:
362+
result.WarningCount++
363+
}
364+
result.Issues = append(result.Issues, GoIssue{
365+
File: issue.Pos.Filename,
366+
Line: issue.Pos.Line,
367+
Column: issue.Pos.Column,
368+
Message: issue.Text,
369+
Linter: issue.FromLinter,
370+
Severity: severity,
371+
SourceLines: issue.SourceLines,
372+
})
373+
}
374+
}
375+
319376
func checkModTidy(ctx context.Context) (tidy bool, detail string) {
320377
tidyCmd := exec.CommandContext(ctx, "go", "mod", "tidy")
321378
if output, err := tidyCmd.CombinedOutput(); err != nil {

‎docs/plans/complexity-refactors.md‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
---
22
title: "Complexity refactors: 61 functions under the thresholds"
33
issue: "#312"
4-
status: in-progress
4+
status: complete
55
created: 2026-08-02
66
updated: 2026-08-02
77
---
@@ -37,7 +37,7 @@ complexity limits. This is the repository's only remaining lint debt.
3737
| 4 | `refactor/complexity-goast-analysis` | goast analysis/serialization: `LoadSourceFile` 67, `analyzeFileMetrics` 55, `assignSlots` 44, `schemaFromConfigVal` 43, `typeToString` 37, `checkLineWidth` 32, `SaveAs` 29, `itemProduction.Execute` 23 | 8 | **complete** |
3838
| 5 | `refactor/complexity-providers` | Concrete providers + satellites: archive `extractEntries` 48, plan `splitReservedKwargs` 39 (**also resolves the parked seven-results carrier-struct question**), file `compensateWrite` 25 + `Link` 23 + `Find` 22, lore `buildPackage` 26, devconfig `reflectToStarlark` 23, platform `compositeManager.dispatch` 21 | 8 | **complete** |
3939
| 6 | `refactor/complexity-conversion` | The conversion surfaces: starlarkbridge `dispatch` 65, `toGoInto` 38, `toStarlarkReflect` 31, `toNaturalGo` 16; op `envValue.ConvertTo` 26, `Convert` 18, `IsTruthy` 19 | 7 | **complete** |
40-
| 7 | `refactor/complexity-engine` | The op engine core, last and most carefully: `ActionPlanner.Plan` 69, `NewMethod` 59, `newReceiverType` 32, executor `dispatchWithPolicy` 32 + `Run` 26, subgraph `validateGuardedEdges` 32, `checkPromiseTypes` 31, `rearm` 23, `parseParameterToken` 21, `assembleGraph` 17; flow `GatherPlanner.Plan` 34, `Gather` 26, `WaitUntil` 25, `ChoosePlanner.Plan` 16, `WaitUntilPlanner.Plan` 16 | 15 | pending |
40+
| 7 | `refactor/complexity-engine` | The op engine core, last and most carefully: `ActionPlanner.Plan` 69, `NewMethod` 59, `newReceiverType` 32, executor `dispatchWithPolicy` 32 + `Run` 26, subgraph `validateGuardedEdges` 32, `checkPromiseTypes` 31, `rearm` 23, `parseParameterToken` 21, `assembleGraph` 17; flow `GatherPlanner.Plan` 34, `Gather` 26, `WaitUntil` 25, `ChoosePlanner.Plan` 16, `WaitUntilPlanner.Plan` 16; **plus lint `Go` 31, a plan-table omission caught and folded in here** | 16 | **complete** |
4141

4242
Total: 60 listed + `TestContext.Check` counted in phase 1 = 61.
4343

@@ -123,6 +123,27 @@ recovered by restoring the develop copy via the API and re-applying with
123123
bounds-verified slices — the lesson (assert slice contents and lengths before
124124
replacing) applied to every later edit.
125125

126+
**Phase 7 (complete):** the engine decomposed with exact order preservation throughout.
127+
`ActionPlanner.Plan` (69) became `bindParameterSlots` → `bindPresentValue` →
128+
`bindResourceValue` (the addressing rules' commentary carried into the helpers); `NewMethod`
129+
(59) its four labeled validators (`validateParameterPositions`, `classifyMethodKind`,
130+
`validatePlanCompanion`, `buildUndoInvoke`); `newReceiverType` (32) split announced/derived
131+
method building. The executor's `dispatchWithPolicy` (32) extracted `waitRetryDelay` and
132+
the three-outcome `gateRetry`; `Run` (26) extracted `prepareResume` and `failAndUnwind`
133+
(the step-21 terminal selection stated once). The subgraph's guard validation split into
134+
decision-node and tree-shape passes; `checkPromiseTypes` into a per-slot check; `rearm`
135+
into per-entry re-arming; `parseParameterToken` into sink/named token parsers. flow gained
136+
a shared `resolveCombinatorSubgraph` (Gather's nil-Graph compensator nuance preserved by an
137+
inline pre-check), `classifyGatherRuns` with `gatherRun` promoted from an anonymous type,
138+
`bindKwargFrame`, a shared `desugarLambdaBody`, and `layoutDecisionTree`. **A plan-table
139+
omission surfaced and was corrected: lint's `Go` (31) was in the original 61 but assigned
140+
to no phase; it folded in here** (`modTidyStatus`/`golangciArgs`/`runGolangciLint`/
141+
`appendGoIssues`).
142+
143+
**Final board: zero.** The repository's full lint output — every linter, uncapped — is
144+
empty. From 2,486 findings at the start of the lint charter to none, with every
145+
suppression in the codebase carrying its stated reason.
146+
126147
## Ordering rationale
127148

128149
Leaf-to-core: the writ/star/goast phases build the decomposition pattern on

‎pkg/op/graph.go‎

Lines changed: 43 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,45 @@ func LoadGraph(env *RuntimeEnvironment, data []byte, format string) (*Graph, err
269269
return assembleGraph(env, &p)
270270
}
271271

272+
// assembleUnits builds the unit symbol table from the flat payload lists. Each unit comes into
273+
// existence with its action already bound — NewNode / NewSubgraph's assert.NonZero invariant holds.
274+
//
275+
// Parameters:
276+
// - `p`: the decoded graph payload.
277+
//
278+
// Returns:
279+
// - `map[string]ExecutableUnit`: the unit table, keyed by ID.
280+
// - `error`: the joined per-unit assembly failures, or nil.
281+
func assembleUnits(p *graphData) (map[string]ExecutableUnit, error) {
282+
283+
var violations []error
284+
unitsByID := make(map[string]ExecutableUnit, len(p.Nodes)+len(p.Subgraphs))
285+
286+
for i := range p.Nodes {
287+
node, err := assembleNode(&p.Nodes[i])
288+
if err != nil {
289+
violations = append(violations, err)
290+
continue
291+
}
292+
unitsByID[node.ID()] = node
293+
}
294+
295+
for i := range p.Subgraphs {
296+
sg, err := assembleSubgraph(&p.Subgraphs[i])
297+
if err != nil {
298+
violations = append(violations, err)
299+
continue
300+
}
301+
unitsByID[sg.ID()] = sg
302+
}
303+
304+
if len(violations) > 0 {
305+
return nil, errors.Join(violations...)
306+
}
307+
308+
return unitsByID, nil
309+
}
310+
272311
// assembleGraph constructs a [*Graph] from a decoded [graphData] payload — the dual to [Graph.marshalData].
273312
//
274313
// It prepares the root the load way: each unit's action is resolved through env.Registry and the concrete
@@ -299,33 +338,12 @@ func assembleGraph(env *RuntimeEnvironment, p *graphData) (*Graph, error) {
299338
// recomputed checksum matches the document's; re-deriving here would drop hand-authored, non-slot-producer edges.
300339
root.edges = p.Edges
301340

302-
var violations []error
303-
304-
// Build the unit symbol table from the flat payload lists. Each unit comes into existence with its action already
305-
// bound — NewNode / NewSubgraph's assert.NonZero invariant holds.
306-
unitsByID := make(map[string]ExecutableUnit, len(p.Nodes)+len(p.Subgraphs))
307-
308-
for i := range p.Nodes {
309-
node, err := assembleNode(&p.Nodes[i])
310-
if err != nil {
311-
violations = append(violations, err)
312-
continue
313-
}
314-
unitsByID[node.ID()] = node
315-
}
316-
317-
for i := range p.Subgraphs {
318-
sg, err := assembleSubgraph(&p.Subgraphs[i])
319-
if err != nil {
320-
violations = append(violations, err)
321-
continue
322-
}
323-
unitsByID[sg.ID()] = sg
341+
unitsByID, err := assembleUnits(p)
342+
if err != nil {
343+
return nil, err
324344
}
325345

326-
if len(violations) > 0 {
327-
return nil, errors.Join(violations...)
328-
}
346+
var violations []error
329347

330348
// Wire root's children + the per-subgraph child links. Each Subgraph's executableUnitsByID was pre-populated with
331349
// placeholder nil entries by assembleSubgraph from its Children list; linkChildren resolves each placeholder against

0 commit comments

Comments
 (0)