refactor(compiler): split semantic IR source maps and target plans#755
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: af0b3a5623
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if previous, exists := endpointIDs[endpointID]; exists { | ||
| report("endpoint %q duplicates semantic id %q from endpoint %q", endpoint.Path, endpointID, previous.Path) |
There was a problem hiding this comment.
Avoid treating duplicate endpoints as IR invariants
When a user declares the same endpoint block twice with the same page/name/method/path (for example two Save actions inheriting /profile), both builder-created endpoints get the same semantic ID, so this new invariant returns an internal compiler error before validateRouteMethodConflicts/page validators can report a normal author diagnostic. Duplicate route registrations are authoring errors, not compiler invariants, so this uniqueness check should either be moved into validation or limited to cases that cannot be produced from user input.
Useful? React with 👍 / 👎.
| escaped = append(escaped, url.PathEscape(part)) | ||
| } | ||
| return strings.Join(escaped, ":") |
There was a problem hiding this comment.
Because url.PathEscape leaves : unchanged while : is also the tuple separator, different semantic tuples can collapse into the same ID. For example, RouteIdentity("GET", "/foo:bar", "baz") and RouteIdentity("GET", "/foo", "bar:baz") both become GET:%2Ffoo:bar:baz; literal route segments and page IDs with colons are accepted today, so valid pages can get the same RouteID and trip the duplicate-ID invariant as an internal compiler error. Use an encoding that also escapes the separator, or switch to a length-prefixed/structured key.
Useful? React with 👍 / 👎.
Summary
Program.GoEndpointswithProgram.SourceMapentries keyed by stableEndpointID.Program.Endpointsso validation, binding lookup, route checks, and generated-output planners consume semantic records instead of raw declaration records.Issue Closure
Closes #668
Related: #664
Related: #667
Verification
scripts/test-go-modules.shwhen Go code or compiler behavior changed.go build ./cmd/gowdkwhen CLI, compiler, runtime, addon, or release behavior changed.node --check editors/vscode/extension.jswhen editor files changed.Commands run:
go test ./internal/gwdkir ./internal/gwdkanalysisgo test ./internal/compilergo test ./internal/appgengo test ./internal/buildgen ./internal/lspgo test ./internal/diagnostics ./internal/diagnosticfixgo build ./cmd/gowdkgo run ./cmd/gowdk inspect ir examples/pages/home.page.gwdkscripts/check-docs-links.shscripts/check-docs-style.shscripts/test-go-modules.shLLM Assistance
gwdkir.Programinto semantic IR and target-specific plans #668 is fully resolved by this PR becausegwdkir.Programnow separates normalized semantic endpoint records from diagnostic source-map records and generated-output consumers carry semantic IDs into target-specific plans. Make typed IR fields the only semantic source of truth #664 and Introduce phase-typed validated program and application plan boundaries #667 remain related follow-up issues because their broader typed-only and phase-boundary acceptance criteria are not fully closed here.Program.GoEndpointssemantic/raw declaration coupling.