chore(config): gen-au-build + ctld-tools.exe + MM doc (CTLD-TOOLS-FINALIZE)#48
Merged
Conversation
…ge (CTLD-TOOLS-FINALIZE) Finalizes the ctld-tools program: - gen-au-build: merge_CTLD.ps1 regenerates src/CTLD_config_defaults.lua from the YAML via ctld-tools on every build; the file is now a git-ignored artifact (untracked). ci.yml (build + busted) and release.yml gain setup-python + poetry; busted generates the defaults before the suite; a conftest fixture does so for the Python tests. Drift check dropped. - ctld-tools.exe: built (PyInstaller, lupa + datamine bundled) and attached to each Release by a separate build-exe job, isolated so packaging never blocks CTLD.lua. Verified locally (validate runs from the packaged exe). - Docs: dedicated mission-maker/ctld-tools.md (EN+FR) + nav; developer build docs and ADR 0009 updated to describe gen-au-build.
Reviewer's GuideFinalizes ctld-tools by making CTLD_config_defaults.lua a generated, git-ignored build artifact produced on every build, wiring Python/poetry setup into CI and release workflows, introducing a dedicated job that builds and uploads ctld-tools.exe independently of the main release, and adding mission-maker and developer documentation plus test fixtures to keep parity tests passing without a committed defaults file. Sequence diagram for gen-au-build CTLD_config_defaults.lua regenerationsequenceDiagram
actor Developer
participant Merge_CTLD_ps1
participant CtldTools
participant SrcDir
Developer->>Merge_CTLD_ps1: run merge_CTLD.ps1
Merge_CTLD_ps1->>CtldTools: poetry run ctld-tools gen-config --yaml CTLD_config.yaml --out CTLD_config_defaults.lua
CtldTools->>SrcDir: write CTLD_config_defaults.lua
CtldTools-->>Merge_CTLD_ps1: exit 0
Merge_CTLD_ps1->>SrcDir: read CTLD_config_defaults.lua
Merge_CTLD_ps1-->>Developer: output merged CTLD.lua build artifact
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The
conftest.pysession fixture writessrc/CTLD_config_defaults.luainto the repo on every test run, which will leave local working trees dirty; consider generating into a temporary path and pointing the tests at that instead to keep the tree immutable. - All workflows are pinned to Python 3.13, which is relatively new and may be unstable on hosted runners; you might want to target a better-supported minor (e.g. 3.11/3.12) unless you rely on 3.13-specific features.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `conftest.py` session fixture writes `src/CTLD_config_defaults.lua` into the repo on every test run, which will leave local working trees dirty; consider generating into a temporary path and pointing the tests at that instead to keep the tree immutable.
- All workflows are pinned to Python 3.13, which is relatively new and may be unstable on hosted runners; you might want to target a better-supported minor (e.g. 3.11/3.12) unless you rely on 3.13-specific features.
## Individual Comments
### Comment 1
<location path=".github/workflows/release.yml" line_range="22-27" />
<code_context>
+ - uses: actions/setup-python@v6
+ with:
+ python-version: "3.13"
+ - name: Install ctld-tools (the build regenerates CTLD_config_defaults.lua via gen-config)
+ working-directory: tools/ctld-tools
+ shell: bash
+ run: |
+ pip install poetry
+ poetry install --without dev --without build
- name: Build CTLD.lua
run: powershell -ExecutionPolicy Bypass -File tools\build\merge_CTLD.ps1
</code_context>
<issue_to_address>
**suggestion (performance):** Consider caching Poetry and the virtualenv to avoid re-installing dependencies on every workflow run.
Given `ctld-tools` likely has non-trivial dependencies, this will add avoidable time and PyPI traffic on every run. You can cache the Poetry environment (e.g. `.venv` or `~/.cache/pypoetry`) with `actions/cache`, and reuse the same pattern in CI so builds stay faster and more consistent across workflows.
</issue_to_address>
### Comment 2
<location path="tools/build/merge_CTLD.ps1" line_range="23-32" />
<code_context>
+$ctldToolsDir = Join-Path $repoRoot "tools\ctld-tools"
+$configYaml = Join-Path $srcDir "CTLD_config.yaml"
+$defaultsLua = Join-Path $srcDir "CTLD_config_defaults.lua"
+Write-Host "Generating CTLD_config_defaults.lua from CTLD_config.yaml (ctld-tools)..."
+Push-Location $ctldToolsDir
+try {
+ & poetry run ctld-tools gen-config --yaml $configYaml --out $defaultsLua
+ if ($LASTEXITCODE -ne 0) { throw "gen-config returned $LASTEXITCODE" }
+}
+catch {
+ Write-Host "[ERROR] Could not generate CTLD_config_defaults.lua via ctld-tools."
+ Write-Host " Run 'poetry install' in tools/ctld-tools first (needs Python + poetry)."
+ Pop-Location
+ exit 1
+}
</code_context>
<issue_to_address>
**suggestion:** Use a `try { ... } finally { Pop-Location }` structure to guarantee location restoration.
Currently `Pop-Location` is called in the `catch` and again after the `try/catch`, which only works because the `catch` path exits the script. That coupling to control flow is fragile. Wrapping this in `try { ... } finally { Pop-Location }` ensures the location is always restored, even if additional error branches or exits are introduced later.
Suggested implementation:
```
Write-Host "Generating CTLD_config_defaults.lua from CTLD_config.yaml (ctld-tools)..."
Push-Location $ctldToolsDir
try {
& poetry run ctld-tools gen-config --yaml $configYaml --out $defaultsLua
if ($LASTEXITCODE -ne 0) { throw "gen-config returned $LASTEXITCODE" }
}
catch {
Write-Host "[ERROR] Could not generate CTLD_config_defaults.lua via ctld-tools."
Write-Host " Run 'poetry install' in tools/ctld-tools first (needs Python + poetry)."
exit 1
}
finally {
Pop-Location
}
```
None required beyond this edit; the try/catch/finally block now guarantees `Pop-Location` is always executed, independent of control flow, and removes the previous coupling between error handling and location restoration.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+22
to
+27
| - name: Install ctld-tools (the build regenerates CTLD_config_defaults.lua via gen-config) | ||
| working-directory: tools/ctld-tools | ||
| shell: bash | ||
| run: | | ||
| pip install poetry | ||
| poetry install --without dev --without build |
There was a problem hiding this comment.
suggestion (performance): Consider caching Poetry and the virtualenv to avoid re-installing dependencies on every workflow run.
Given ctld-tools likely has non-trivial dependencies, this will add avoidable time and PyPI traffic on every run. You can cache the Poetry environment (e.g. .venv or ~/.cache/pypoetry) with actions/cache, and reuse the same pattern in CI so builds stay faster and more consistent across workflows.
Comment on lines
+23
to
+32
| Write-Host "Generating CTLD_config_defaults.lua from CTLD_config.yaml (ctld-tools)..." | ||
| Push-Location $ctldToolsDir | ||
| try { | ||
| & poetry run ctld-tools gen-config --yaml $configYaml --out $defaultsLua | ||
| if ($LASTEXITCODE -ne 0) { throw "gen-config returned $LASTEXITCODE" } | ||
| } | ||
| catch { | ||
| Write-Host "[ERROR] Could not generate CTLD_config_defaults.lua via ctld-tools." | ||
| Write-Host " Run 'poetry install' in tools/ctld-tools first (needs Python + poetry)." | ||
| Pop-Location |
There was a problem hiding this comment.
suggestion: Use a try { ... } finally { Pop-Location } structure to guarantee location restoration.
Currently Pop-Location is called in the catch and again after the try/catch, which only works because the catch path exits the script. That coupling to control flow is fragile. Wrapping this in try { ... } finally { Pop-Location } ensures the location is always restored, even if additional error branches or exits are introduced later.
Suggested implementation:
Write-Host "Generating CTLD_config_defaults.lua from CTLD_config.yaml (ctld-tools)..."
Push-Location $ctldToolsDir
try {
& poetry run ctld-tools gen-config --yaml $configYaml --out $defaultsLua
if ($LASTEXITCODE -ne 0) { throw "gen-config returned $LASTEXITCODE" }
}
catch {
Write-Host "[ERROR] Could not generate CTLD_config_defaults.lua via ctld-tools."
Write-Host " Run 'poetry install' in tools/ctld-tools first (needs Python + poetry)."
exit 1
}
finally {
Pop-Location
}
None required beyond this edit; the try/catch/finally block now guarantees Pop-Location is always executed, independent of control flow, and removes the previous coupling between error handling and location restoration.
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
1 participant
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.
Finalizes the
ctld-toolsprogram (per maintainer feedback after lots 1–3).What
merge_CTLD.ps1regeneratessrc/CTLD_config_defaults.luafromCTLD_config.yamlviactld-tools gen-configon every build. The file is now a git-ignoredartifact (untracked).
ci.yml(build+busted) andrelease.ymlgainsetup-python+poetry;
bustedgenerates the defaults before the suite; aconftest.pyfixture does so for thePython tests. Drift check dropped — dev workflow is just "edit YAML, rebuild".
ctld-tools.exe— built (PyInstaller, lupa + datamine bundled) and attached to each Release bya separate
build-exejob (needs: release), isolated so a packaging failure never blocks theCTLD.luarelease. Verified locally:validateruns from the packaged exe (lupa + embeddeddcs_types.jsonwork).mission-maker/ctld-tools.md(EN + FR): fulluser-config.yamlformat,commands, examples (block + flow), ME loading. Linked in the nav. Developer build docs + ADR 0009
updated to describe gen-au-build.
Validation
defaults.luadeleted (conftest regens).merge_CTLD.ps1tested locally end to end (regenerate → merge → parse OK).Notes
.mizinjection, TUI.Summary by Sourcery
Finalize ctld-tools integration by generating engine defaults at build time, packaging and attaching ctld-tools.exe to releases, and adding dedicated mission-maker documentation.
New Features:
Build:
CI:
Documentation:
Tests:
Chores: