Repository navigation
Reuse checked models for cancellable map runs and builds - #1459
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ddfb4871ec
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dbc909477a
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5396517870
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Reject excluded native type definitions
When a retained but excluded .j unit defines a type such as type editorhandle extends handle and selected war3map.j declares a global or parameter of that type, the selected-unit check resolves it against the full model, but this path validates only the variable's own compilation unit; WurstNativeType.imTranslateType then emits editorhandle without invoking requireCompilationUnit. The defining unit is omitted, so -noPJass produces a broken script and PJass builds fail late instead of reporting the excluded declaration. Fresh evidence beyond the resolved function/global report is this unguarded TypeDef path; reject resolved types from excluded compilation units as well.
AGENTS.md reference: AGENTS.md:L95-L95
ℹ️ 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".
|
Addressed the top-level native-type finding in 566a7e0. Native types now retain their referenced source declaration; IM type and default-value translation validate that declaration and all native supertypes against the compilation view. Failing repros and passing regressions cover excluded native types in globals, parameters, returns and selected subtypes on both targets and commands. The 197-case focused run and additional native/typecasting/type-class/compile-time checks passed. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 566a7e022b
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 86d49d2d1d
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
1 participant
RunMap's cancellation scope forced a private AST copy, so the checked-model fast path could never recognize it. Every unchanged run rebuilt semantic attributes and validated the whole project again.
Run, editor build and CLI build now reuse the managed model. Shared AST preparation finishes at safe cancellation boundaries; private IM/backend work remains cancellable. An import-root compilation view selects validation and translation units without removing editor files. The manager tracks which units were checked, while AST-to-IM rejects references to excluded functions, variables, classes and native types, including native supertypes.
Validation preparation and name-preservation analysis use that view too. Adopting checked facts refreshes synthetic variable-event preservation markers for the compilation scope, captures the decisions as immutable source identities in private IM, and restores the editor's exact prior markers before backend work. Thus excluded registrations cannot keep map globals alive, and a later full-model consumer retains its original metadata. Cancellation during shared AST preparation waits for that phase to finish; it does not require cleaning/reparsing on the next request. Editor builds retain the existing open-buffer semantics.
With optimizations disabled, warmed unchanged RunMap improved from 6.67 to 3.92 seconds on castle-fight and 4.55 to 2.79 seconds on zombie-defense, with identical Lua. On Survival Chaos Reborn (1,700 units, Jass, compile-time object injection and PJass enabled), baseline warm runs took 8.66/7.88 seconds; the latest private-preservation run took 4.80/4.35 seconds. Full typechecking was skipped; the preparation stage includes about 0.13 seconds to refresh preservation metadata. All three runs passed PJass and map injection, and the 100,010-line Jass matches baseline except the compiler-version header. First configured-script checks remain necessary.
Remaining large-map costs are AST-to-IM translation, required Jass lowering, compile-time/object work and MPQ/import handling. The profiling report and standalone harness are in
docs/RUNMAP_PERFORMANCE.mdanddocs/profiling/.Validation includes 100 source-edit and 100 configuration-edit oracle sequences against fresh builds, 288 focused checks for validation scope, and additional preservation-ownership, native/typecasting/type-class/compile-time, generic, cancellation and interpreter checks. Regression repros cover run/build on both targets, edits and repairs, configured-script changes, editor-unit retention, both directions of excluded-unit dependencies, native type origins, excluded variable-event registrations, subsequent full-model translation and IM copying. Review findings are addressed in their replies. Codex review completed without new findings and full CI passed on 18cc511. All six earlier findings are addressed; two final review snapshots were unchanged. Ready to merge after seven review rounds.