Skip to content

Commit 1bd7e03

Browse files
authored
Refactor compiler orchestrators to enforce 60-line largefunc limits (part 1) (#36177)
1 parent e0d8d6f commit 1bd7e03

5 files changed

Lines changed: 674 additions & 688 deletions
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
# ADR-36177: Decompose Compiler Orchestrators into Single-Responsibility Helpers
2+
3+
**Date**: 2026-06-01
4+
**Status**: Draft
5+
6+
## Context
7+
8+
The repository enforces a custom `largefunc` linter that caps functions at 60 lines. Three core compiler orchestration entry points were well above that limit: `setupEngineAndImports` (`compiler_orchestrator_engine.go`), `processToolsAndMarkdown` (`compiler_orchestrator_tools.go`), and `validateToolConfiguration` (`compiler_validators.go`). These functions had grown into long, linear flows interleaving engine extraction, import resolution, network/permission merging, strict-mode toggling, and multiple validation passes. A recurring source of length and risk was manual strict-mode save/restore: each validation block saved `c.strictMode`, set the effective mode, and restored it on every early return, duplicating the restore logic at every exit point.
9+
10+
## Decision
11+
12+
We will refactor each oversized orchestrator into a short coordinator that delegates to cohesive, named helper functions split along the natural phase boundaries of the flow — engine extraction/override/import resolution/default application, tools+MCP merge and normalization, markdown artifact extraction, and validation passes grouped by concern. We will additionally introduce a `withEffectiveStrictMode` helper that sets the effective strict mode and restores it via `defer`, replacing the hand-written save/restore-on-every-return pattern. The primary driver is conformance with the existing `largefunc` 60-line limit while preserving execution order and existing warning/error semantics.
13+
14+
As a safety improvement folded into the extraction, `applyEngineImportDefaults` now returns the effective `*EngineConfig` and callers use that return value, because the helper may allocate a new config when its input is nil — relying on the mutated pointer alone would drop that allocation.
15+
16+
## Alternatives Considered
17+
18+
### Alternative 1: Exempt the orchestrators from the `largefunc` limit
19+
We could add `nolint` directives or raise the threshold for these orchestration functions, since coordinating many phases is inherently long. Rejected because it weakens a repo-wide quality gate, and the flow decomposes cleanly into phase helpers that are independently readable and testable — the length was incidental, not essential.
20+
21+
### Alternative 2: Shorten in place without extracting named helpers
22+
We could trim each function below 60 lines by collapsing comments and inlining branches without creating new package-level helpers. Rejected because the bulk of the length came from genuinely distinct phases and the repeated strict-mode save/restore boilerplate; only extraction (helpers plus the `defer`-based `withEffectiveStrictMode`) removes the duplication rather than merely compressing it.
23+
24+
## Consequences
25+
26+
### Positive
27+
- Each orchestrator reads as a short sequence of named phases, making the compile flow easier to follow and the individual phases independently testable.
28+
- The `withEffectiveStrictMode` helper centralizes strict-mode restoration in a single `defer`, eliminating the duplicated restore-on-every-return logic and the risk of a missed restore leaking strict mode across workflows.
29+
- Returning the effective `*EngineConfig` from `applyEngineImportDefaults` removes a latent bug where a nil-input allocation would be discarded by callers.
30+
31+
### Negative
32+
- More package-level functions increase the symbol surface and add a layer of indirection when reading a single orchestrator end to end.
33+
- The phase-helper decomposition pattern and the `withEffectiveStrictMode` convention must be maintained by future contributors to keep the orchestrators uniform and under the limit.
34+
35+
### Neutral
36+
- No change to compilation behavior, validation order, or warning/error output; this is a structural refactor within the existing compiler pipeline.
37+
- Some validation blocks were converted to table-driven dispatch, a stylistic shift that is behavior-equivalent to the prior sequential calls.
38+
39+
---
40+
41+
*This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/26736316999) workflow. The PR author must review, complete, and finalize this document before the PR can merge.*

0 commit comments

Comments
 (0)