Repository navigation
DECLARE_RUNNERS per-language table + components/ collection root - #615
Conversation
Ruby declare-mode wires into generate (labelle-scripting v0.9.0's
tools/declare-ruby): DECLARE_LANGUAGE becomes the DeclareRunner table
(lua -> labelle-declare/.lua, ruby -> labelle-declare-ruby/.rb) with
a min_pin per row so the absent-tool note stays pointed ('pin
labelle-scripting >= 0.9.0'); per-row filesystem capability probe —
older pins gracefully skip exactly as before; the in-process tool
cache keys pkg+step so a lua build can never satisfy a ruby lookup;
non-listed languages get the reworded not-yet-supported note.
parseSchema untouched — the scripting repo's cross-runner byte-parity
golden is the contract proof.
Component declarations live where their kind lives (the engine#237
refinement): declare collection = components/*.<ext> UNION in-script
chunk-scope declarations (both feed the runner, components first —
the runner owns duplicate attribution); components/ language files
are also EMBEDDED and register BEFORE scripts (plain alphabetical,
no ordering prefixes — the view constants exist when scripts load;
the enabler: EmbedScript.file went target-relative so two convention
dirs feed one emission stream, generated bytes unchanged). Recursive
organizational subdirs mirror the Zig component scan; a typescript
authoring source in components/ is a pointed error (.d.ts exempt,
runnable .js embeds); applies on the legacy-grace path too. Policy
scan now polices scripts/ and components/ identically.
+17 tests incl. the union argv pin, the ruby declare e2e (fake
tools/declare-ruby: components/hunger.rb -> generated component +
registry + registered before scripts/), old-pin graceful skip, and
the transpile-swap components-prefix preservation. Negative-verified:
inverted ordering and stubbed components collection each fail exactly
the two ordering pins. 1968/1972 clean-HOME, authoritative exit 0.
Claude-Session: https://claude.ai/code/session_01P7YLw4hXFCCaY2LAUt4G1j
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Warning Review limit reached
Next review available in: 51 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. 📝 WalkthroughWalkthroughEmbedded scripting now collects component declarations alongside script-directory files, uses target-relative embed paths, applies language checks to both shared directories, and supports Lua/Ruby declaration runners with component-first input ordering. ChangesScripting pipeline
Estimated code review effort: 4 (Complex) | ~60 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/language_policy.zig (1)
529-532: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winExclude
.d.tsfiles from shared-directory language buckets.This classifies declaration-only files as TypeScript because they end in
.ts, contradicting the splice contract that.d.tsis not a runnable script. A Lua or script-less project withcomponents/labelle.d.tscurrently fails policy validation before collection.Proposed fix
else => { + if (std.mem.endsWith(u8, entry.name, ".d.ts")) continue; for (SUPPORTED_LANGUAGES, buckets) |lang, *bucket| {🤖 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/language_policy.zig` around lines 529 - 532, Update the shared-directory matching logic around matches to exclude entries ending in “.d.ts” before applying scriptExtensions(lang), while preserving normal TypeScript and other runnable-script classification.
🤖 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/root.zig`:
- Around line 975-977: In the combined_embeds update, call
scripting_splice.concatEmbeds and store its successful result before freeing the
existing combined_embeds slice. Then free the old slice and assign the new
result, ensuring combined_embeds never retains a freed pointer while
concatEmbeds can fail; preserve the existing s.scripts assignment.
---
Outside diff comments:
In `@src/language_policy.zig`:
- Around line 529-532: Update the shared-directory matching logic around matches
to exclude entries ending in “.d.ts” before applying scriptExtensions(lang),
while preserving normal TypeScript and other runnable-script classification.
🪄 Autofix (Beta)
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: 04f3127e-089b-49f0-82fd-f64ed86d68ad
📒 Files selected for processing (10)
src/codegen/lifecycle/callback.zigsrc/codegen/lifecycle/loop.zigsrc/language_policy.zigsrc/root.zigsrc/scripting_declare.zigsrc/scripting_splice.zigsrc/scripting_transpile.zigtest/scripting_declare_tests.zigtest/scripting_splice_tests.zigtest/scripting_transpile_tests.zig
…finding) Freeing the old slice before the try left combined_embeds dangling across concatEmbeds — an OOM there would double-free via the deferred cleanup. The new slice is computed first, then the old freed, then assigned. Claude-Session: https://claude.ai/code/session_01P7YLw4hXFCCaY2LAUt4G1j
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e3c7173fd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (s.family == .embed) { | ||
| scripting_scripts = try scripting_splice.collectEmbedScripts(allocator, game_dir, target_dir, s.*); | ||
| s.scripts = scripting_scripts.?; | ||
| component_embeds = try scripting_splice.collectComponentEmbeds(allocator, game_dir, s.*); |
There was a problem hiding this comment.
Reject component-dir files for native script languages
For native scripting projects such as rust or crystal, this branch skips collectComponentEmbeds, while the new language-policy scan now treats components/*.rs/*.cr as the declared language and lets them pass. The native path later stages only splice.dir (scripts/ or the legacy language dir) via stageNativeSources, so a file like components/hunger.rs succeeds generation but is neither scanned as a Zig component nor staged/registered as scripting code. Please either reject components/*.<native ext> for native rows or include them in the native staging path.
Useful? React with 👍 / 👎.
The final convergence piece before v0.86.0 — two changes closing the loop on both user decisions from labelle-toolkit/labelle-engine#237:
1. Ruby declare-mode wires into generate
DECLARE_LANGUAGE→ theDeclareRunnertable (lua →labelle-declare/.lua, ruby →labelle-declare-ruby/.rb, from labelle-scripting v0.9.0), plus amin_pinper row so the absent-tool note stays pointed. Per-row filesystem capability probe — older pins gracefully skip exactly as before; the in-process tool cache keys pkg+step (a lua build can never satisfy a ruby lookup).parseSchemauntouched — the scripting repo's cross-runner byte-parity golden is the contract proof.2.
components/hunger.rbnext tocomponents/worker.zigDeclare collection =
components/*.<ext>union in-script chunk-scope declarations (both feed the runner, components first; the runner owns duplicate attribution with first-declared-in). Components-dir language files are also embedded and registered before scripts — the view constants exist when scripts load (the enabler:EmbedScript.filewent target-relative so two convention dirs feed one emission stream; generated bytes unchanged, goldens untouched-passing). Recursive organizational subdirs mirror the Zig component scan; a.tsauthoring source incomponents/is a pointed error (.d.tsexempt); the legacy-grace path gets components/ declarations too; the policy scan polices both dirs identically.Verification
+17 tests: the union argv pin (recording fake runner), the ruby declare e2e (
components/hunger.rb→ generated component + registry +registerScriptbeforescripts/), old-pin graceful skip, transpile-swap prefix preservation, per-row probes with both-row negative controls. Negative-verified: inverted ordering and stubbed components collection each fail exactly the two ordering pins. 1968/1972 clean-HOME, authoritative exit 0.After this releases (v0.86.0): the scripting examples PR —
components/hunger.zig→components/hunger.rbin the ruby game,feed_watcher.rb, all examples toscripts/.https://claude.ai/code/session_01P7YLw4hXFCCaY2LAUt4G1j
Summary by CodeRabbit
New Features
components/andscripts/, with deterministic registration order.Bug Fixes
Tests