Model compilation contract

MODEL_COMPILATION_ON_BUILDTIME_CONTRACT.md

@@ -2,16 +2,13 @@
 
 ## Scope
 
-Expand the toolkit so authored ASCII Neverwinter Nights `.mdl` assets can be
+Expand the toolkit so authored ASCII Neverwinter Nights `.mdl` assets can be compiled into binary `.mdl` build artifacts during `sow-toolkit build-haks` without changing the authored source tree.
-compiled into binary `.mdl` build artifacts during `sow-toolkit build-haks`
-without changing the authored source tree.
 
 This contract covers:
 
 - `toolkit/` as the implementation owner
 - `sow-assets/` as the primary consumer repository
-- any future consumer repo that packages NWN model assets through the same HAK
+- any future consumer repo that packages NWN model assets through the same HAK build pipeline
-  build pipeline
 
 This contract does not cover:
 
@@ -22,31 +19,24 @@ This contract does not cover:
 
 ## Goal
 
-Make model compilation a first-class, deterministic, configurable build stage in
+Make model compilation a first-class, deterministic, configurable build stage in the Go toolkit so generated HAKs contain compiled binary models while authored repositories continue to keep ASCII `.mdl` files as source of truth.
-the Go toolkit so generated HAKs contain compiled binary models while authored
-repositories continue to keep ASCII `.mdl` files as source of truth.
 
 ## Why This Exists
 
-Repo-local context already shows that ASCII `.mdl` files are an intentional part
+Repo-local context already shows that ASCII `.mdl` files are an intentional part of authored asset workflows:
-of authored asset workflows:
 
 - validation reports ASCII/decompiled models as informational, not erroneous
 - topdata autogen manifest generation scans authored `.mdl` paths directly
 - part discovery derives IDs from authored `.mdl` filenames
-- HAK building already has other generated-asset stages such as NWScript and
+- HAK building already has other generated-asset stages such as NWScript and music conversion
-  music conversion
 
 Upstream NWN documentation also supports compiling before release:
 
-- `nwn.wiki` documents that ASCII models are slower because the game compiles
+- `nwn.wiki` documents that ASCII models are slower because the game compiles them at load time and recommends shipping compiled models
-  them at load time and recommends shipping compiled models
 - `nwnmdlcomp` usage docs confirm ASCII-to-binary compilation as a standard flow
-- community documentation shows `nwnmdlcomp.exe` expects old NWN registry/data
+- community documentation shows `nwnmdlcomp.exe` expects old NWN registry/data layout and may need a 1.69-era data root
-  layout and may need a 1.69-era data root
 
-Therefore the correct implementation is not “replace asset scanning with binary
+Therefore the correct implementation is not “replace asset scanning with binary models everywhere”. The correct implementation is:
-models everywhere”. The correct implementation is:
 
 1. keep authored ASCII `.mdl` in `paths.assets`
 2. preserve current validation and manifest derivation behavior against source
@@ -55,14 +45,12 @@ models everywhere”. The correct implementation is:
 ## Ownership Split
 
 - `toolkit/`
-  - owns config schema, validation, discovery, cache policy, build staging, and
+  - owns config schema, validation, discovery, cache policy, build staging, and HAK integration
-    HAK integration
   - owns Wine execution and NWN data preparation logic
   - owns tests and user-facing CLI behavior
 - `sow-assets/`
   - owns opting into the feature through `nwn-tool.yaml`
-  - owns providing local tool binaries and any fake/minimal NWN data tree it
+  - owns providing local tool binaries and any fake/minimal NWN data tree it wants to use
-    wants to use
   - owns wrapper scripts and CI invocation details
 - `sow-module/`
   - is an indirect consumer only when it consumes generated HAKs
@@ -80,21 +68,15 @@ Current HAK packaging is Go-native and centered on:
 - `internal/project/*`
 - `internal/validator/*`
 
-`build-haks` already does all asset discovery, planning, manifest writing, HAK
+`build-haks` already does all asset discovery, planning, manifest writing, HAK reuse, and generated-asset preparation inside the toolkit. Adding model compilation as a disconnected shell script would duplicate the existing build graph and create drift.
-reuse, and generated-asset preparation inside the toolkit. Adding model
-compilation as a disconnected shell script would duplicate the existing build
-graph and create drift.
 
 ## Existing relevant behavior
 
 - `project.AssetExtensions` already includes `.mdl`
-- `validator.ValidateProject` detects text `.mdl` and reports them via
+- `validator.ValidateProject` detects text `.mdl` and reports them via `Report.DecompiledModels`
-  `Report.DecompiledModels`
 - `emitValidatorReport` already surfaces that information in CLI output
-- `collectAssetResources` packages raw files from `paths.assets` directly into
+- `collectAssetResources` packages raw files from `paths.assets` directly into HAK resources
-  HAK resources
+- `BuildHAKs` already supports generated assets and temporary staging through the music pipeline
-- `BuildHAKs` already supports generated assets and temporary staging through the
-  music pipeline
 - script compilation already establishes a precedent for:
   - config-backed tool resolution
   - environment discovery
@@ -107,24 +89,20 @@ graph and create drift.
 - autogen and topdata logic must continue to see source `.mdl` paths
 - validation must still be able to detect ASCII source models
 - HAK duplicate detection and manifest generation must remain deterministic
-- build output reuse must not falsely reuse HAKs built from stale compiled model
+- build output reuse must not falsely reuse HAKs built from stale compiled model content
-  content
 - consumer repos must be able to opt out cleanly
 
 ## Architecture Decision
 
-Implement model compilation as a generated-asset overlay inside the existing HAK
+Implement model compilation as a generated-asset overlay inside the existing HAK build pipeline, not as a separate top-level shell build.
-build pipeline, not as a separate top-level shell build.
 
 That means:
 
 - source scanning still indexes authored `.mdl`
 - a model preparation stage produces compiled build artifacts in cache/staging
-- `collectAssetResources` substitutes compiled `.mdl` bytes for eligible source
+- `collectAssetResources` substitutes compiled `.mdl` bytes for eligible source models during HAK packaging
-  models during HAK packaging
 - non-model assets continue to flow unchanged
-- the written HAK manifest still records the authored relative asset path, not a
+- the written HAK manifest still records the authored relative asset path, not a cache path
-  cache path
 
 This mirrors the existing music pipeline more than the script pipeline:
 
@@ -138,8 +116,7 @@ This mirrors the existing music pipeline more than the script pipeline:
 - no mutation of files under `paths.assets`
 - no requirement to compile every `.mdl` in every repo by default
 - no requirement to support Windows-only shell wrappers as the primary interface
-- no requirement to support arbitrary model compilers on day one beyond
+- no requirement to support arbitrary model compilers on day one beyond `nwnmdlcomp.exe` via Wine
-  `nwnmdlcomp.exe` via Wine
 - no attempt to “fix” malformed source models automatically
 
 ## Config Contract
@@ -223,8 +200,7 @@ Do not silently guess a fake NWN data tree.
 
 ## Eligibility rules
 
-A model is eligible for build-time compilation when all of the following are
+A model is eligible for build-time compilation when all of the following are true:
-true:
 
 - asset extension is `.mdl`
 - `models.compile.enabled` is true
@@ -239,8 +215,7 @@ If a `.mdl` file is already binary and `preserve_binary_sources` is true:
 
 ## Integration point
 
-Insert model preparation into `planOrBuildHAKs` before `collectAssetResources`
+Insert model preparation into `planOrBuildHAKs` before `collectAssetResources` finalizes HAK resource content, alongside other generated-asset preparation.
-finalizes HAK resource content, alongside other generated-asset preparation.
 
 Expected flow:
 
@@ -252,8 +227,7 @@ Expected flow:
 
 ## Prepared asset representation
 
-Introduce a prepared-model result similar in spirit to `preparedMusicAssets`,
+Introduce a prepared-model result similar in spirit to `preparedMusicAssets`, for example:
-for example:
 
 - generated assets keyed by original asset-relative path
 - skipped source-relative paths
@@ -270,8 +244,7 @@ Only the file bytes should differ.
 
 ## Cache and staging contract
 
-Compiled models are generated artifacts and must live outside the authored asset
+Compiled models are generated artifacts and must live outside the authored asset tree.
-tree.
 
 Default cache root:
 
@@ -338,21 +311,16 @@ The implementation must:
 - never download game data
 - never synthesize copyrighted content
 
-If only `data/nwn_base.key` exists, the implementation may create a staged alias
+If only `data/nwn_base.key` exists, the implementation may create a staged alias view for the compiler by:
-view for the compiler by:
 
 - symlinking inside a cache-owned staging dir, or
-- copying just the key file names into a cache-owned staging dir when symlinks
+- copying just the key file names into a cache-owned staging dir when symlinks are unavailable
-  are unavailable
 
-Do not mutate the user’s real install directory unless they explicitly pointed
+Do not mutate the user’s real install directory unless they explicitly pointed the config at a writable fake data tree and the implementation clearly documents that behavior.
-the config at a writable fake data tree and the implementation clearly documents
-that behavior.
 
 ## Wine Registry Contract
 
-When `runtime.setup_registry` is enabled, prepare the Wine registry keys needed
+When `runtime.setup_registry` is enabled, prepare the Wine registry keys needed by `nwnmdlcomp.exe`:
-by `nwnmdlcomp.exe`:
 
 ```text
 HKLM\Software\WOW6432Node\BioWare\NWN\Neverwinter
@@ -375,8 +343,7 @@ The implementation may either:
 - manage a dedicated toolkit prefix, or
 - use a user-provided existing prefix
 
-The contract should prefer an isolated prefix to avoid polluting unrelated Wine
+The contract should prefer an isolated prefix to avoid polluting unrelated Wine state.
-state.
 
 ## Compiler Invocation Contract
 
@@ -394,28 +361,24 @@ Implementation requirements:
 
 - invoke through `os/exec`, not shell string concatenation
 - convert input/output paths with `winepath -w`
-- allow compiler working directory configuration if supermodel resolution or
+- allow compiler working directory configuration if supermodel resolution or relative lookup requires it
-  relative lookup requires it
 - capture combined stdout/stderr per compile attempt
 
 ## Supermodel and sibling dependency considerations
 
-The plan must explicitly account for model compilation not being a pure
+The plan must explicitly account for model compilation not being a pure single-file transform.
-single-file transform.
 
 Potential dependencies include:
 
 - `setsupermodel` references
 - sibling helper models in the same asset family
-- textures or material references that may not block compilation but do matter
+- textures or material references that may not block compilation but do matter for diagnostics
-  for diagnostics
 
 Minimum contract:
 
 - compile against a staging layout that preserves original relative structure
 - ensure each model’s directory context exists in staging
-- document that some supermodel cases may require staging additional authored
+- document that some supermodel cases may require staging additional authored `.mdl` siblings into the compiler-visible workspace
-  `.mdl` siblings into the compiler-visible workspace
 
 Do not assume “compile one file in isolation” is always sufficient.
 
@@ -424,8 +387,7 @@ Do not assume “compile one file in isolation” is always sufficient.
 Preferred strategy:
 
 1. keep source inventory rooted in `paths.assets`
-2. materialize a compiler workspace under cache that mirrors required relative
+2. materialize a compiler workspace under cache that mirrors required relative directories for eligible `.mdl`
-   directories for eligible `.mdl`
 3. compile outputs into a sibling compiled tree
 4. package compiled output bytes while preserving source-relative logical names
 
@@ -475,8 +437,7 @@ The build must fail when an eligible model compile:
 - produces a zero-byte output file
 - cannot be staged or fingerprinted
 
-Warnings from the compiler do not fail the build unless future config enables
+Warnings from the compiler do not fail the build unless future config enables that policy.
-that policy.
 
 Failure output must include:
 
@@ -494,13 +455,11 @@ The canonical user-facing entrypoint remains:
 sow-toolkit build-haks
 ```
 
-Add targeted flags only if they materially improve operability and match
+Add targeted flags only if they materially improve operability and match existing CLI patterns. Recommended flags:
-existing CLI patterns. Recommended flags:
 
 - `build-haks --force-models`
 - `build-haks --skip-models`
-- `build-haks --model <glob-or-prefix>` optional, only if there is a real use
+- `build-haks --model <glob-or-prefix>` optional, only if there is a real use case for scoped rebuilds
-  case for scoped rebuilds
 
 Flag rules:
 
@@ -508,21 +467,16 @@ Flag rules:
 - `--skip-models` must bypass model preparation even if config is enabled
 - `--force-models` must invalidate incremental reuse for model outputs only
 
-Do not introduce a parallel standalone command as the only way to use the
+Do not introduce a parallel standalone command as the only way to use the feature. A helper/diagnostic subcommand is acceptable later, but build-haks must own the real integration.
-feature. A helper/diagnostic subcommand is acceptable later, but build-haks must
-own the real integration.
 
 ## Validation Contract
 
-Current validation already reports authored ASCII `.mdl` as informational.
+Current validation already reports authored ASCII `.mdl` as informational. That behavior should remain, but the plan should extend validation to cover model compilation readiness when the feature is enabled.
-That behavior should remain, but the plan should extend validation to cover model
-compilation readiness when the feature is enabled.
 
 Recommended validation additions:
 
 - compiler path resolves or search candidates exist
-- `wine` and `winepath` are resolvable on non-Windows platforms when mode is
+- `wine` and `winepath` are resolvable on non-Windows platforms when mode is `wine`
-  `wine`
 - NWN data root exists and has required keys
 - cache path is safe and repo-relative unless explicitly documented otherwise
 - include/exclude glob config is well-formed
@@ -530,24 +484,19 @@ Recommended validation additions:
 
 Validation severity:
 
-- config/discovery failures that make enabled compilation impossible should be
+- config/discovery failures that make enabled compilation impossible should be errors
-  errors
 - authored ASCII `.mdl` detection remains info
 - binary `.mdl` sources alongside enabled compilation are not errors
 
 ## Manifest And Reuse Contract
 
-Generated `haks.json` must continue to describe authored asset-relative paths,
+Generated `haks.json` must continue to describe authored asset-relative paths, not cache file paths.
-not cache file paths.
 
-HAK reuse logic must account for compiled model output bytes. Reuse must not
+HAK reuse logic must account for compiled model output bytes. Reuse must not mistakenly treat an unchanged authored path as unchanged content if the compiled artifact changed.
-mistakenly treat an unchanged authored path as unchanged content if the compiled
-artifact changed.
 
 That means one of:
 
-- the existing asset `ContentID` for `.mdl` must reflect compiled output hash
+- the existing asset `ContentID` for `.mdl` must reflect compiled output hash when compilation is enabled, or
-  when compilation is enabled, or
 - chunk hashing must incorporate the prepared compiled output fingerprint
 
 Without this, stale HAK reuse would be incorrect.
@@ -561,8 +510,7 @@ Requirements:
 - default jobs is `1`
 - jobs > 1 must not corrupt outputs or logs
 - each compile must write to a unique output path and unique log path
-- shared registry/prefix setup must happen before parallel work or under safe
+- shared registry/prefix setup must happen before parallel work or under safe synchronization
-  synchronization
 
 If `nwnmdlcomp.exe` or Wine proves unstable under parallel execution:
 
@@ -572,9 +520,7 @@ If `nwnmdlcomp.exe` or Wine proves unstable under parallel execution:
 
 ## Testing Contract
 
-Implementation is not complete without automated tests. Use fake executables and
+Implementation is not complete without automated tests. Use fake executables and temporary directories rather than requiring a real Wine/NWN environment in unit tests.
-temporary directories rather than requiring a real Wine/NWN environment in unit
-tests.
 
 ## Unit tests
 
@@ -615,8 +561,7 @@ Keep and extend tests proving:
 
 ## Integration/manual acceptance tests
 
-Document real-world manual checks for a workstation with Wine and
+Document real-world manual checks for a workstation with Wine and `nwnmdlcomp.exe` available.
-`nwnmdlcomp.exe` available.
 
 ### Test 1: readiness validation
 
@@ -695,8 +640,7 @@ Optional but recommended:
 
 1. Schema and config foundation
    - add `models.compile` config structs to `internal/project/project.go`
-   - add effective config defaults and path expansion in
+   - add effective config defaults and path expansion in `internal/project/effective.go`
-     `internal/project/effective.go`
    - add project helper methods for compiler/cache/env resolution
 
 2. Validation foundation
@@ -704,8 +648,7 @@ Optional but recommended:
    - extend CLI validation reporting where needed
 
 3. Preparation layer
-   - add model compiler resolution, Wine resolution, NWN data staging, registry
+   - add model compiler resolution, Wine resolution, NWN data staging, registry setup, and incremental metadata logic
-     setup, and incremental metadata logic
    - add a `preparedModelAssets` representation
 
 4. Pipeline integration
@@ -730,24 +673,19 @@ Optional but recommended:
 
 These should be decided explicitly during implementation, not left implicit:
 
-1. Should the toolkit own a dedicated Wine prefix under cache by default, or
+1. Should the toolkit own a dedicated Wine prefix under cache by default, or require a user-provided prefix?
-   require a user-provided prefix?
+2. Should NWN data alias files be created in a separate staging tree, or should the configured data root be used directly when already compatible?
-2. Should NWN data alias files be created in a separate staging tree, or should
-   the configured data root be used directly when already compatible?
 3. Is parallel model compilation stable enough to expose beyond best-effort?
-4. Do we want dedicated `build-haks` flags now, or only config for the first
+4. Do we want dedicated `build-haks` flags now, or only config for the first implementation?
-   implementation?
 5. Should per-model logs always be persisted, or only on failure / debug mode?
 
 ## Acceptance Criteria
 
 The feature is complete when all of the following are true:
 
-- `sow-toolkit build-haks` can package compiled binary `.mdl` content from
+- `sow-toolkit build-haks` can package compiled binary `.mdl` content from authored ASCII `.mdl` source when `models.compile.enabled: true`
-  authored ASCII `.mdl` source when `models.compile.enabled: true`
 - authored `.mdl` files remain unchanged on disk
-- validation and topdata/autogen behaviors that depend on authored `.mdl` source
+- validation and topdata/autogen behaviors that depend on authored `.mdl` source still work
-  still work
 - generated HAK manifests still reference authored asset-relative paths
 - HAK reuse logic correctly tracks compiled output changes
 - the feature is opt-in, deterministic, and tested
@@ -763,8 +701,6 @@ The feature is complete when all of the following are true:
   - `internal/topdata/parts_discovery.go`
   - `README.md`
 - Upstream docs consulted for this contract:
-  - `nwn.wiki` MDL page: compiled models are recommended for release because
+  - `nwn.wiki` MDL page: compiled models are recommended for release because ASCII loads more slowly
-    ASCII loads more slowly
   - Neverwinter Vault `nwnmdlcomp` page: standard compile/decompile CLI usage
-  - Neverwinter Vault forum discussions: `nwnmdlcomp` expects old NWN registry
+  - Neverwinter Vault forum discussions: `nwnmdlcomp` expects old NWN registry keys/data layout and some supermodel cases need additional care
-    keys/data layout and some supermodel cases need additional care