Conversation
Loading a package runs the go command, and it was the one slow thing asteval did: everything else in it reads trees and types already in hand. Sharing a package meant that anything wanting a string literal evaluated, or a template parsed, linked go/packages and sat one import away from calling it. internal/load now holds what needs the go command, or its result: the package load, the parse errors it recovered from, finding a package by directory or path, evaluating a templates variable, finding a receiver type, and the diagnostics for a lookup that came up empty. The code moves unchanged; the names lose the prefix the package now says (load.Packages, load.Templates), and asteval.Templates, which only its own test called, is folded into load.HTMLTemplates. This is the first step to a seam: the next ones take the package list out of route resolution and generation, so they can be handed types built in memory. Assisted-by: Claude:claude-opus-5 gofumpt
Every stage below the command line took []*packages.Package and dug through it for what it needed: a *types.Package to find the templates package's functions, a FileSet for positions, and a search through the imports for net/http or encoding. So nothing could run without a module on disk and the go command, and every question about how a route resolves, or what a handler looks like, was asked of the 80 second integration suite. The stages now take plain values: - muxt.Package is a FileSet, the package's types, and a lookup for the other packages it needs. muxt.Templates is a templates variable's set, its Funcs functions, and where each template name was written. muxt.Source bundles them with the receiver type, in the order the variables were named. - load.Source builds a muxt.Source from a load, and load.AnalysisSource what the type-checking commands need. That is where a go/packages result stops. A variable that fails to load carries its error, so a command still reports it at the point it reaches that variable and errors keep the order they had. - generate.TemplateRoutesFiles, analysis.NewRoutes, analysis.Check and the template call listings take those values. muxt.Definitions takes a muxt.Templates, so it no longer needs check.DefinitionFinder. - The fake server, which hands the package list to counterfeiter, moves to internal/fakeserver; goimports formatting moves from astgen into generate, the one package that formats files. muxt no longer links go/packages or typelate/check, and generate no longer links go/packages. generate's own tests drop their package load and run in milliseconds instead of half a second. Assisted-by: Claude:claude-opus-5 gofumpt
A test that wants to know how a route resolves needs go/types values: a receiver whose methods take an *http.Request, a parameter that implements encoding.TextUnmarshaler. Getting those from a real package runs the go command, and type checking the standard library from source instead takes over five seconds, most of it net/http. Resolution only ever asks a handful of questions of the standard library, so internal/typestest declares just that API -- net/http, context, net/url, mime/multipart, io, encoding, encoding/json, time and a few more -- under the real import paths, and type checks source against them in memory. The stubs check in microseconds, once per test binary. TestArgument is its first user: its 75 cases read testdata/example through the stubs rather than packages.Load, and the muxt package's tests go from 0.15s to 0.01s. The example no longer needs a go.mod. Assisted-by: Claude:claude-opus-5 gofumpt
A route path helper takes each path parameter as the type the handler parses it into, and that type was only known once the handler had been generated: argument parsing wrote it onto the definition as it went, and the route path helpers, generated afterwards, read it back. It worked because the two ran in that order, and the path helpers could not be asked about a route without generating its handler first. The type is a fact about the call, so resolution records it now. The rule is the one parsing followed: the first place the call passes a parameter, depth first in argument order, decides -- its parameter type, unless a string is assignable to it, when the value is passed along unparsed. TestPathValueTypes states it. Argument parsing also rewrote the definition's call expression in place, swapping each argument for the local it declared, so generating a handler twice from one definition read the first run's rewrites. It rewrites a copy now, and generation leaves the routes it reads as they were resolved. The new generate tests build their input in memory -- a package checked against the stub standard library, templates parsed from a string -- and call TemplateRoutesFiles and the handler assembly directly. Assisted-by: Claude:claude-opus-5 gofumpt
goimports does two jobs on a generated file: it drops the imports the file does not use, and it adds an import for any selector it cannot resolve. The second job had been doing muxt's work. The routes file took its import list before building TemplateData and its methods, so the fmt, errors, io and strings they register arrived too late, and goimports found and added them. Finding a package means walking the file's directory and the module cache, so every generate paid for a search whose answer muxt already had, and what it found depended on the machine. With no sibling files to declare the templates variable -- as in a unit test -- it searched for a package named templates, and one generation took 80ms. The routes file now fills in its imports after the declarations that register them. Unused imports are dropped with goimports' own rule -- a selector on an identifier that resolves to nothing in the file names a package -- and goimports is run with FormatOnly, keeping its import grouping and formatting. Every generated file in the integration suite is unchanged, and generating a file in memory takes under a millisecond. Assisted-by: Claude:claude-opus-5 gofumpt
generate's own tests covered 4% of it: what a handler looks like for a form struct, an sse route, a status code in the name or a redirect in the template was only stated by the integration suite, which compiles and runs every scratch module and takes 80 seconds. testdata/*.txtar now holds one case per feature: the receiver's Go source, the templates, the configuration as JSON, and the files and log lines generation produces. TestSnapshots type checks the source against the stub standard library and generates in memory, so the 32 cases run in 70ms and cover 92% of the package. go test -run TestSnapshots -update rewrites the want/ files; the diff is the review. A snapshot says what the generator does, not that the result compiles or serves requests; that stays the integration suite's job. What it adds is an assertion on every line generation writes, at a speed mutation testing can afford. Each case's files were checked once against the real loader and CLI in a scratch module, and matched. Assisted-by: Claude:claude-opus-5 gofumpt
muxt check is the command the integration suite runs most, and its reports -- a field the data type lacks, a template nothing renders, a route waiting for muxt generate -- were only stated there, behind a package load per script. analysis had 10% coverage of its own. analysis/testdata/*.txtar now holds cases for check, the route listing, and the template caller and call listings, run the same way as generate's snapshots: the Go source is type checked against the stub standard library, the templates are parsed from the archive, and the test finds the ExecuteTemplate calls and wires the trees and definitions the way internal/load does. Each case's report was compared once with the real CLI in a scratch module; they matched. typestest gains CheckSyntax, which also returns the files and type information a test walks, and stubs for html/template and embed. Assisted-by: Claude:claude-opus-5 gofumpt
The mutation run loaded packages with their test files through its own packages.Load call. It moves into load.PackagesWithTests unchanged, so internal/load is the only caller of go/packages, and the claim the contributor guide now makes is true. The guide's architecture section named packages that no longer exist (internal/muxt/parse, internal/source). It now draws the run as it is -- the load, the in-memory source, resolved routes, and what is generated or reported -- and points at the snapshot tests and internal/typestest before an integration script, which is for what needs the go command. Assisted-by: Claude:claude-opus-5 gofumpt
crhntr
force-pushed
the
refactor/load-pacakges
branch
from
September 12, 2026 22:33
548e6e1 to
cdef5ba
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Three unresolved findings remain, including two critical defects and one moderate CLI failure.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR isolates Go package loading behind internal/load, enabling route resolution, generation, and analysis to use plain go/types values and in-memory fixtures.
Changes:
- Centralizes
packages.Loadwithmuxt.Packageandmuxt.Sourceadapters. - Adds type-checking stubs and snapshot-based test coverage.
- Refactors generation, analysis, mutation testing, CLI, and fake-server plumbing.
File summaries
| File | Summary |
|---|---|
internal/typestest/typestest.go |
In-memory type-checking helpers. |
internal/typestest/typestest_test.go |
Type-checking helper tests. |
internal/typestest/stubs.go |
Standard-library stubs. |
internal/muxt/unmarshal.go |
Package-based type lookup. |
internal/muxt/testdata/example/go.mod |
Example module fixture. |
internal/muxt/path_value_types_test.go |
Path-value type tests. |
internal/muxt/package.go |
Package abstraction. |
internal/muxt/name_error_test.go |
Name-error tests. |
internal/muxt/definition.go |
Resolved path-value storage. |
internal/muxt/definition_test.go |
Definition tests. |
internal/muxt/definition_internal_test.go |
Internal definition tests. |
internal/muxt/call_test.go |
critical (1 vote; issue in call.go): repeated path names can retain the first type and skip a required conversion. |
internal/muxt/call_internal_test.go |
Internal call tests. |
internal/mutation/traverse.go |
Mutation traversal refactor. |
internal/mutation/plan.go |
Mutation plan loading. |
internal/mutation/diff.go |
Mutation diff updates. |
internal/load/templates_test.go |
Template-loading tests. |
internal/load/source.go |
Loaded-package source adapter. |
internal/load/package.go |
Centralized package loading. |
internal/load/package_test.go |
Package-loading tests. |
internal/load/diagnostic.go |
Loading diagnostics. |
internal/load/diagnostic_test.go |
Diagnostic tests. |
internal/generate/validation_test.go |
Generation validation tests. |
internal/generate/testdata/synthesized_method_note.txtar |
Generation snapshot fixture. |
internal/generate/testdata/route_without_call.txtar |
Generation snapshot fixture. |
internal/generate/testdata/response_argument.txtar |
Generation snapshot fixture. |
internal/generate/testdata/request_body.txtar |
Generation snapshot fixture. |
internal/generate/testdata/redirect.txtar |
Generation snapshot fixture. |
internal/generate/testdata/receiver_method_sets.txtar |
Generation snapshot fixture. |
internal/generate/testdata/nested_calls.txtar |
Generation snapshot fixture. |
internal/generate/testdata/multipart.txtar |
Generation snapshot fixture. |
internal/generate/testdata/marshal_json.txtar |
Generation snapshot fixture. |
internal/generate/testdata/last_event_id.txtar |
Generation snapshot fixture. |
internal/generate/testdata/inferred_methods.txtar |
Generation snapshot fixture. |
internal/generate/testdata/form_values.txtar |
Generation snapshot fixture. |
internal/generate/testdata/form_struct.txtar |
Generation snapshot fixture. |
internal/generate/testdata/flag_unexported_identifiers.txtar |
Generation snapshot fixture. |
internal/generate/testdata/flag_muxt_version.txtar |
Generation snapshot fixture. |
internal/generate/testdata/flag_logger_path_prefix_middleware.txtar |
Generation snapshot fixture. |
internal/generate/testdata/flag_htmx.txtar |
Generation snapshot fixture. |
internal/generate/testdata/flag_custom_names.txtar |
Generation snapshot fixture. |
internal/generate/testdata/execute_callback.txtar |
Generation snapshot fixture. |
internal/generate/testdata/err_signals_without_datastar.txtar |
Generation snapshot fixture. |
internal/generate/testdata/err_route_paths_method_collision.txtar |
Generation snapshot fixture. |
internal/generate/testdata/err_response_state_with_response_argument.txtar |
Generation snapshot fixture. |
internal/generate/testdata/err_resolution.txtar |
Generation snapshot fixture. |
internal/generate/testdata/err_name_errors.txtar |
Generation snapshot fixture. |
internal/generate/testdata/err_duplicate_pattern.txtar |
Generation snapshot fixture. |
internal/generate/sse.go |
SSE generation refactor. |
internal/generate/source_test.go |
Source generation tests. |
internal/generate/snapshot_test.go |
Snapshot test harness. |
internal/generate/routes_test.go |
Route generation tests. |
internal/generate/html.go |
HTML generation updates. |
internal/generate/groups.go |
Route group generation. |
internal/generate/generated_test.go |
Generated-output tests. |
internal/generate/format.go |
critical (1 vote): explicit import aliases can be pruned, leaving generated code uncompilable. |
internal/generate/file.go |
Generated file construction. |
internal/generate/file_test.go |
Generated file tests. |
internal/fakeserver/fakeserver.go |
Fake-server generation extraction. |
internal/cli/commands.go |
moderate (2 votes): output files in subdirectories can fail because the output directory is not loaded. |
internal/astgen/format.go |
AST formatting updates. |
internal/asteval/parse.go |
AST evaluation parsing. |
internal/analysis/testdata/routes.txtar |
Analysis snapshot fixture. |
internal/analysis/testdata/check_wrong_field.txtar |
Analysis snapshot fixture. |
internal/analysis/testdata/check_unused_templates.txtar |
Analysis snapshot fixture. |
internal/analysis/testdata/check_template_not_found.txtar |
Analysis snapshot fixture. |
internal/analysis/testdata/check_passes.txtar |
Analysis snapshot fixture. |
internal/analysis/testdata/check_bad_route_name.txtar |
Analysis snapshot fixture. |
internal/analysis/testdata/calls.txtar |
Analysis snapshot fixture. |
internal/analysis/testdata/callers.txtar |
Analysis snapshot fixture. |
internal/analysis/testdata/callers_match.txtar |
Analysis snapshot fixture. |
internal/analysis/templates.go |
Template analysis refactor. |
internal/analysis/template_calls.go |
Template-call analysis. |
internal/analysis/template_callers.go |
Template-caller analysis. |
internal/analysis/routes.go |
Route analysis. |
internal/analysis/check.go |
Analysis checks. |
CLAUDE.md |
Architecture and workflow documentation. |
Review details
Files not reviewed (1)
- internal/generate/generated_test.go: Generated file
- Files reviewed: 86/87 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+69
to
+70
| if id, ok := sel.X.(*ast.Ident); ok && id.Obj == nil { | ||
| used[id.Name] = true |
Comment on lines
+155
to
+160
| case ArgumentTypeRequestPathValue: | ||
| if seen[arg.Identifier] || IsSSEArgument(arg.Identifier) { | ||
| continue | ||
| } | ||
| seen[arg.Identifier] = true | ||
| if !isStringAssignable(arg.ParamType) { |
| warnPartialAST(log.New(cmd.ErrOrStderr(), "", 0), pl) | ||
| files, err := generate.TemplateRoutesFiles(*workingDirectory, config, fileSet, pl, log.New(stdout, "", 0)) | ||
| // The routes file belongs to the package in its own directory. | ||
| src, err := load.Source(filepath.Dir(filepath.Join(*workingDirectory, config.OutputFileName)), pl, load.SourceConfiguration{ |
The mutation run's planning -- which templates each ExecuteTemplate call reaches, with what dot, and which variations apply -- read the package list directly: the loaded templates, the checker built on them, and the syntax trees it searched for the string literal a template was written in. So stating any of it took a module on disk and the go command. Planning now runs on an input: the package's types and each templates variable as type checking reads it. loadInput builds one with the go command; planFrom and revisionOf read nothing else. A template's string literal is found by parsing its file's text, which planning already holds, instead of searching the loader's syntax trees. The templates variable a type checker reads moves from analysis to internal/templateset, so the mutation package shares it without importing analysis, and load.TemplateSets builds it for both. internal/load/loadtest builds what a package load returns without the go command: it writes the files, type checks them against typestest's stub standard library (which gains io/fs and more of html/template), and assembles the *packages.Package. Tests then go through load's real template evaluation, definitions and ExecuteTemplate calls. internal/mutation/testdata/*.txtar snapshots a dry run's report for ten cases -- a literal template, partials and trims, a template pattern, skipped mutants, the operand budget, other delimiters, a --diff revision, and the errors a plan returns -- planned from a loadtest package in milliseconds. Each report was compared once with the real CLI in a scratch module, --diff against a git repository, and matched. Assisted-by: Claude:claude-opus-5 gofumpt
A large share of the integration suite checks that flags mean what they say, and it could only do that by running the whole command: load a package, generate, compile. Deciding what a command line means happens before any of that, but nothing let a test stop there. Each command now builds its configuration -- flags parsed, defaults applied, what cannot work rejected -- and hands it to a runner, which loads the package and runs the implementation. Commands wires the real runners; configurationOf wires ones that record what they are handed. TestCommandLineConfigurations states what command lines parse into, as composite literals, and TestCommandLineRejections the ones refused, with the error a user sees. The listing commands' configurations carry every templates variable, rather than one set per loop pass, so the literal is the whole run. load.GenerateSource and load.RoutesSource hydrate a configuration into the implementation's input. The generate and analysis snapshots take their configuration from the same kind of literal, inline in snapshots_test.go with the command line it stands for, instead of a config.json layered over test defaults. An implementation test only ever sees a configuration a command line can produce; rejecting the rest is the command line's job. The generate snapshots now run with the defaults the command line applies, so TemplateData gains its MuxtVersion method in each, and the version case becomes the case that leaves it out. Assisted-by: Claude:claude-opus-5 gofumpt
What internal/load handed on came in two shapes for one idea: a muxt.Source for generate and the route listing, and a package with []templateset.Variable for check, the listings and mutation. Neither was data. A package found its imports through a function, a variable reported name positions through another, and trees and definitions through interfaces backed by check.Templates. A variable that failed to load carried its error inside it. ExecuteTemplate calls carried their AST node, so reading a position needed the FileSet. A test could not write any of it as a literal; the analysis snapshots re-implemented a definition finder to fake one. internal/source now holds the one shape: a Package with its FileSet, its types, its imports by path, and its templates variables. A Variable holds its template set, its functions, its definitions by template name, and its calls, each with a position, the template name and the type of dot. No function fields, no interfaces, no errors. muxt, generate, analysis and mutation read it; analysis and mutation build their check.Global from it. muxt.Package, muxt.Source, muxt.Templates and internal/templateset are gone. load.Package reads a load into it and fails at the first variable that does not evaluate, rather than carrying errors for a later stage to report. That only changes which error is reported when two variables fail differently, which no command line in the integration suite exercises. load.GenerateSource and load.RoutesSource add the receiver and still report a missing package, then a missing receiver, before any variable; TestHydration states that order. The generate and analysis snapshots now load through loadtest and load's hydration, as the commands do, rather than building their input by hand. Every generated file and error is unchanged; the analysis reports move by the lines their sources no longer spend declaring the templates variable. loadtest includes encoding, fmt and net/http in a load, as load.Packages does. Assisted-by: Claude:claude-opus-5 gofumpt
A generated file could declare imports it did not use, and formatFile dropped them after the fact. The cause was one import registry shared by every file a run writes: with --output-multiple-files, each per-file routes file and the main one declared every package any of them had registered. The registry lived on the same value that held the loaded packages, so nothing marked where one file's imports ended and the next began. Each file a run writes now builds its declarations against a File of its own, so the imports it declares are exactly the ones its declarations registered. With that, no file in the snapshots or the integration suite has an import to drop -- checked by failing on any before removing the filter -- so formatFile no longer parses the output to find them. astgen.ImportManager loses Types: finding a loaded package is not what an import registry is for, and the one caller asks the output package. The snapshot tests now fail on a generated file that imports a package it does not use, which is what the filter used to hide. Assisted-by: Claude:claude-opus-5 gofumpt
With each file declaring exactly the imports it uses, goimports had one job left: laying the import block out in groups, the standard library before everything else. That kept golang.org/x/tools/imports -- and its walks of the module cache when asked for more -- in generate's build. formatFile now writes the import block itself, grouped the way goimports groups it (a path whose first element has a dot after those that have none), sorted within each group and set apart by a blank line, and formats the file with go/format. TestFormatFileImports states the layout, which was compared once with goimports' output for the same imports. The fake server's main.go, whose imports its template writes out, is formatted the same way. generate no longer links any of golang.org/x/tools. Every generated file in the snapshots and the integration suite is unchanged. Assisted-by: Claude:claude-opus-5 gofumpt
This was referenced Sep 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
internal/loadis now the only caller ofpackages.Load. Route resolution, generation andanalysistake plain go/types values and template sets (muxt.Source), so they can be tested with inputs built in memory.internal/typestesttype checks source against stub standard library packages in microseconds.internal/{generate,analysis}/testdata/*.txtar,-updateto rewrite): generate coverage 4% → 92%, analysis 10% → 56%, each suite well under a second.fmt/errorsand walking the module cache). Output is unchanged.cmd/muxtintegration tests are untouched and pass. Gremlins oninternal/generate: 89% efficacy in 55s (needs--timeout-coefficient 100).🤖 Generated with Claude Code
https://claude.ai/code/session_01UjkpA4fvZ7xQAsY65Beprp