feat: constants phases 1–3 + i18n phase 1 — the shared scan-and-codegen pass, complete

[apotema]

Implements phases 1 and 2 of RFC-CONSTANTS (labelle-engine#811, tracking labelle-engine#810).

# constants/decay.yaml
hunger:
  rate: 0.02          # need units per second
health:
  drain_rate: 0.0     # DISABLED — see CLAUDE.md

const C = @import("constants").C;
const r: f32 = C.decay.hunger.rate;    // comptime_float, coerces at the use site
// C.decay.hunger.rte does not compile, naming the missing declaration

warning: constants/decay.yaml:4: C.decay.health.drain_rate is never read

Phase 1 — parser, codegen, wiring

constants_yaml.zig — the strict-subset parser, hand-written, resolving Open Question 1: the subset is small enough that a parser is less code than a dependency, and it makes §2 enforceable by construction — nothing implements YAML 1.1 implicit typing, so nothing can silently apply it:

written	result
enabled: no	error: 'no' is ambiguous in YAML (1.1 reads it as a boolean). Write true/false, or quote it
delay: 12:30, id: 0755	refused as numeric literals, with the accepted grammar named
&anchor, !!tag, {a: 1}, - item, ---	rejected by name, not "syntax error"
version: 1.20	fine — scalars keep their source text, so the YAML float-truncation trap cannot exist

Source-text preservation is also what makes 5.0 emit as comptime_float and 5 as comptime_int — observable: the probe test does integer division on a C.* int.

constants_phase.zig — scan, validate, emit. One namespace per file, errors print file:line, nothing half-written lands. No constants/ → nothing emitted, byte-identical build.zig, stale constants.zig deleted.

Wiring — the promoted-scripts pattern: one constants module, overrideImport into game_mod, addImport on every artifact including the tests target.

Phase 2 — usage warnings, sound under aliasing

usage_scan.zig is the shared pass §6 calls for, parameterised by module name and root symbol so i18n's K reuses it unchanged.

The ruling (recorded in both RFCs) implemented with no dataflow analysis at all: a chain that stops at an interior node marks the whole subtree used, so const cfg = C.decay.hunger; is covered at the binding site and cfg never needs tracking. Renamed roots and re-rooted aliases are followed; every escape the scanner can't follow — the root passed as a value, a bare module binding, const x = f(C); — widens to all-used. Comments and string literals never count; foo.C.x is somebody else's C.

One direction, held everywhere: the scanner may under-report dead constants; it can never fire on a constant that is actually read. A warning that can be wrong teaches people to ignore the lint, and the lint is the point.

Verified

• 22 unit tests (parser, phase, scanner, tree-walk including the .labelle-skip)
• End-to-end on examples/null: direct use + alias-only use → warnings for exactly the two unread constants, nothing else
• Stale cleanup and byte-identical no-op re-verified after removing constants/
• Suite failure set byte-identical to main's 33 pre-existing environmental E2E failures (test: stop autocrlf from poisoning the embedded templates #657 fixes the 16 phantom golden failures separately)

Deliberately absent

Phase 3 (pack constants, game-over-pack precedence per RFC §1.1) and phase 4 (FP's 38-constant migration, domain by domain).

Summary by CodeRabbit

• New Features

  • Added optional generation of typed constants from YAML files, including nested values, pack-specific values, compatible overrides, deterministic output, and unused-constant warnings.
  • Added optional internationalization generation from JSONC locale files with typed keys, locale selection, fallbacks, environment initialization, placeholder formatting, and coverage validation.
  • Generated modules integrate across supported build targets when enabled.

• Bug Fixes

  • Invalid constants and locale files now provide detailed validation errors without partial output.
  • Stale generated modules are removed when source files are missing or empty.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

The assembler now parses constants YAML and locale JSONC files, generates deterministic constants.zig and i18n.zig modules, validates usage and locale coverage, removes stale output, and wires enabled modules into supported build targets.

Changes

Generated constants and internationalization

Layer / File(s)	Summary
Strict constants YAML parser src/constants_yaml.zig	Adds arena-backed parsing for mappings and scalar values. The parser validates structure, identifiers, duplicates, scalar syntax, and unsupported YAML constructs.
Generated symbol usage analysis src/usage_scan.zig, src/constants_phase.zig, src/i18n_phase.zig	Tracks imports, aliases, dotted accesses, subtree usage, and escapes while ignoring comments, literals, and excluded directories.
Constants generation phase src/constants_phase.zig, src/root.zig	Loads game and pack files, applies validated overrides, emits deterministic constants.zig, removes stale output, and reports validation errors without partial writes.
Internationalization parsing and generation src/config.zig, src/i18n_locales.zig, src/i18n_interp.zig, src/i18n_phase.zig	Adds i18n configuration, parses JSONC locales, validates reference coverage and placeholders, generates typed locale data, and supports fallback and strict coverage checks.
Generation orchestration and build wiring src/root.zig, src/build_files/build_zig.zig	Runs both generation phases and conditionally wires constants_mod and i18n_mod into game and artifact root modules.

Estimated code review effort: 5 (Critical) | ~90 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Root as root.zig
  participant Constants as constants_phase.runPhase
  participant I18n as i18n_phase.runPhase
  participant Build as build_zig.zig
  participant Artifacts as artifact root modules
  Root->>Constants: generate constants.zig from game and pack YAML
  Root->>I18n: generate i18n.zig from locale JSONC
  Constants-->>Root: return constants generation state
  I18n-->>Root: return i18n generation state
  Root->>Build: pass generation states
  Build->>Artifacts: attach enabled generated modules

Loading

Possibly related issues

• labelle-toolkit/labelle-engine#809 — Describes the locale scanning, generated i18n APIs, configuration, usage validation, interpolation, and locale switching implemented here.

Possibly related PRs

• labelle-toolkit/labelle-assembler#478 — Relates to pack source resolution and generated-target integration.
• labelle-toolkit/labelle-assembler#538 — Relates to shared build-file and root wiring.
• labelle-toolkit/labelle-assembler#549 — Relates to BuildZigOptions and generated module wiring.

Poem

A rabbit parses keys at dawn,
YAML and locales march on.
Modules bloom in build roots bright,
Missing files fade from sight.
Typed translations hop along.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the constants and i18n code-generation work, although it does not mention the included i18n phase 2 changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/constants-phase-1

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 7

🧹 Nitpick comments (4)

src/constants_yaml.zig (2)

468-478: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a tab-indentation case to the rejected-constructs test.

No test covers a tab in the indentation. That gap is why the unused has_tab flag reported at Lines 196-205 stayed hidden. Add the case so the intended message is pinned.

💚 Proposed test addition

     try testing.expect(std.mem.indexOf(u8, (try parseErr(a, "---\na: 1\n")).msg, "multi-document") != null);
+    try testing.expect(std.mem.indexOf(u8, (try parseErr(a, "a:\n\tb: 1\n")).msg, "tab") != null);

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/constants_yaml.zig` around lines 468 - 478, Extend the “rejected
constructs are named” test with an input containing tab indentation, and assert
that parseErr reports the intended tab-related error message. Use the existing
arena allocator and message-matching pattern so the has_tab detection path is
covered.

---

243-248: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Extend the ambiguity list to null and the single-letter YAML 1.1 booleans.

isForbiddenBool covers yes/no/on/off and the mixed-case true/false spellings. It does not cover y, Y, n, N, which YAML 1.1 also resolves as booleans. It also does not cover the null spellings null, Null, NULL, ~, which YAML resolves to null. Today owner: null and flag: n become the Zig strings "null" and "n". That is a silent typing surprise of the same class the module header says it rejects by name.

♻️ Proposed change

 fn isForbiddenBool(s: []const u8) bool {
-    const forbidden = [_][]const u8{ "no", "No", "NO", "yes", "Yes", "YES", "on", "On", "ON", "off", "Off", "OFF", "True", "TRUE", "False", "FALSE" };
+    const forbidden = [_][]const u8{ "no", "No", "NO", "yes", "Yes", "YES", "on", "On", "ON", "off", "Off", "OFF", "True", "TRUE", "False", "FALSE", "y", "Y", "n", "N" };
     for (forbidden) |f| {
         if (std.mem.eql(u8, s, f)) return true;
     }
     return false;
 }
+
+/// YAML resolves these to null; a constants file has no null, so name the
+/// trap instead of emitting the string "null".
+fn isForbiddenNull(s: []const u8) bool {
+    const forbidden = [_][]const u8{ "null", "Null", "NULL", "~" };
+    for (forbidden) |f| {
+        if (std.mem.eql(u8, s, f)) return true;
+    }
+    return false;
+}

Then reject it in classify beside the bool check:

         if (isForbiddenBool(text)) {
             return failc(self.arena, line, "'{s}' is ambiguous in YAML (1.1 reads it as a boolean). Write true/false, or quote it: \"{s}\"", .{ text, text });
         }
+        if (isForbiddenNull(text)) {
+            return failc(self.arena, line, "'{s}' reads as null in YAML, and constants have no null. Quote it if it is meant as a string: \"{s}\"", .{ text, text });
+        }

Also applies to: 299-305

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/constants_yaml.zig` around lines 243 - 248, Extend isForbiddenBool to
recognize YAML 1.1 single-letter booleans y, Y, n, and N, plus null spellings
null, Null, NULL, and ~; ensure classify rejects these ambiguous scalars
alongside the existing boolean check instead of treating them as strings.

src/constants_phase.zig (2)

206-213: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Extract the tmp-path setup and cover the empty-constants/ branch.

The four tests repeat the same eight lines that rebuild .zig-cache/tmp/<sub_path> and join game and target. The layout assumption is duplicated four times, so a change in the std.testing.tmpDir layout breaks all four in the same way. Extract one helper that returns both paths.

No test covers the branch at Lines 85-89: a constants/ directory that exists but holds no *.yaml. That branch also deletes the stale file and returns false. Please add a case for it.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/constants_phase.zig` around lines 206 - 213, Extract the repeated
`.zig-cache/tmp/<sub_path>` path construction from the four tests into a shared
helper returning the `game` and `target` paths, and update those tests to use
it. Add a test for an existing empty `constants/` directory, verifying the stale
file is removed and `runPhase` returns false.

---

24-35: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value

Synchronize lazy std.Io initialization.

runPhase accesses _io_ready, _threaded, and _io without synchronization. Concurrent first calls can create multiple std.Io.Threaded instances and race on _io. Protect initialization with a once mechanism supported by the targeted Zig 0.16 toolchain. Do not assume that std.once is available.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/constants_phase.zig` around lines 24 - 35, Synchronize lazy
initialization in phaseIo by replacing the unsynchronized _io_ready check and
shared-state writes with a once mechanism supported by the targeted Zig 0.16
toolchain, such as std.Io.Once if available. Ensure the initialization of
_threaded and _io occurs exactly once before returning _io, and do not rely on
std.once.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/build_files/build_zig.zig`:
- Line 1128: Update the build flow around emitConstantsModule and
emitPromotedScriptModules so promoted modules receive opts.constants, emit
constants_mod before generating them, and conditionally add the constants import
when constants are configured. Preserve behavior when no constants or promoted
scripts are enabled.
- Around line 146-168: Update the iOS and Android alias conditions that declare
`target` to also include `opts.constants`, ensuring constants-only builds define
the target consumed by emitConstantsModule. Add regression cases covering
minimal constants-only iOS and Android projects.

In `@src/constants_phase.zig`:
- Around line 73-78: Update the directory iteration loop around
src_dir.iterate() and iter.next(io) to propagate iteration errors instead of
converting them to null; preserve normal loop termination and the existing file
and YAML filtering behavior.
- Around line 170-179: Update writeEscaped so remaining control bytes are
emitted as \xNN escapes instead of raw bytes, while preserving the existing
handling for backslash, quote, newline, tab, and carriage return. Classify bytes
in the control range and format each as a two-digit hexadecimal escape before
writing.
- Around line 141-163: Update identifier validation and formatting used by
generated filenames and mapping keys: reject invalid names, Zig keywords, and
“_” with source-line diagnostics, or emit them using quoted Zig identifiers.
Ensure emitMapping uses this formatter for declaration names rather than writing
keys directly. Extend writeEscaped to escape all control bytes, not only
newline, tab, and carriage return.

In `@src/constants_yaml.zig`:
- Around line 196-205: Update nextLine to set has_tab = true on the sentinel
Line returned for tab-indented input, so the existing Line.has_tab guard emits
the intended tab error before indentation matching or block-state updates.
Shorten the surrounding stale comment to describe the flag-based rejection path
rather than the old sentinel-only approach.
- Around line 270-288: Update stripComment so an unquoted '#' is treated as a
comment marker only at the start of the input or when immediately preceded by
whitespace; preserve '#' characters within bare scalars such as C#. Keep the
existing quoted-string handling and return behavior unchanged.

---

Nitpick comments:
In `@src/constants_phase.zig`:
- Around line 206-213: Extract the repeated `.zig-cache/tmp/<sub_path>` path
construction from the four tests into a shared helper returning the `game` and
`target` paths, and update those tests to use it. Add a test for an existing
empty `constants/` directory, verifying the stale file is removed and `runPhase`
returns false.
- Around line 24-35: Synchronize lazy initialization in phaseIo by replacing the
unsynchronized _io_ready check and shared-state writes with a once mechanism
supported by the targeted Zig 0.16 toolchain, such as std.Io.Once if available.
Ensure the initialization of _threaded and _io occurs exactly once before
returning _io, and do not rely on std.once.

In `@src/constants_yaml.zig`:
- Around line 468-478: Extend the “rejected constructs are named” test with an
input containing tab indentation, and assert that parseErr reports the intended
tab-related error message. Use the existing arena allocator and message-matching
pattern so the has_tab detection path is covered.
- Around line 243-248: Extend isForbiddenBool to recognize YAML 1.1
single-letter booleans y, Y, n, and N, plus null spellings null, Null, NULL, and
~; ensure classify rejects these ambiguous scalars alongside the existing
boolean check instead of treating them as strings.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 3e835472-15f4-4484-8630-ce7e7e33802e

📥 Commits

Reviewing files that changed from the base of the PR and between 3bce493 and 6dfd91d.

📒 Files selected for processing (4)

• src/build_files/build_zig.zig
• src/constants_phase.zig
• src/constants_yaml.zig
• src/root.zig

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6dfd91de1c

ℹ️ 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".

[chatgpt-codex-connector]

Wire constants into promoted script modules

When a FlowNodes-bearing game script uses @import("constants"), it is compiled as its own named module, but that module's import table is created before this call and never receives constants_mod. Adding the module only to game_mod and artifact roots does not make it visible to the standalone promoted module, so otherwise valid constants usage in these scripts fails with an unresolved module import.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Preserve or reject unsupported double-quoted escapes

For valid YAML escape sequences outside this small switch, the parser silently drops the backslash; for example, label: "\u00e9" is emitted as "u00e9" rather than "é". This silently corrupts constant values, so unsupported escapes should produce ConstantsInvalid or be decoded according to YAML rather than normalized by removing the slash.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Reject constants imports that collide with plugins

When a project has a plugin named constants and also adds a constants/ directory, the plugin was already installed in game_mod under this same key, and this overrideImport replaces it with the generated data module. The artifact-level addImport("constants", ...) similarly replaces the plugin import, so generated registries that call @import("constants") receive the wrong module and fail; reserve this name for the feature or diagnose the collision.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Reject Zig-reserved constant names

This character-class check accepts Zig keywords and primitives, so YAML such as fn: 1 or type: 1 passes generation but produces an invalid declaration such as pub const fn = 1;. Validate against Zig's keyword/primitive tables or emit names through std.zig.fmtId; the analogous filename-stem check in constants_phase.zig needs the same treatment.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Propagate constants directory iteration failures

If directory iteration fails after yielding some entries—for example on a transient or permission-related filesystem error—catch null treats the failure as normal EOF and generates constants.zig from only the files seen so far. This can silently omit constants or even disable the module entirely; propagate the iterator error with try as the repository's other generation scans do.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Preserve hashes inside bare scalar values

When a bare scalar contains # without preceding whitespace, YAML treats it as part of the scalar, but this lexer always starts a comment; for example, id: icon#selected is silently emitted as "icon". Preserve such hashes or reject the scalar explicitly instead of generating a truncated constant value.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Reject unsupported implicit null scalars

Bare YAML null spellings such as null, Null, NULL, and ~ fall through here and become ordinary strings even though the constants type model has no null kind. Thus target: null succeeds and emits target = "null", silently changing the authored value; reject these ambiguous spellings and require quotes when the literal text is intended.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Reject signed leading-dot numerics before string fallback

For number-shaped values such as -.5 or +.5, this removes only the sign and then requires the next character to be a digit, so the value falls through as an ordinary string instead of triggering the numeric-literal error used for .5. These inputs should be rejected under the documented numeric grammar rather than silently changing from a numeric value to a string.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fbef4dfaf4

ℹ️ 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".

[chatgpt-codex-connector]

Emit the cross-target alias when constants are enabled

For iOS or Android projects that enable constants but have no plugins, non-mock ECS, GUI, or promoted scripts, the alias guards at lines 937 and 951 do not emit const target = ios_target/android_target; this newly generated module nevertheless references target. Since .mock is the ECS default, a minimal mobile game with constants/*.yaml produces a build.zig with an undefined identifier. Include opts.constants in both alias conditions or use the platform-specific target here.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Decode doubled quotes in single-quoted YAML

When a single-quoted YAML value contains an apostrophe, YAML escapes it by doubling the quote; for example, label: 'it''s ready' represents it's ready. Copying the interior verbatim instead emits it''s ready, silently changing the constant's value. Decode doubled single quotes according to YAML or reject this syntax explicitly.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Recognize whitespace around import arguments

When valid Zig source writes an import with trivia, such as @import ("constants").C, this exact-prefix matcher misses the import and never registers the C root. Subsequent real constant accesses are therefore unmarked, causing warnUnused to report constants that are actually read. Tokenize the builtin call rather than requiring the source bytes to be formatted exactly as @import("...").

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Suppress warnings when source files cannot be scanned

When a game Zig file cannot be read—for example because it exceeds the 4 MiB limit or encounters a transient I/O error—this catch continue omits all uses from that file but still completes collectMarks successfully. warnUnused then emits false warnings for constants that the skipped file actually reads, contrary to the stated fail-silent behavior. Propagate the scan failure so the outer catch {} suppresses the warning pass, or conservatively mark everything used.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Run the unused-constant scan only once per generation

The normal generate command first generates the game target and then calls generateTestsTarget, and both paths invoke runPhase, so every unused-constant diagnostic is printed twice and the entire game source tree is scanned twice. Gate this warning pass off for the tests target, or perform it once outside the per-target generation path.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Discover constant aliases before scanning their uses

Zig declarations are not order-dependent, so valid code can use C.decay.rate in a function before a later file-level const C = @import("constants").C;. This single top-down pass ignores the earlier access because C has not yet been added to roots, and it never revisits it after discovering the alias, producing a false unused warning. Discover root aliases in a preliminary pass or rescan after the alias table is complete.

Useful? React with 👍 / 👎.

[apotema]

Phase 3 pushed (770b21e) — the PR now carries the complete RFC-CONSTANTS implementation apart from phase 4 (FP's migration, which belongs in the game repo).

Pack constants surface as C.<pack>__<file>.*; the game overrides by filename (constants/citizens__hunger.yaml), keeping "filename is the namespace" a single rule. The merge is deep and enforces both §1.1 rules with errors that name the path:

constants/citizens__hunger.yaml:1: 'rte' overrides nothing -- the pack does not define it.
  A typo here, or a key the pack renamed, would otherwise silently keep the pack's value
constants/citizens__hunger.yaml:1: 'rate' changes kind from float to int -- values are
  untyped and coerce at the use site... Keep the kind: write 5.0, not 5

The usage scan (phase 2) now also walks pack source dirs outside the game tree, since a cached pack's own scripts are the likeliest readers of its own constants — not scanning them would fire false "unused" warnings, the forbidden direction.

27 unit tests; the suite's failure set remains byte-identical to main's pre-existing 33.

[coderabbitai]

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/constants_phase.zig (1)

355-359: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Remove the duplicated constants/ prefix in the warning.

leaf.file already holds the display path. Line 138 sets it to constants/<name>.yaml, and line 90 sets it to packs/<pack>/constants/<name>.yaml. Line 357 prepends constants/ again, so the warning prints constants/constants/decay.yaml:3 or constants/packs/citizens/constants/hunger.yaml:2. Neither path exists.

🐛 Proposed fix

-            std.debug.print("warning: constants/{s}:{d}: C.{s} is never read\n", .{ leaf.file, leaf.line, leaf.path });
+            std.debug.print("warning: {s}:{d}: C.{s} is never read\n", .{ leaf.file, leaf.line, leaf.path });

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/constants_phase.zig` around lines 355 - 359, Remove the hardcoded
`constants/` prefix from the warning format in the `leaves` iteration, and use
`leaf.file` directly so paths set by the leaf construction logic display exactly
once.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/constants_phase.zig`:
- Around line 84-86: Update the openDir error handling near pdir_path and pdir
to continue only for error.FileNotFound and error.NotDir, matching the existing
game-path handling; propagate all other errors instead of silently skipping the
pack.
- Around line 110-113: Add a uniqueness check for every namespace before
appending to reg.entries, including plain game-domain entries and the
pack-derived namespace in the shown registration flow. When a duplicate is
found, reject it with an error identifying both defining files; otherwise
preserve the existing append behavior.
- Around line 349-353: Update the filtering loop around collectMarks so the
“inside game tree” check is path-aware rather than using std.mem.startsWith on
raw strings. Normalize or compare paths with a component-boundary-aware
relation, handling relative and absolute game_dir/src_dir consistently, so
siblings such as game_extra are scanned; alternatively remove the filter and
always collect pack marks as permitted by the surrounding logic.

---

Outside diff comments:
In `@src/constants_phase.zig`:
- Around line 355-359: Remove the hardcoded `constants/` prefix from the warning
format in the `leaves` iteration, and use `leaf.file` directly so paths set by
the leaf construction logic display exactly once.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e3942522-e9de-4740-962f-052b9a08b856

📥 Commits

Reviewing files that changed from the base of the PR and between fbef4df and 770b21e.

📒 Files selected for processing (4)

• src/build_files/build_zig.zig
• src/constants_phase.zig
• src/constants_yaml.zig
• src/root.zig

🚧 Files skipped from review as they are similar to previous changes (3)

• src/root.zig
• src/constants_yaml.zig
• src/build_files/build_zig.zig

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 770b21e79a

ℹ️ 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".

[chatgpt-codex-connector]

Wire constants into isolated pack modules

When a pack script reads its own generated namespace through @import("constants"), it is compiled under the isolated module created by emitPackModules, whose import table does not inherit either game_mod or the artifact root imports. Consequently, pack code—the source tree that warnUnused explicitly scans as a likely constants consumer—fails with an unresolved constants import; inject constants_mod into each pack module when constants are enabled.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Reject unterminated quoted scalars

When a value starts with a quote but does not close it, such as label: "ready, neither quoted branch matches and this fallback accepts the malformed YAML as a bare string. Generation then succeeds with a constant whose value includes the leading quote, silently changing an authoring typo into data; reject quote-prefixed scalars unless their quoted form is complete.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Preserve override source locations in unused warnings

When a game override replaces a pack scalar and the resulting constant is unused, this copies the override's line number into the pack tree but leaves the registry entry's file set to the original pack YAML. collectLeaves therefore reports the game line against the pack file, potentially directing the user to an unrelated line or even beyond that file's end; scalar provenance needs to carry the override file as well as its line.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Avoid duplicating the constants path in warnings

For every unused game constant, leaf.file already contains a path such as constants/decay.yaml (and pack leaves contain packs/<pack>/constants/...), so this additional constants/ prefix produces nonexistent locations such as constants/constants/decay.yaml and constants/packs/.... Print the stored path directly so diagnostics point to the authored file.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Sanitize valid pack names before forming namespaces

When a light pack has a valid project name containing punctuation, such as my-pack, the rest of the pack pipeline supports it by deriving my_pack through packNamespacePrefix, but this check rejects the same pack as soon as it ships constants. Use the established pack-name sanitizer when forming C.<pack>__<stem> so otherwise supported packs can participate in the constants feature and game override filenames can use the canonical prefix.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Check pack ancestry on a path boundary

When an external pack directory merely shares the game directory's byte prefix—for example, game /work/game and pack /work/game-pack—this test treats the pack as if it were already inside the scanned game tree and skips it. Uses from that pack are then omitted and valid pack constants receive false unused warnings; compare normalized path components or require a separator boundary after game_dir.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Reject colliding composite pack namespaces

When two otherwise valid packs and stems compose to the same identifier—for example pack a with b__c.yaml and pack a__b with c.yaml—both entries become C.a__b__c. The existing pack-prefix collision gate does not catch this because the pack prefixes themselves differ, so generation emits duplicate declarations and any override lookup is ambiguous; diagnose duplicate ns values before appending them.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Reject unsupported block-scalar indicators

When a YAML value is an empty literal or folded block scalar, such as message: | or message: > at EOF, this fallback accepts the indicator itself as an ordinary string and emits "|" or ">" rather than the YAML value (an empty string). Non-empty block scalars happen to fail later due to indentation, but the empty form is silently corrupted; explicitly reject |/> and their chomping variants as unsupported syntax.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Accept exponent-only floating literals

When a constant uses scientific notation without a decimal point, such as rate: 1e3, this mandatory-dot check rejects it even though it is a valid numeric spelling and the parser's own diagnostic advertises an optional exponent after the digit form. Recognize digits[eE][+-]?digits as a float so exponent notation does not fail generation solely because it omits a dot.

Useful? React with 👍 / 👎.

[apotema]

i18n phase 1 pushed (9a626be) — the branch now implements both RFCs' assembler-side scope: RFC-CONSTANTS phases 1–3 and RFC-I18N phase 1.

// locales/pt-BR.jsonc
{ "menu": { "new_game": "Novo Jogo" } }

const i18n = @import("i18n");
i18n.t(i18n.K.menu.new_game)      // "Novo Jogo"; K.menu.new_gme does not compile
i18n.setLocale("en");             // instant, no I/O, no failure path

warning: i18n: K.hud.frame_label is used but missing from: pt-BR (backfilled with en)

The payoff of building usage_scan.zig parameterised in phase 2 landed exactly as §6 predicted: the i18n coverage pass is the same scanner with module "i18n" and root "K" — zero new analysis code, alias widening and all.

Every §3 validation row for phase 1 is implemented and tested with the RFC's own examples: mandatory .default (no implicit "en"), tag-names-no-file with the scanned list, key-absent-from-reference (the rename catch), usage-aware warnings with .strict promotion, rectangular table via reference backfill — verified live on a generated module: default locale, backfill, switching, unknown-tag refusal, the LABELLE_LOCALE hook, and the typo compile error.

Deliberately absent per the phasing tables: i18n tf()/interpolation (phase 2 — the per-(key, locale) segment walk is specified in the RFC), pack locales + per-pack reference (phase 3), plurals (4), and the generated-main call to initFromEnvValue (rides with phase 2's main-template work).

20 new unit tests (55 total across the branch); suite failure set still byte-identical to main's pre-existing 33.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9a626be339

ℹ️ 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".

[chatgpt-codex-connector]

Keep conservative scan widening out of strict errors

When code uses the ordinary module-alias form const i18n = @import("i18n");, usage_scan.consumeAfterModule deliberately sets marks.all, so this loop treats every reference key as used. With .strict = true, any untranslated key then fails generation even if the game never accesses it, contradicting the usage-aware coverage contract; track module aliases precisely or avoid promoting conservatively widened marks to errors.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Escape decoded control bytes before emitting Zig

A valid JSON locale string such as "\u0000" is decoded by std.json to a literal NUL byte, but this fallback writes that byte directly into i18n.zig. Zig source cannot contain a raw NUL, so generation succeeds while the subsequently generated module fails to parse; emit \xNN escapes for control bytes rather than copying them verbatim.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Validate locale key depth before filling fixed arrays

The locale parser accepts arbitrarily nested objects, but a key with 17 or more segments writes past segs[16] here before any depth check. Such an otherwise structurally valid locale makes the assembler trap during code generation (and can become unchecked memory corruption in unsafe builds); either allocate the segment/namespace stacks dynamically or reject excessive depth with I18nInvalid.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Invoke the locale environment override during startup

When a developer or CI job sets LABELLE_LOCALE, nothing in the generated roots reads that variable or calls this hook—the only repository occurrence of initFromEnvValue is its declaration here. The promised override therefore has no effect unless every game manually implements environment lookup and invokes this function; wire it into generated startup code or make the module perform the lookup itself.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Reject unterminated JSONC block comments

When a /* comment reaches EOF without */ and the file ends in a newline, this loop blanks all comment content and exits because i + 1 == out.len, leaving valid JSON behind. The locale parser then accepts the malformed file instead of reporting the unterminated comment, so an authoring error can pass generation silently; track whether a closing delimiter was found and return a parse failure otherwise.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Validate locale filenames as BCP-47 tags

Every .jsonc filename stem is accepted as a locale even though this module defines filenames as BCP-47 declarations. A project can therefore generate and ship tags such as en_US, english, or an empty tag, which will not interoperate with callers or LABELLE_LOCALE values using standard language tags; validate each stem before adding it to tags and report invalid locale filenames.

Useful? React with 👍 / 👎.

[coderabbitai]

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/build_files/build_zig.zig (1)

1151-1157: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Wire generated modules into isolated module roots.

The constants and i18n imports on artifact roots do not propagate to promoted or pack modules. Emit constants_mod and i18n_mod before those modules. Add conditional imports to promoted modules. Pass opts.constants to emitPackModules and override each pack module's constants import with constants_mod.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/build_files/build_zig.zig` around lines 1151 - 1157, The module emission
flow around emitPromotedScriptModules and emitPackModules must wire shared
modules into isolated roots: emit constants_mod and i18n_mod before promoted and
pack modules, add conditional constants_mod/i18n_mod imports to promoted
modules, and change emitPackModules to receive opts.constants while overriding
each pack module’s constants import with constants_mod.

🧹 Nitpick comments (3)

src/i18n_phase.zig (2)

217-220: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Drop the unused reference_idx parameter.

emitModule discards reference_idx at Line 220. Remove the parameter and the argument at Line 190 instead of keeping the discard.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/i18n_phase.zig` around lines 217 - 220, Remove the unused reference_idx
parameter from emitModule and delete its discard assignment. Update the
emitModule call at the referenced call site to stop passing reference_idx, while
preserving the remaining arguments and behavior.

---

445-462: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Add a case for an unresolvable .reference.

This test covers .default naming no file. The parallel branch at Line 109, where .reference names no locale file, has no test. Add .{ .default = "en", .reference = "de" } and expect error.I18nInvalid.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/i18n_phase.zig` around lines 445 - 462, Add a test case in the existing
naming-validation test that runs runPhase with .default = "en" and .reference =
"de", where no de locale file exists, and assert that it returns
error.I18nInvalid. Keep the current default-name and missing-key assertions
unchanged.

src/i18n_locales.zig (1)

22-29: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Consider binary search in Locale.get.

entries is sorted by key, but get scans linearly. i18n_phase.emitModule calls get once per (locale, key) pair while building the table, and the coverage loop calls it again per (locale, used key). That makes table emission O(locales × keys²). A large key set makes this noticeable at build time.

♻️ Proposed binary search

     pub fn get(self: *const Locale, key: []const u8) ?[]const u8 {
-        // Sorted, so this could bisect; locale files are small enough that
-        // linear is clearer and the assembler runs this once per build.
-        for (self.entries) |e| {
-            if (std.mem.eql(u8, e.key, key)) return e.value;
-        }
-        return null;
+        // Sorted by key, so bisect: the phase calls this once per
+        // (locale, key) pair when emitting the table.
+        var lo: usize = 0;
+        var hi: usize = self.entries.len;
+        while (lo < hi) {
+            const mid = lo + (hi - lo) / 2;
+            switch (std.mem.order(u8, self.entries[mid].key, key)) {
+                .lt => lo = mid + 1,
+                .gt => hi = mid,
+                .eq => return self.entries[mid].value,
+            }
+        }
+        return null;
     }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/i18n_locales.zig` around lines 22 - 29, Update Locale.get to use binary
search over the sorted entries instead of scanning linearly, comparing each
entry key with key and narrowing the search range until a match is found or
returning null when absent. Preserve the existing return values and key ordering
assumptions.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/build_files/build_zig.zig`:
- Around line 169-181: Extend both iOS and Android target-alias guards to
activate when any of opts.constants, opts.i18n, or opts.pack_modules.len > 0 is
enabled, even without other guard conditions. Ensure target is defined before
generated modules such as emitI18nModule use .target = target, and add
regression coverage for each of the six platform/configuration combinations.

In `@src/i18n_phase.zig`:
- Around line 80-85: Update the directory iteration loop around
src_dir.iterate() and iter.next(io) so iteration errors are propagated instead
of converted to null. Preserve normal loop termination when iteration completes
successfully, while allowing I/O failures to return from the enclosing function
via its existing error mechanism.
- Around line 309-321: Bound segment parsing in emitKeyTree before assigning
segs[n], and reject keys exceeding the 16-level capacity with a diagnostic. Add
the corresponding depth validation in i18n_locales.flatten so the locale-file
error includes the offending key path, then add coverage for a 17-level key
while preserving existing behavior for keys within the limit.
- Around line 356-365: Update writeEscaped to escape every remaining control
byte: preserve the existing named escapes for backslash, quote, newline, tab,
and carriage return, and emit \xNN escapes for bytes below 0x20 or equal to 0x7F
instead of writing them directly. Continue passing through other bytes
unchanged.

---

Outside diff comments:
In `@src/build_files/build_zig.zig`:
- Around line 1151-1157: The module emission flow around
emitPromotedScriptModules and emitPackModules must wire shared modules into
isolated roots: emit constants_mod and i18n_mod before promoted and pack
modules, add conditional constants_mod/i18n_mod imports to promoted modules, and
change emitPackModules to receive opts.constants while overriding each pack
module’s constants import with constants_mod.

---

Nitpick comments:
In `@src/i18n_locales.zig`:
- Around line 22-29: Update Locale.get to use binary search over the sorted
entries instead of scanning linearly, comparing each entry key with key and
narrowing the search range until a match is found or returning null when absent.
Preserve the existing return values and key ordering assumptions.

In `@src/i18n_phase.zig`:
- Around line 217-220: Remove the unused reference_idx parameter from emitModule
and delete its discard assignment. Update the emitModule call at the referenced
call site to stop passing reference_idx, while preserving the remaining
arguments and behavior.
- Around line 445-462: Add a test case in the existing naming-validation test
that runs runPhase with .default = "en" and .reference = "de", where no de
locale file exists, and assert that it returns error.I18nInvalid. Keep the
current default-name and missing-key assertions unchanged.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 9ba9aec0-3591-4fc3-ab07-3a259975becf

📥 Commits

Reviewing files that changed from the base of the PR and between 770b21e and 9a626be.

📒 Files selected for processing (5)

• src/build_files/build_zig.zig
• src/config.zig
• src/i18n_locales.zig
• src/i18n_phase.zig
• src/root.zig

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Bound the segment count before writing into the fixed [16] buffers.

segs and stack are fixed at 16 entries, and the split loop writes segs[n] without a bounds check. i18n_locales.flatten places no limit on object nesting, so a locale file with 17 or more nested levels produces a key with 17 segments. That writes past the end of segs. A safe build panics; a release build corrupts the stack.

Reject over-deep keys with a diagnostic. The parser is the better place, because it can name the key path; a guard here is the minimum.

🛡️ Minimum guard in `emitKeyTree`

+const max_key_depth = 16;
+
 fn emitKeyTree(w: *std.Io.Writer, entries: []const locales_mod.Entry, base_depth: usize) !void {
-    var stack: [16][]const u8 = undefined;
+    var stack: [max_key_depth][]const u8 = undefined;
     var depth: usize = 0;
 
     for (entries, 0..) |e, idx| {
         // Split the key into segments.
-        var segs: [16][]const u8 = undefined;
+        var segs: [max_key_depth][]const u8 = undefined;
         var n: usize = 0;
         var it = std.mem.splitScalar(u8, e.key, '.');
         while (it.next()) |seg| {
+            if (n == max_key_depth) return error.KeyTooDeep;
             segs[n] = seg;
             n += 1;
         }

Add the matching check in i18n_locales.flatten so the failure surfaces as a locale-file error that names the key, and add a test for a 17-level key.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	fn emitKeyTree(w: *std.Io.Writer, entries: []const locales_mod.Entry, base_depth: usize) !void {
	var stack: [16][]const u8 = undefined;
	var depth: usize = 0;
	for (entries, 0..) |e, idx| {
	// Split the key into segments.
	var segs: [16][]const u8 = undefined;
	var n: usize = 0;
	var it = std.mem.splitScalar(u8, e.key, '.');
	while (it.next()) |seg| {
	segs[n] = seg;
	n += 1;
	}
	const max_key_depth = 16;
	fn emitKeyTree(w: *std.Io.Writer, entries: []const locales_mod.Entry, base_depth: usize) !void {
	var stack: [max_key_depth][]const u8 = undefined;
	var depth: usize = 0;
	for (entries, 0..) |e, idx| {
	// Split the key into segments.
	var segs: [max_key_depth][]const u8 = undefined;
	var n: usize = 0;
	var it = std.mem.splitScalar(u8, e.key, '.');
	while (it.next()) |seg| {
	if (n == max_key_depth) return error.KeyTooDeep;
	segs[n] = seg;
	n += 1;
	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/i18n_phase.zig` around lines 309 - 321, Bound segment parsing in
emitKeyTree before assigning segs[n], and reject keys exceeding the 16-level
capacity with a diagnostic. Add the corresponding depth validation in
i18n_locales.flatten so the locale-file error includes the offending key path,
then add coverage for a 17-level key while preserving existing behavior for keys
within the limit.

[apotema]

i18n phase 2 pushed (d1c6564) — tf(), placeholder parity, and the per-(key, locale) segment walk.

The design note the RFC evaluation added — formatting is a segment walk, not a comptime format string — is now load-bearing code, proven on a generated module:

i18n.tf(K.hud.stock, .{ .count = 3, .max = 10 })
// en: "3 of 10 items"
// de: "Von 10 Artikeln: 3"    ← same call, reordered by the active locale

The RFC §4's three examples verified as actual compile errors:

tf(K.hud.stock, .{ .count = 3 })           error: tf: missing argument 'max'
tf(K.hud.stock, .{ .cnt = 3, .max = 10 })  error: tf: missing argument 'count'
t(K.hud.stock)                             error: this key has placeholders; use tf

Parity compares placeholder sets (§3 row 4): German's reorder passes; a dropped {count} is a build error showing both sets. {{/}} escape, matching std.fmt.

Open Question 1 resolved pragmatically: the frame arena is module-owned — a 16 KiB ring, results valid until wrap, resetFrameArena() exported for the frame loop when main-template wiring lands. Truncate-never-fail; t()'s zero-failure path untouched.

28 unit tests here (83 on the branch); suite failure set still identical to main's 33.

Remaining per the phasing tables: i18n phase 3 (pack locales, game-over-pack, per-pack .reference), plurals (4), and the generated-main LABELLE_LOCALE/resetFrameArena wiring.

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (2)

src/i18n_interp.zig (1)

136-198: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider adding two edge cases to the parser tests.

The current tests do not cover an empty placeholder {} or adjacent placeholders {a}{b}. Both exercise branches that are easy to break later: the isIdentifier empty-string guard and the "flush pending literal" branch at Line 53.

♻️ Suggested additional test

test "empty and adjacent placeholders" {
    var arena_state = std.heap.ArenaAllocator.init(testing.allocator);
    defer arena_state.deinit();
    const a = arena_state.allocator();

    switch (try parse(a, "{}")) {
        .ok => return error.TestUnexpectedResult,
        .fail => |m| try testing.expect(std.mem.indexOf(u8, m, "identifier") != null),
    }

    const segs = try parseOk(a, "{a}{b}");
    try testing.expectEqual(`@as`(usize, 2), segs.len);
    try testing.expectEqualStrings("a", segs[0].arg);
    try testing.expectEqualStrings("b", segs[1].arg);
}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/i18n_interp.zig` around lines 136 - 198, Add a parser test covering both
edge cases: verify parse rejects the empty placeholder "{}" with an error
mentioning "identifier", and verify parsing adjacent placeholders "{a}{b}"
returns exactly two argument segments in order. Place the coverage alongside the
existing parser tests.

src/i18n_phase.zig (1)

751-766: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add the inverse parity case: the reference has no placeholders and a locale adds one.

The current test only covers a locale that drops a placeholder. The opposite direction reaches a different branch, because ref_names is empty and interps[ki] would become null if the parity check did not fire first at Line 219. A test locks that ordering in.

♻️ Suggested additional test

test "phase 2: a locale adding a placeholder the reference lacks is an error" {
    const io = phaseIo();
    var tmp = std.testing.tmpDir(.{});
    defer tmp.cleanup();
    try tmp.dir.createDirPath(io, "game/locales");
    try tmp.dir.createDirPath(io, "target");
    try tmp.dir.writeFile(io, .{ .sub_path = "game/locales/en.jsonc", .data = "{ \"hud\": { \"stock\": \"Stock\" } }" });
    try tmp.dir.writeFile(io, .{ .sub_path = "game/locales/pt.jsonc", .data = "{ \"hud\": { \"stock\": \"{count} itens\" } }" });

    const p = try tmpPaths(&tmp, testing.allocator);
    defer testing.allocator.free(p.game);
    defer testing.allocator.free(p.target);

    try testing.expectError(error.I18nInvalid, runPhase(testing.allocator, p.game, p.target, .{ .default = "en" }));
}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/i18n_phase.zig` around lines 751 - 766, Add the inverse
placeholder-parity test alongside the existing phase 2 test: in the reference
locale’s `hud.stock`, use text with no placeholder, and in `pt`, add a
placeholder. Keep the same temporary-directory setup, `runPhase` invocation, and
`error.I18nInvalid` expectation to verify parity is checked before interpolation
handling.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/i18n_phase.zig`:
- Around line 457-476: Update the generated appendValue helper’s integer and
float branches to use buffers large enough for the widest supported numeric
values, including i128/u128 and the supported float type. Replace both silent
catch return paths around std.fmt.bufPrint with visible fallback-marker output
so a formatting failure cannot remove the placeholder silently.
- Around line 419-455: Update the generated tf function and appendBytes flow so
each result reserves its required buffer region before walking segments,
truncates content within that reservation instead of wrapping mid-result, and
writes the terminating NUL directly with guaranteed space. Preserve valid
start/end slice bounds and sentinel termination across boundary crossings, then
add a generated-code test that invokes tf enough times to cross the frame buffer
boundary.

---

Nitpick comments:
In `@src/i18n_interp.zig`:
- Around line 136-198: Add a parser test covering both edge cases: verify parse
rejects the empty placeholder "{}" with an error mentioning "identifier", and
verify parsing adjacent placeholders "{a}{b}" returns exactly two argument
segments in order. Place the coverage alongside the existing parser tests.

In `@src/i18n_phase.zig`:
- Around line 751-766: Add the inverse placeholder-parity test alongside the
existing phase 2 test: in the reference locale’s `hud.stock`, use text with no
placeholder, and in `pt`, add a placeholder. Keep the same temporary-directory
setup, `runPhase` invocation, and `error.I18nInvalid` expectation to verify
parity is checked before interpolation handling.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a4cad70f-b3cd-4042-af21-654e02ed4d27

📥 Commits

Reviewing files that changed from the base of the PR and between 9a626be and d1c6564.

📒 Files selected for processing (2)

• src/i18n_interp.zig
• src/i18n_phase.zig

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d1c6564563

ℹ️ 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".

[chatgpt-codex-connector]

Rebase the result slice when the frame buffer wraps

When a tf result crosses the remaining 16 KiB capacity, this resets frame_len to zero after start was captured from the old buffer position. The return at line 430 then uses that stale start, which can form a reversed slice and trap or return unrelated bytes; earlier segments of the same result are also discarded. Wrap or reserve space for the entire result before capturing its start.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Decode escaped braces for non-interpolated keys

When a translation contains escaped braces but no placeholders, such as "Set {{name}}", parsing correctly produces the literal Set {name}, but this discards those decoded segments because the placeholder-name set is empty. The generated API consequently requires t(), which reads the original table string and displays the doubled braces; preserve the decoded text in the emitted table or otherwise retain this escape-only representation.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Allocate enough space for every supported integer

When a placeholder argument is a u128, i128, or sufficiently large comptime_int, its decimal representation can exceed this 24-byte buffer. bufPrint then returns error.NoSpaceLeft, and catch return silently omits the argument from the translated text even though appendValue advertises integer support; use formatting storage sized for the integer type or explicitly reject unsupported widths.

Useful? React with 👍 / 👎.

[apotema]

i18n phase 3 pushed (1a4ef04) — pack locales, closing the pack story symmetrically with constants. The branch now implements every assembler-side phase of both RFCs except plurals and the generated-main env wiring.

The scenario from the original ruling, verified as a test:

• pack ships en+fr; game ships en+pt
• pack keys surface as K.citizens__hunger.starving
• the game's en override wins over the pack's string
• the game adds pt for a pack that never shipped it — no fork
• hunger.fed, translated nowhere else, backfills from the pack's reference (§2.2's chain working by construction)
• the pack-only fr is warned about and skipped — the game's locales/ decides the Options menu
• "starvng" under the pack namespace is a build error naming pack and key

pack.labelle gains .i18n_reference, threaded from the manifest through root.zig.

The implementation shape is the part worth reviewing: everything folds into one merged key space before the existing pipeline, so the rename catch, coverage, placeholder parity, segment codegen and emission all run unchanged — nothing downstream knows packs exist. Same trick as constants phase 3.

31 i18n tests (86 on the branch); suite failure set still identical to main's 33.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1a4ef0404b

ℹ️ 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".

[chatgpt-codex-connector]

Add generated imports to promoted script modules

When a FlowNodes-bearing script imports constants or i18n, it is compiled as the isolated module created by emitPromotedScriptModules, whose import table contains neither generated module. Declaring these modules afterward and adding them to game_mod and artifact roots does not make them visible to the promoted module, so an otherwise valid promoted script fails with an unresolved import; wire both generated modules into every promoted script module.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Expose i18n to isolated pack modules

When a light-pack script accesses the locale keys shipped by that pack through @import("i18n"), this wiring exposes i18n_mod only to game_mod and artifact roots. The isolated modules created by emitPackModules do not inherit either import table and do not list i18n, so pack code using the newly supported pack-locales feature fails to compile; inject i18n_mod into each pack module when i18n is enabled.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Decode or reject unsupported YAML escape sequences

When a double-quoted constant uses a valid YAML escape not handled by this switch, the backslash is silently discarded rather than the escape being decoded or rejected. For example, icon: "\u263A" is generated as the literal text u263A instead of ☺, corrupting authored constant data while generation succeeds; implement the remaining accepted escapes or report them as unsupported.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Reject colliding composite i18n pack namespaces

When two valid pack/key combinations compose to the same prefixed key—for example pack a with key b__c and pack a__b with key c—both appends produce a__b__c. The merged reference then causes emitKeyTree to generate duplicate K declarations and the generated module fails to compile; detect duplicate composite keys or use an unambiguous canonical pack prefix before appending.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Preserve the pack reference as the fallback source

When a pack declares a reference different from the project reference and also ships the project's reference tag, this expression chooses that tag instead of pd.reference for the merged reference row. For example, with project reference en, pack reference fr, and pack files for both, a missing pt pack string is backfilled from English rather than the pack's declared French reference, contradicting the configured locale → pack-reference fallback chain and displaying the wrong language; keep the pack-reference value as the fallback independently of the translation used for the project's reference locale.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Scan external pack sources for strict i18n coverage

When a cached or otherwise external pack is the only code that reads one of its localized keys, this scan visits only game_dir, so the key remains unmarked. If one of the game's offered locales lacks that pack string, .strict = true therefore succeeds instead of reporting the used untranslated key; include each pack source directory in the usage scan, while avoiding rescanning packs already beneath the game tree.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Parse the pack i18n reference from the manifest

When pack.labelle declares .i18n_reference, the ZON parsing shape has no corresponding field and parsing uses ignore_unknown_fields = true, so the declaration is silently discarded and this public field remains null. root.zig consequently always passes the project reference to the i18n phase, causing packs authored in another locale either to require the wrong reference file or to use the wrong fallback; add the field to ZonPackManifest, transfer it into PackManifest, and release it in deinit.

Useful? React with 👍 / 👎.

[apotema]

Review triage — all 48 findings addressed in 2132749

Both bots did genuinely good work here. Verdict counts: 22 fixed, 2 already fixed before the review landed, 3 deferred with reasons, the rest were duplicates across the two bots or nitpicks folded into the fixes.

The two criticals — both real, both confirmed before fixing

tf ring wrap (CodeRabbit + Codex, independently) — appendBytes could reset frame_len mid-result, invalidating the captured start: a reversed slice (panic in safe builds) or bytes of an unrelated earlier string. Now: wrap only between results, truncating copy that never moves backwards, last byte reserved for the sentinel. Verified by hammering 3,000 tf calls through the 16 KiB ring on a generated module — every result well-formed, every sentinel intact.

Mobile target alias (CodeRabbit + Codex) — a minimal iOS/Android game with constants or i18n emitted a build.zig with an undefined target. The alias guards now include opts.constants, opts.i18n, and opts.pack_modules (the last was a pre-existing gap of the same shape).

Fixed, by area

Wiring: promoted-script and pack modules now import constants_mod/i18n_mod (both bots found this independently — it broke @import("constants") in exactly the sources the usage scanner covers); data modules emit first; plugin named constants/i18n refused by name.

YAML parser (7 silent corruptions now loud): icon#selected is data (# needs preceding whitespace); unterminated quotes error; null/~ named errors; '' decodes; unknown \u-style escapes refuse instead of dropping the slash; 1e3 is a float; -.5 hits the numeric error. Plus the tab sentinel that never set has_tab — the error the first commit intended finally fires.

Usage scanner: two passes per file (use-above-binding counted as nothing — declarations are order-independent); whitespace-tolerant @import (; unreadable files and aborted iterations widen to all-used rather than under-mark. And the strict×widening interplay Codex caught: widened marks are never promoted to strict errors, with a printed note — failing builds over keys nothing renders would invert the usage-aware contract.

Codegen: @""-quoted declarations (fn:/type: keys compile); \xNN for control bytes in both emitters (JSON-decoded NUL can no longer corrupt the [:0] sentinel); key depth bounded before the fixed stacks; escape-only strings decode in the table (t() shows Set {name}, not doubled braces); int/float format buffers sized for i128 with a visible ? on failure.

Structural: composite namespace collisions error naming both files; pack names sanitize like the rest of the pipeline (my-pack participates as my_pack); override unused-warnings point at the overriding file; path-boundary check (game-pack is not inside game); iteration errors propagate; unterminated /* refuses; BCP-47 filename validation naming pt_BR as the classic typo; diagnostics print once per generation, not once per target; the doubled constants/constants/ path in warnings.

Deferred, with reasons

• LABELLE_LOCALE not called from generated startup (Codex) — correct, and known: it needs main_template wiring, deferred twice already and tracked for the next leg. The hook is pure and tested; the call site is the remaining piece.
• Module-alias precision (const m = @import("i18n"); m.K... widens instead of tracking) — deliberate: tracking loosely risks under-marking, the forbidden direction. The strict-mode note makes the trade visible.
• Full YAML single-quote edge cases beyond '' — the strict subset's posture is reject-or-decode-simply; anything fancier belongs to a future OQ if anyone hits it.

95 unit tests across the six files (up from 86); suite failure set still byte-identical to main's pre-existing 33; examples/null regenerates clean.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2132749606

ℹ️ 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".

[chatgpt-codex-connector]

Gate reserved plugin names on generated modules

When a project uses an existing plugin named constants or i18n but has no corresponding constants/ or locales/ content, this unconditional check now aborts generation even though no generated module exists to collide with the plugin. Apply the reservation only when the relevant phase actually emits a module, preserving the promised no-op behavior for projects that do not enable these features.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Sanitize pack names when composing locale keys

When a valid pack name contains punctuation, such as my-pack, this emits my-pack__hunger.starving, but game locale keys must consist of Zig-identifier segments and therefore cannot spell that raw prefix. Using the canonical my_pack__... prefix instead fails the earlier lookup against pd.name, so such packs cannot use the advertised game override/additional-locale flow; compose and match keys with the same pack-name sanitizer used elsewhere.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Reject content after a quoted YAML scalar

When malformed input begins and ends with a quote but contains tokens after an earlier closing quote, such as label: "ready" trailing ", this condition accepts the entire span and emits the value ready" trailing instead of reporting invalid YAML. Validate the quoted scalar while scanning for its actual closing delimiter and reject any non-comment content that follows it.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Gate pack-locale warnings on the diagnostics flag

When a pack ships a locale the game does not offer, the normal generate command reaches this warning once for the game target and again through generateTestsTarget; unlike the other diagnostics in this phase, it ignores the diagnostics = false tests-target setting. The same warning is therefore printed twice for every such pack locale, so pass the diagnostics gate into this merge path before emitting it.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Reject single-letter YAML 1.1 booleans

When a constant is written as bare y, Y, n, or N, YAML 1.1 treats it as a boolean, but this incomplete forbidden-spelling list lets it fall through as a string. That reintroduces the same silent cross-version typing ambiguity this parser explicitly rejects for yes and no; include the single-letter forms so authors must write true/false or quote the intended text.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 58542b313c

ℹ️ 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".

[chatgpt-codex-connector]

Treat public root aliases as cross-file escapes

When a helper file re-exports the generated root with pub const K = @import("i18n").K; (or the equivalent C alias) and another file accesses keys through that helper, this records only a file-local root and the consuming file has no recognized generated-module import. The used key is therefore never marked: constants can receive a false unused warning, while strict i18n coverage can miss a translation that is used in production. Detect public re-exports or conservatively widen the scan when the root may escape its file.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Trim YAML separation tabs from scalar values

When a valid mapping uses a tab as separation after the colon, such as label:\tready, this trims only spaces and classifies the value as a string beginning with a literal tab. The generated constant consequently becomes "\tready" rather than "ready"; trailing separation tabs are likewise retained. Tabs should remain rejected in indentation, but inline YAML whitespace must be removed around the scalar.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Compare BCP-47 locale tags case-insensitively

When a locale file is named pt-BR.jsonc but LABELLE_LOCALE is set to the equivalent BCP-47 spelling pt-br, this bytewise comparison ignores the override and leaves the default language active. BCP-47 tags are case-insensitive, so the environment hook and setLocale should canonicalize tags or use an ASCII case-insensitive comparison.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Reject YAML special floating scalars

When a constant uses a YAML floating spelling such as .inf, -.Inf, or .nan, it matches neither the numeric recognizers nor looksNumeric and falls through as a bare string. This silently changes a YAML numeric scalar into text, contrary to the parser's rule that unsupported implicit typing must be rejected; explicitly refuse these spellings and require quoting when the text itself is intended.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Widen coverage when the source root cannot be opened

When the game source root cannot be opened for iteration, for example because direct access to locales/ is allowed but directory listing of the project root is denied, this returns with an empty used-key set. Strict coverage then treats every key as unused and can silently miss translations required by code; this I/O failure should set marks.all just like iteration and file-read failures do.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Preserve the environment fallback after a rejected locale

When application startup probes a persisted locale that is no longer shipped, setLocale returns false but sets env_checked before finding a match. A subsequent first lookup therefore never applies an otherwise valid LABELLE_LOCALE override and remains in the default locale, despite the API promising that a false return changes nothing; mark the environment settled only after a locale is successfully selected.

Useful? React with 👍 / 👎.

[apotema]

Round 2 triage — six for six, all fixed in 5df3ee5.

The standout: setLocale settled the env question before checking the tag, so a startup probe of a no-longer-shipped persisted locale returned false ("changes nothing") having silently eaten the LABELLE_LOCALE fallback. Verified live: a rejected setLocale("de") followed by the first t() still applies LABELLE_LOCALE=pt-BR.

Also: pub const K = @import(...).C is now a cross-file escape (widen — a per-file scan can't see the helper's consumers; private aliases still track precisely); BCP-47 tags compare case-insensitively (pt-br finds pt-BR); a tab after the colon is YAML separation, not value content; .inf/.nan spellings are named errors like null; and an unopenable source root widens both scanners to all-used instead of reading as everything-unused.

101 unit tests. Round 3 is the next push's reviews; per the standing directive this PR auto-merges after round 3 settles, CI green.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5df3ee5dc6

ℹ️ 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".

[chatgpt-codex-connector]

Parse quoted identifier segments in usage chains

When a generated key contains a Zig keyword, valid source must access it as, for example, K.menu.@"error". Here consumeDot advances past the second dot but nextIdent cannot consume the quoted identifier, so the scanner records only menu and later marks that entire subtree as used. With .i18n.strict = true, an untranslated sibling such as K.menu.help then causes generation to fail even though it is unused; consume quoted identifiers instead of truncating the chain.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Match pack locale tags case-insensitively

When the game offers pt-BR and an independently authored pack ships the equivalent pt-br.jsonc, this bytewise comparison leaves the pack locale unmapped and reports it as ignored. The merged pt-BR row then backfills the pack's keys from its reference locale, so users receive the wrong language even though BCP-47 tags are case-insensitive; canonicalize tags or use the same case-insensitive comparison as the generated runtime API.

Useful? React with 👍 / 👎.

[apotema]

Round 3 triage — two findings, both fixed in 67d4d11.

Both were fallout from round 2's own changes, which is what review rounds are for: the chain walker couldn't consume @"error"-style quoted identifiers (round 2 introduced @""-quoted codegen), truncating K.menu.@"error" at menu and widening the subtree — under .strict that fails builds over untranslated siblings nothing renders. And mergePacks still mapped pack locale files bytewise after round 2 made the runtime case-insensitive, so a pack's pt-br.jsonc was "ignored" against a game's pt-BR.

103 unit tests. This completes the three review rounds per the standing directive — merging when this push's CI settles green, unless a new major finding lands first.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 67d4d11919

ℹ️ 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".

[chatgpt-codex-connector]

Match configured locale tags case-insensitively

When a locale file is named pt-BR.jsonc but .i18n.default or .i18n.reference uses the equivalent BCP-47 spelling pt-br, this bytewise lookup rejects the configuration before generation. The generated runtime and pack merger already compare tags case-insensitively, so apply the same comparison when resolving configured tags.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Reject locale filenames that differ only by case

On case-sensitive filesystems, both en.jsonc and EN.jsonc are appended even though BCP-47 treats them as the same tag. The generated locale selector then exposes duplicate languages, and setLocale("en") selects whichever duplicate appears last rather than necessarily the configured default; reject case-insensitive duplicates while building the tag list.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Reject a trailing escape in double-quoted YAML

When a scalar ends with a backslash immediately before its apparent closing quote, such as label: "abc\", YAML treats that quote as escaped and the scalar as unterminated. Here the outer check accepts the final quote as the delimiter, while this loop copies the remaining backslash literally because it has no following byte, silently accepting malformed YAML as the value abc\; return an unterminated/invalid-escape error instead.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Scope discovered root aliases to their declarations

When two functions reuse the same local name, for example one declares const Keys = @import("i18n").K while another declares an unrelated local Keys, the first discovery pass places that name in a file-wide root set. Every Keys.foo in the second function is then treated as an i18n access, so .strict can fail on an untranslated key that production code never reads; track alias scope or conservatively handle rebinding instead of applying every discovered local alias across the whole file.

Useful? React with 👍 / 👎.