Conversation
- Add moq-robo: Rust robot publisher with ffmpeg-next video pipeline, fake sensor telemetry, and viewer command handling - Add web viewer: TypeScript app with robot discovery grid, video player with sensor HUD overlay, and multi-viewer control panel - Move js/demo to demo/web, new robot demo at demo/robot - Both Rust and TypeScript source coexist in demo/robot/src/ - Update all doc/config references to new demo/web path - Remove --silent from justfile bun commands Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add moq_mux::import::Avc1 for AVCC-formatted H.264 (avc1 style with out-of-band description). Parses SPS for width/height, auto-detects keyframes from NAL types, mirrors the Avc3 API (initialize/decode). - Robot publisher produces two renditions: HD via Avc1 transmux and 240p preview via Avc3 transcode with burned-in "240p" label. - JS viewer uses Watch API directly with pixel budget switching: 240p for thumbnails, HD when expanded. Non-expanded cards disabled to save bandwidth. - Rename robot → robo everywhere (directory, module, struct, paths). - Restructure justfiles: nested modules (dev::robo, dev::web) with default recipes, just dev::robo::add to spawn extra publishers. - Fix announce flapping: separate publish/consume origins. - Add controller alert overlay (yellow/red based on count). - Random robot ID if --id not provided. - Real-time pacing for video pipeline. - Update all docs for just dev → just, dev/ path restructure. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace angle-switching with an action-based state machine: - idle/dead states loop their video files - 5 named actions (cup, wrench, ball, cone, box) play once then return to idle - Actions queue when one is already playing; kill interrupts immediately - Status track advertises available actions, current state, and queued action - Viewer dynamically renders action buttons with active/queued highlighting - SD preview matches source aspect ratio instead of hardcoded 16:9 - CLI uses --media-dir with convention-based file discovery Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace ffmpeg demuxer with mp4_atom for fMP4 parsing. All media files are preloaded into RAM at startup and parsed into H.264 samples. The playback loop is fully async with tokio::time::sleep for pacing. The 240p transcoder (ffmpeg decode/scale/encode) runs on a dedicated std::thread, communicating via tokio::sync::mpsc channels. No unsafe Send impls or block_in_place needed — ffmpeg types never cross thread boundaries. Also adds anyhow::Context convention to CLAUDE.md. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace the robo demo's fMP4 video playback with a real-time rendered drone game using tiny-skia (2D rendering) and Rapier2D (physics). The game features a drone on a 5x5 grid that can move, grab/drop a ball via a spring-physics wire, and auto-dock when battery runs low. Video is encoded to H.264 at two renditions (720p + 360p) with resolution labels. Also renames robo→drone throughout, extracts publishing recipes into dev/pub module, and adds flat justfile shortcuts (just drone, just pub). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (3)
WalkthroughThe PR reorganizes demos from 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
doc/setup/dev.md (1)
17-21:⚠️ Potential issue | 🟡 MinorDocumentation inconsistency:
justno longer lists commands.Line 18 shows
justunder the comment "List all available commands", but with the new default target, runningjustwill start the demo instead. To list commands, usejust --listorjust -l.📝 Suggested fix
# List all available commands -just +just --list # Run the demo just🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@doc/setup/dev.md` around lines 17 - 21, The documentation incorrectly states that running "just" will list all commands; update the text in doc/setup/dev.md to reflect the new default target by replacing the first "just" example with the correct command to list tasks ("just --list" or "just -l") and keep the second "just" example as the command to run the demo so readers know "just" now starts the demo while "just --list" / "just -l" shows available commands.
🧹 Nitpick comments (11)
dev/drone/justfile (1)
7-10: Consider removing redundant sleep calls.The
sleep 1andsleep 2beforejust waitappear redundant sincewaitalready polls repeatedly. This pattern differs fromdev/justfilewhich uses onlywait. Unless there's a specific reason for the staggered delays, simplifying to justjust waitwould be more consistent.♻️ Suggested simplification
bun run concurrently --kill-others --prefix-colors auto \ "just relay" \ - "sleep 1 && just wait && just start {{count}}" \ - "sleep 2 && just wait && just web" + "just wait && just start {{count}}" \ + "just wait && just web"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dev/drone/justfile` around lines 7 - 10, Remove the redundant leading sleep invocations in the bun run concurrently command: replace the "sleep 1 && just wait" and "sleep 2 && just wait" entries with simply "just wait" so all concurrent tasks use the same polling wait behavior (leave "just relay", "just wait", "just start {{count}}", "just web" as the commands run by concurrently). This aligns with the dev/justfile pattern and avoids unnecessary staggered delays introduced by the "sleep 1" and "sleep 2" prefixes.dev/drone/src/index.html (1)
172-174: Consider moving inline styles to the style block.The footer has extensive inline styles while the rest of the document uses a dedicated
<style>block. For consistency and maintainability, consider moving these styles to the existing CSS section.♻️ Suggested refactor
Add to the
<style>block:footer { position: fixed; bottom: 0; left: 0; right: 0; padding: 0.4rem 1rem; background: `#111`; border-top: 1px solid `#222`; font-family: monospace; font-size: 0.65rem; color: `#555`; text-align: center; } footer code { color: `#888`; }Then simplify the HTML:
-<footer style="position:fixed;bottom:0;left:0;right:0;padding:0.4rem 1rem;background:`#111`;border-top:1px solid `#222`;font-family:monospace;font-size:0.65rem;color:`#555`;text-align:center;"> - Run <code style="color:`#888`">just dev::drone::start <em>N</em></code> to add more drones +<footer> + Run <code>just dev::drone::start <em>N</em></code> to add more drones </footer>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dev/drone/src/index.html` around lines 172 - 174, Move the inline styling from the <footer> element into the document's existing <style> block: create CSS rules targeting footer (and footer code for the code color) with the position, bottom/left/right, padding, background, border-top, font-family, font-size, color, and text-align properties, then remove the style attribute from the <footer> element in the HTML so the footer markup uses the new CSS rules instead.dev/drone/package.json (1)
1-28: LGTM overall. The package configuration is well-structured with appropriate workspace dependencies and dev tooling. The comment syntax using"//"key is a pragmatic workaround for JSON's lack of comments.Note: Vite 8.0.3 is now available (latest as of March 2026), but the current constraint
^7.3.1is valid. Consider upgrading to Vite 8 if there are no compatibility concerns with other workspace packages.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dev/drone/package.json` around lines 1 - 28, Update the Vite dependency in devDependencies from "vite": "^7.3.1" to the desired newer version (e.g., "^8.0.3") and then run local builds/tests (the "dev" script and "build" script referencing vite) to verify compatibility with the workspace packages (`@moq/hang`, `@moq/lite`, `@moq/watch`, `@moq/signals`); if any incompatibilities appear, either pin a compatible Vite version or revert to "^7.3.1" and document the decision.dev/drone/src/index.ts (2)
179-183: Extract pixel budget constants.The pixel budget values are magic numbers. Consider extracting them as named constants for clarity.
📝 Proposed fix
+const PIXEL_BUDGET_EXPANDED = 1920 * 1080; +const PIXEL_BUDGET_THUMBNAIL = 478 * 360; + // ... in the effect: - const pixels = exp === droneId ? 1920 * 1080 : 478 * 360; + const pixels = exp === droneId ? PIXEL_BUDGET_EXPANDED : PIXEL_BUDGET_THUMBNAIL;As per coding guidelines: "Avoid using magic numbers; use named constants instead"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dev/drone/src/index.ts` around lines 179 - 183, The code uses magic numbers for pixel budgets inside the this.#signals.run callback; extract 1920 * 1080 and 478 * 360 into named constants (e.g., FULL_HD_PIXELS and THUMBNAIL_PIXELS) and replace the inline expressions in the branch that computes pixels (where effect.get(expanded) is compared to droneId and videoSource.target.set({ pixels }) is called) so the intent is clear and maintainable.
77-299: Consider splitting DroneCard constructor.The
DroneCardconstructor handles video setup, sensor subscription, status tracking, controller alerts, and command publishing—all in ~220 lines. Consider extracting logical sections into private helper methods for improved readability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dev/drone/src/index.ts` around lines 77 - 299, The DroneCard constructor is too large and should be split into focused private helpers; extract logical sections (video setup, sensor subscription, status tracking, controller alert logic, keyboard handling, and command publishing) from the constructor into clearly named private methods such as setupVideo(video-related args), setupSensorHud(broadcast,hud), setupStatusTracking(broadcast,status,controls), setupAlertUpdater(status,alert), setupKeyboardHandlers(droneId) and setupCommandPublishing(connection, expanded, droneId) and call those from the constructor; ensure each helper registers/returns any created resources or accepts this.#signals to register cleanup (and moves the keyHandler registration/removal into setupKeyboardHandlers) so cleanup remains centralized via this.#signals.cleanup and existing symbols (DroneCard constructor, this.#signals.run, videoSource, videoDecoder, broadcast, status, commandTrack) can be referenced to relocate logic without changing behavior.doc/setup/demo/drone.md (1)
51-62: Add language specification to fenced code block.The fenced code block should have a language specified to avoid lint warnings. For text-based diagrams, use
textorplaintext.📝 Proposed fix
-``` +```text drone/ {id}/ ← drone broadcast🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@doc/setup/demo/drone.md` around lines 51 - 62, The fenced code block in doc/setup/demo/drone.md lacks a language spec which triggers lint warnings; update the opening fence to specify a plain-text language (e.g., use ```text or ```plaintext) for the block that starts with "drone/" so the diagram is treated as text and the linter stops complaining.dev/drone/src/main.rs (1)
77-80:std::process::exit(0)bypasses async cleanup.Using
std::process::exit(0)on Ctrl-C terminates immediately without running destructors or awaiting pending futures. For a demo this is acceptable, but be aware that graceful shutdown (e.g., closing MoQ sessions cleanly) won't occur.Also applies to: 93-100
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dev/drone/src/main.rs` around lines 77 - 80, The current tokio::signal::ctrl_c() arm calls std::process::exit(0) which aborts async cleanup; change this to trigger a graceful shutdown instead: have main create a shutdown mechanism (e.g., a CancellationToken or a tokio::sync::oneshot/broadcast channel) that you pass into run_one (and any other components like MoQ sessions) and, in the ctrl_c arm, send/trigger that shutdown signal or simply return so the program can await run_one's completion and let destructors run; apply the same change to the other ctrl_c usage noted (the block that spans the later occurrence).dev/drone/src/drone.rs (1)
115-118: Consider handling poisoned mutex gracefully.
lock().unwrap()will panic if another thread panicked while holding the lock. While unlikely in this demo, usinglock().ok()orlock().expect("...")with a clear message would be more defensive.🛡️ Defensive alternative
let json = { - let state = inner.state.lock().unwrap(); + let Ok(state) = inner.state.lock() else { + // Mutex poisoned, skip this tick + tokio::time::sleep(std::time::Duration::from_millis(100)).await; + continue; + }; serde_json::to_string(&*state)? };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dev/drone/src/drone.rs` around lines 115 - 118, The current code calls inner.state.lock().unwrap() which will panic on a poisoned mutex; change this to handle the PoisonError and return a proper error instead of panicking (e.g. match inner.state.lock() or use inner.state.lock().map_err(|e| /* convert to your function's error with a clear message */)? ), then pass the guarded state to serde_json::to_string(&*state)? as before; reference inner.state.lock() and serde_json::to_string to locate the spot and ensure you propagate or convert the poisoned-lock error with a clear message rather than using unwrap().dev/drone/src/video.rs (2)
100-106: Thread join in Drop may cause shutdown delays.The
Dropimplementation joins the encoder thread synchronously. If the encoder thread is blocked (e.g., onblocking_recvwaiting for frames), this could delay cleanup. Consider closing the channel sender first or using a timeout.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dev/drone/src/video.rs` around lines 100 - 106, The Drop impl for EncoderHandle currently does a synchronous join on self.thread which can block shutdown if the encoder thread is waiting; modify drop to first close the frame sender to unblock the encoder (drop or take the sender field, e.g., EncoderHandle.sender) and only then join the thread, or avoid blocking join by detaching the thread instead (do not call JoinHandle::join in EncoderHandle::drop) or implement a timed/shutdown handshake before calling JoinHandle::join; ensure you reference EncoderHandle, its drop method, the thread field (self.thread) and the channel sender field when making the change.
154-159: Consider avoiding frame clone for HD encoding.
yuv_hd.clone()duplicates the YUV frame buffer before burning the label. Sinceburn_labelmodifies the frame in-place, you could reorder operations to burn the HD label, encode HD, then downscale from the original (unlabeled) frame—avoiding the clone entirely.♻️ Proposed optimization
- // Burn "720p" label and encode HD. - let mut hd_frame = yuv_hd.clone(); - burn_label(&mut hd_frame, "720p"); - if let Err(e) = hd_encoder.encode_yuv(&hd_frame, ts, &mut hd) { + // Downscale first (before burning label on HD). + let mut sd_frame = ffmpeg_next::frame::Video::empty(); + if let Err(e) = downscaler.run(&yuv_hd, &mut sd_frame) { + tracing::error!(error = %e, "downscale error"); + continue; + } + + // Now burn label on HD and encode. + burn_label(&mut yuv_hd, "720p"); + if let Err(e) = hd_encoder.encode_yuv(&yuv_hd, ts, &mut hd) { tracing::error!(error = %e, "HD encode error"); } - // Downscale to 360p, burn label, encode SD. - let mut sd_frame = ffmpeg_next::frame::Video::empty(); - if let Err(e) = downscaler.run(&yuv_hd, &mut sd_frame) { - tracing::error!(error = %e, "downscale error"); - continue; - } + // Burn label on SD and encode. burn_label(&mut sd_frame, "360p");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dev/drone/src/video.rs` around lines 154 - 159, Avoid cloning yuv_hd: instead of let mut hd_frame = yuv_hd.clone(), call burn_label on the HD buffer directly (burn_label(&mut yuv_hd, "720p")) then encode with hd_encoder.encode_yuv(&yuv_hd, ts, &mut hd); create the low-res buffer by downscaling from the original unlabeled source frame (the pre-burn variable you already have) and burn the low-res label there—this removes the yuv_hd.clone() and keeps burn_label and encode_yuv usage on the appropriate buffers (refer to burn_label, yuv_hd, hd_encoder, encode_yuv, ts, and hd).dev/drone/src/game.rs (1)
33-54: Use one source of truth for action names.
dev/drone/src/drone.rs:197validates commands againstaction_names(), whiledev/drone/src/video.rs:41parses them withGameAction::from_str(). Keeping two hard-coded lists here makes it easy for a future action to be accepted upstream but silently dropped in the game loop.♻️ Suggested refactor
+const ACTIONS: &[(&str, GameAction)] = &[ + ("left", GameAction::Left), + ("right", GameAction::Right), + ("up", GameAction::Up), + ("down", GameAction::Down), + ("grab", GameAction::Grab), + ("drop", GameAction::Drop), + ("dock", GameAction::Dock), +]; + impl GameAction { pub fn from_str(s: &str) -> Option<Self> { - match s { - "left" => Some(Self::Left), - "right" => Some(Self::Right), - "up" => Some(Self::Up), - "down" => Some(Self::Down), - "grab" => Some(Self::Grab), - "drop" => Some(Self::Drop), - "dock" => Some(Self::Dock), - _ => None, - } + ACTIONS + .iter() + .find_map(|(name, action)| (*name == s).then_some(action.clone())) } } @@ pub fn action_names() -> Vec<String> { - ["left", "right", "up", "down", "grab", "drop", "dock"] - .iter() - .map(|s| s.to_string()) - .collect() + ACTIONS.iter().map(|(name, _)| (*name).to_string()).collect() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dev/drone/src/game.rs` around lines 33 - 54, The two hard-coded lists of action strings are duplicated between GameAction::from_str and action_names; consolidate to a single source of truth by deriving the list from the GameAction enum (or a single const array used by both). Update GameAction::from_str to parse by consulting that single list (or implement a matching helper built from the enum variants), and change action_names() to generate its Vec<String> from the same source (e.g., a const ACTION_STRS or an iterator over GameAction variants), ensuring both parsing and validation use the identical set of action names (referencing GameAction::from_str and action_names in your changes).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@dev/drone/Cargo.toml`:
- Line 21: Update the dependency pins in Cargo.toml so they explicitly reference
the latest stable patch versions: change ffmpeg-next from "8" to "8.1" and
add/replace the rapier2d and tiny-skia entries to pin them to "0.32" and "0.12"
respectively; ensure the dependency keys are exactly ffmpeg-next, rapier2d, and
tiny-skia so Cargo uses the specified versions.
In `@dev/drone/justfile`:
- Around line 17-18: The VITE_RELAY_URL environment assignment in the "web"
justfile target uses an unquoted {{url}} which is unsafe for shell parsing;
update the command in the web target so VITE_RELAY_URL is quoted (e.g.,
VITE_RELAY_URL="{{url}}") to ensure any special characters in {{url}} (like & or
spaces) are handled correctly—modify the "web" target where VITE_RELAY_URL is
set and leave the existing url='http://localhost:4443/anon' declaration intact.
In `@dev/drone/src/game.rs`:
- Around line 267-295: start_dock can leave a stale self.target when the drone
is already in the dock tile but off-center because self.dock_path stays empty;
update start_dock so that if self.dock_path.first() is None you explicitly
retarget to the dock center (e.g. set self.target = Some((self.dock.0 as f32,
self.dock.1 as f32)) or to (gx, gy)) or clear the target as appropriate,
ensuring self.docking remains true; modify the block that currently sets
self.target only when first exists to handle the empty-path case using the
identifiers start_dock, self.dock_path, self.target, gx/gy and self.dock.
- Around line 417-424: reset_to_dock currently only calls do_drop() when
self.carrying is true, so if a reset happens while self.grabbing is still true
the spring joint/wire remains attached; modify reset_to_dock to ensure the
tether is detached in mid-grab by invoking the drop/detach logic whenever
grabbing is true (e.g., check self.grabbing and call self.do_drop() or
unconditionally call self.do_drop() before clearing dock state), and also
clear/reset any related fields like self.grabbing and the spring/joint state so
the ball is no longer tethered.
In `@dev/drone/src/index.html`:
- Around line 80-98: The D-pad button elements styled by .dpad-btn / .dpad-up /
.dpad-left / .dpad-right / .dpad-down lack accessible labels; update the code
that creates or renders these buttons to add descriptive aria-label attributes
(e.g., "Up", "Left", "Right", "Down") or aria-labelledby references so screen
readers can identify each control, and ensure any dynamic creation logic sets
the aria-label on the element before inserting into the DOM.
In `@dev/pub/justfile`:
- Around line 95-106: The publish invocation is using a non-existent flag
"--format annex-b" instead of the publish subcommand; update the pipeline that
runs cargo run --bin moq-cli -- publish to replace the flag style with the avc3
subcommand so the command becomes: call the publish subcommand with avc3 (i.e.,
use "publish ... --url/--name ... avc3 {{args}}") — locate the publish
invocation in the h264 recipe that pipes ffmpeg into cargo run and swap the
"--format annex-b" token for the "avc3" subcommand.
In `@dev/web/justfile`:
- Around line 3-5: Fix the typo in the top comment: change "targetting" to
"targeting" in the comment above the default justfile target so the line reads
"Run the web server targeting the specified relay"; the change is near the
comment that describes the default target which uses the variable url and target
name default.
In `@doc/setup/demo/web.md`:
- Line 13: Replace the namespace-qualified just invocation with the top-level
alias: change the command string "just dev::web" to "just web" so it matches the
root justfile module alias (mod web 'dev/web') and the rest of the
documentation; update the occurrence in the demo web doc where the command is
shown.
In `@rs/moq-mux/src/import/avc1.rs`:
- Around line 143-170: The is_keyframe method currently handles length_size
values 1, 2, and 4 but returns false for length_size == 3; update the match in
is_keyframe to support a 3-byte big-endian length (compute nal_len from
data[offset..offset+3] as a 24-bit BE integer) so NAL unit parsing works for
length_size == 3, keeping the same bounds checks around offset and nal_len and
preserving the existing logic that checks nal_type == 5 for IDR slices.
In `@rs/moq-relay/README.md`:
- Line 55: Clarify that the example key path "key = \"dev/relay/root.jwk\"" is
interpreted relative to the process working directory; update the README.md text
near that snippet to explicitly state that relative paths are resolved from
where moq-relay is launched (or recommend using an absolute path or environment
variable for determinism), and show a brief note or example demonstrating an
absolute path alternative.
---
Outside diff comments:
In `@doc/setup/dev.md`:
- Around line 17-21: The documentation incorrectly states that running "just"
will list all commands; update the text in doc/setup/dev.md to reflect the new
default target by replacing the first "just" example with the correct command to
list tasks ("just --list" or "just -l") and keep the second "just" example as
the command to run the demo so readers know "just" now starts the demo while
"just --list" / "just -l" shows available commands.
---
Nitpick comments:
In `@dev/drone/justfile`:
- Around line 7-10: Remove the redundant leading sleep invocations in the bun
run concurrently command: replace the "sleep 1 && just wait" and "sleep 2 &&
just wait" entries with simply "just wait" so all concurrent tasks use the same
polling wait behavior (leave "just relay", "just wait", "just start {{count}}",
"just web" as the commands run by concurrently). This aligns with the
dev/justfile pattern and avoids unnecessary staggered delays introduced by the
"sleep 1" and "sleep 2" prefixes.
In `@dev/drone/package.json`:
- Around line 1-28: Update the Vite dependency in devDependencies from "vite":
"^7.3.1" to the desired newer version (e.g., "^8.0.3") and then run local
builds/tests (the "dev" script and "build" script referencing vite) to verify
compatibility with the workspace packages (`@moq/hang`, `@moq/lite`, `@moq/watch`,
`@moq/signals`); if any incompatibilities appear, either pin a compatible Vite
version or revert to "^7.3.1" and document the decision.
In `@dev/drone/src/drone.rs`:
- Around line 115-118: The current code calls inner.state.lock().unwrap() which
will panic on a poisoned mutex; change this to handle the PoisonError and return
a proper error instead of panicking (e.g. match inner.state.lock() or use
inner.state.lock().map_err(|e| /* convert to your function's error with a clear
message */)? ), then pass the guarded state to serde_json::to_string(&*state)?
as before; reference inner.state.lock() and serde_json::to_string to locate the
spot and ensure you propagate or convert the poisoned-lock error with a clear
message rather than using unwrap().
In `@dev/drone/src/game.rs`:
- Around line 33-54: The two hard-coded lists of action strings are duplicated
between GameAction::from_str and action_names; consolidate to a single source of
truth by deriving the list from the GameAction enum (or a single const array
used by both). Update GameAction::from_str to parse by consulting that single
list (or implement a matching helper built from the enum variants), and change
action_names() to generate its Vec<String> from the same source (e.g., a const
ACTION_STRS or an iterator over GameAction variants), ensuring both parsing and
validation use the identical set of action names (referencing
GameAction::from_str and action_names in your changes).
In `@dev/drone/src/index.html`:
- Around line 172-174: Move the inline styling from the <footer> element into
the document's existing <style> block: create CSS rules targeting footer (and
footer code for the code color) with the position, bottom/left/right, padding,
background, border-top, font-family, font-size, color, and text-align
properties, then remove the style attribute from the <footer> element in the
HTML so the footer markup uses the new CSS rules instead.
In `@dev/drone/src/index.ts`:
- Around line 179-183: The code uses magic numbers for pixel budgets inside the
this.#signals.run callback; extract 1920 * 1080 and 478 * 360 into named
constants (e.g., FULL_HD_PIXELS and THUMBNAIL_PIXELS) and replace the inline
expressions in the branch that computes pixels (where effect.get(expanded) is
compared to droneId and videoSource.target.set({ pixels }) is called) so the
intent is clear and maintainable.
- Around line 77-299: The DroneCard constructor is too large and should be split
into focused private helpers; extract logical sections (video setup, sensor
subscription, status tracking, controller alert logic, keyboard handling, and
command publishing) from the constructor into clearly named private methods such
as setupVideo(video-related args), setupSensorHud(broadcast,hud),
setupStatusTracking(broadcast,status,controls), setupAlertUpdater(status,alert),
setupKeyboardHandlers(droneId) and setupCommandPublishing(connection, expanded,
droneId) and call those from the constructor; ensure each helper
registers/returns any created resources or accepts this.#signals to register
cleanup (and moves the keyHandler registration/removal into
setupKeyboardHandlers) so cleanup remains centralized via this.#signals.cleanup
and existing symbols (DroneCard constructor, this.#signals.run, videoSource,
videoDecoder, broadcast, status, commandTrack) can be referenced to relocate
logic without changing behavior.
In `@dev/drone/src/main.rs`:
- Around line 77-80: The current tokio::signal::ctrl_c() arm calls
std::process::exit(0) which aborts async cleanup; change this to trigger a
graceful shutdown instead: have main create a shutdown mechanism (e.g., a
CancellationToken or a tokio::sync::oneshot/broadcast channel) that you pass
into run_one (and any other components like MoQ sessions) and, in the ctrl_c
arm, send/trigger that shutdown signal or simply return so the program can await
run_one's completion and let destructors run; apply the same change to the other
ctrl_c usage noted (the block that spans the later occurrence).
In `@dev/drone/src/video.rs`:
- Around line 100-106: The Drop impl for EncoderHandle currently does a
synchronous join on self.thread which can block shutdown if the encoder thread
is waiting; modify drop to first close the frame sender to unblock the encoder
(drop or take the sender field, e.g., EncoderHandle.sender) and only then join
the thread, or avoid blocking join by detaching the thread instead (do not call
JoinHandle::join in EncoderHandle::drop) or implement a timed/shutdown handshake
before calling JoinHandle::join; ensure you reference EncoderHandle, its drop
method, the thread field (self.thread) and the channel sender field when making
the change.
- Around line 154-159: Avoid cloning yuv_hd: instead of let mut hd_frame =
yuv_hd.clone(), call burn_label on the HD buffer directly (burn_label(&mut
yuv_hd, "720p")) then encode with hd_encoder.encode_yuv(&yuv_hd, ts, &mut hd);
create the low-res buffer by downscaling from the original unlabeled source
frame (the pre-burn variable you already have) and burn the low-res label
there—this removes the yuv_hd.clone() and keeps burn_label and encode_yuv usage
on the appropriate buffers (refer to burn_label, yuv_hd, hd_encoder, encode_yuv,
ts, and hd).
In `@doc/setup/demo/drone.md`:
- Around line 51-62: The fenced code block in doc/setup/demo/drone.md lacks a
language spec which triggers lint warnings; update the opening fence to specify
a plain-text language (e.g., use ```text or ```plaintext) for the block that
starts with "drone/" so the diagram is treated as text and the linter stops
complaining.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 78d35ac1-2f6f-4bd0-9f49-93abcf1eb4e3
⛔ Files ignored due to path filters (3)
Cargo.lockis excluded by!**/*.lockbun.lockis excluded by!**/*.lockdev/web/src/favicon.svgis excluded by!**/*.svg
📒 Files selected for processing (58)
CLAUDE.mdCargo.tomlREADME.mddev/drone/Cargo.tomldev/drone/justfiledev/drone/package.jsondev/drone/src/drone.rsdev/drone/src/game.rsdev/drone/src/index.htmldev/drone/src/index.tsdev/drone/src/main.rsdev/drone/src/sensor.rsdev/drone/src/video.rsdev/drone/tsconfig.jsondev/drone/vite.config.tsdev/justfiledev/pub/justfiledev/pub/media/.gitignoredev/relay/justfiledev/relay/leaf0.tomldev/relay/leaf1.tomldev/relay/localhost.tomldev/relay/prod.tomldev/relay/root.tomldev/throttle/enabledev/web/.envdev/web/README.mddev/web/justfiledev/web/package.jsondev/web/src/discover.tsdev/web/src/highlight.tsdev/web/src/index.cssdev/web/src/index.htmldev/web/src/index.tsdev/web/src/mse.htmldev/web/src/publish.htmldev/web/src/publish.tsdev/web/src/vite-env.d.tsdev/web/tailwind.config.jsdev/web/tsconfig.jsondev/web/vite.config.tsdoc/.vitepress/config.tsdoc/app/web.mddoc/concept/standard/interop.mddoc/index.mddoc/js/@moq/lite.mddoc/js/index.mddoc/setup/demo/drone.mddoc/setup/demo/web.mddoc/setup/dev.mddoc/setup/index.mdjs/demo/tsconfig.jsonjustfilepackage.jsonrs/moq-mux/src/import/avc1.rsrs/moq-mux/src/import/decoder.rsrs/moq-mux/src/import/mod.rsrs/moq-relay/README.md
💤 Files with no reviewable changes (1)
- js/demo/tsconfig.json
| anyhow = { version = "1", features = ["backtrace"] } | ||
| bytes = "1" | ||
| clap = { version = "4", features = ["derive"] } | ||
| ffmpeg-next = "8" |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What are the latest versions of the Rust crates ffmpeg-next, rapier2d, and tiny-skia on crates.io?
💡 Result:
The latest versions on crates.io as of 2026-03-28 are: - ffmpeg-next: 8.1.0 (published 2026-03-18) - rapier2d: 0.32.0 (published 2026-01-09) - tiny-skia: 0.12.0 (published 2026-02-02)
Citations:
- 1: https://crates.io/crates/ffmpeg-next
- 2: https://crates.io/crates/rapier2d
- 3: https://crates.io/crates/tiny-skia
🏁 Script executed:
sed -n '20,35p' dev/drone/Cargo.toml | cat -nRepository: moq-dev/moq
Length of output: 586
🏁 Script executed:
find . -name "CLAUDE.md" -o -name "claude.md" | head -5Repository: moq-dev/moq
Length of output: 66
🏁 Script executed:
cat ./CLAUDE.mdRepository: moq-dev/moq
Length of output: 6332
Update dependency versions to explicitly pin the latest stable releases.
The current semver ranges permit the latest versions, but per CLAUDE.md guidelines to "use the newest stable version" for new dependencies, these should be pinned to the actual latest available:
ffmpeg-next = "8.1"(currently 8.1.0)rapier2d = "0.32"(currently 0.32.0)tiny-skia = "0.12"(currently 0.12.0)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@dev/drone/Cargo.toml` at line 21, Update the dependency pins in Cargo.toml so
they explicitly reference the latest stable patch versions: change ffmpeg-next
from "8" to "8.1" and add/replace the rapier2d and tiny-skia entries to pin them
to "0.32" and "0.12" respectively; ensure the dependency keys are exactly
ffmpeg-next, rapier2d, and tiny-skia so Cargo uses the specified versions.
Comment on lines
+80
to
+98
| /* D-pad: 3x3 CSS grid */ | ||
| .dpad { | ||
| display: grid; | ||
| grid-template-columns: 56px 56px 56px; | ||
| grid-template-rows: 56px 56px 56px; | ||
| gap: 4px; | ||
| } | ||
| .dpad-btn { | ||
| background: #222; color: #e0e0e0; border: 1px solid #555; | ||
| border-radius: 6px; cursor: pointer; font-size: 1.2rem; | ||
| display: flex; align-items: center; justify-content: center; | ||
| transition: all 0.15s; | ||
| } | ||
| .dpad-btn:hover { background: #333; border-color: #4ade80; } | ||
| .dpad-btn:active { background: #444; transform: scale(0.95); } | ||
| .dpad-up { grid-column: 2; grid-row: 1; } | ||
| .dpad-left { grid-column: 1; grid-row: 2; } | ||
| .dpad-right { grid-column: 3; grid-row: 2; } | ||
| .dpad-down { grid-column: 2; grid-row: 3; } |
Contributor
There was a problem hiding this comment.
Add accessible labels to D-pad buttons.
The D-pad buttons lack accessible labels, making them unusable for screen reader users. Consider adding aria-label attributes when these buttons are created in JavaScript, or document that accessibility labels should be added to the button elements.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@dev/drone/src/index.html` around lines 80 - 98, The D-pad button elements
styled by .dpad-btn / .dpad-up / .dpad-left / .dpad-right / .dpad-down lack
accessible labels; update the code that creates or renders these buttons to add
descriptive aria-label attributes (e.g., "Up", "Left", "Right", "Down") or
aria-labelledby references so screen readers can identify each control, and
ensure any dynamic creation logic sets the aria-label on the element before
inserting into the DOM.
The toml files and justfile recipes referenced relay/ prefixed paths which were correct when running from dev/ but wrong now that the working directory is dev/relay/ itself. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Fix start_dock empty path: target dock center when already on dock tile - Fix reset_to_dock: also detach wire when grabbing (not just carrying) - Fix pub h264 recipe: --format annex-b → avc3 subcommand - Fix avc1 is_keyframe: add length_size == 3 support - Quote VITE_RELAY_URL in drone justfile for shell safety - Add aria-labels to d-pad buttons - Fix web.md: just dev::web → just web - Fix dev.md: just now runs demo, just --list lists commands - Fix drone.md: add language spec to fenced code block - Fix web justfile typo: targetting → targeting - Clarify relay README key path is relative to working directory Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
dev/drone/src/video.rs (3)
35-43: Consider handling channel disconnection explicitly.The
try_recv()loop treats bothEmptyandDisconnectedas non-fatal. If the command sender is dropped, this loop will silently continue without commands. For a demo this is acceptable, but you may want to log or handle disconnection if commands are expected to remain available.Optional: Handle disconnection explicitly
// Drain all pending commands (non-blocking). - while let Ok(name) = cmd_rx.try_recv() { - if let Some(action) = game::GameAction::from_str(&name) { - game.apply_action(action); + loop { + match cmd_rx.try_recv() { + Ok(name) => { + if let Some(action) = game::GameAction::from_str(&name) { + game.apply_action(action); + } + } + Err(tokio::sync::mpsc::error::TryRecvError::Empty) => break, + Err(tokio::sync::mpsc::error::TryRecvError::Disconnected) => { + tracing::debug!("command channel disconnected"); + break; + } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dev/drone/src/video.rs` around lines 35 - 43, The loop draining commands currently ignores a Disconnected result from cmd_rx.try_recv(), so if the sender is dropped the loop will silently spin; update the try_recv handling on cmd_rx to match on Err(e) and explicitly detect and handle TryRecvError::Disconnected (e.g., log a warning via your logger or break/stop the loop) while preserving the current behavior for TryRecvError::Empty, and keep the existing flow that converts names with game::GameAction::from_str and calls game.apply_action(action).
100-106: Thread panic information is silently discarded.If the encoder thread panics,
thread.join()returnsErrcontaining the panic payload, but this is ignored. For debugging, consider logging the panic.Optional: Log thread panic on drop
impl Drop for EncoderHandle { fn drop(&mut self) { if let Some(thread) = self.thread.take() { - let _ = thread.join(); + if let Err(panic_payload) = thread.join() { + tracing::error!("encoder thread panicked: {:?}", panic_payload); + } } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dev/drone/src/video.rs` around lines 100 - 106, In the Drop implementation for EncoderHandle, do not ignore the Result from thread.join(); instead match on the join outcome and log any panic payload. In the impl Drop for EncoderHandle (fn drop), replace the unused let _ = thread.join() with code that matches thread.join() -> Ok(_) => (), Err(payload) => { extract payload as &str or String via downcast_ref::<&str>() / downcast_ref::<String>() and log it (e.g. log::error! or eprintln!) with contextual text like "encoder thread panicked" }. Ensure you reference EncoderHandle, the Drop::drop method, and the call to thread.join() when making the change.
180-186: Frame cloning is necessary but consider documenting the reason.The
yuv_hd.clone()is required because the original frame is needed for downscaling while a modified copy is used for HD encoding. This is correct but a brief comment would clarify the intent.Optional: Add clarifying comment
// Burn "720p" label and encode HD. + // Clone because we need the unmodified yuv_hd for SD downscaling. let mut hd_frame = yuv_hd.clone(); burn_label(&mut hd_frame, "720p");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dev/drone/src/video.rs` around lines 180 - 186, Add a short clarifying comment above the cloning line explaining that yuv_hd.clone() is required because burn_label(&mut hd_frame, "720p") mutates the frame and we must preserve the original yuv_hd for subsequent downscaling/SD processing; reference the symbols yuv_hd.clone(), burn_label, and hd_encoder.encode_yuv so the intent and necessity of the mutable copy are clear to future readers.rs/moq-mux/src/import/avc1.rs (1)
53-65: SPS parsing failures are silently ignored.If
ebsp_to_rbsporSps::parsefails, width/height remain 0 and becomeNonein the config. This is acceptable for fault tolerance, but logging a debug message on parse failure would help diagnose malformed streams.Optional: Log SPS parse failures
if offset + sps_len <= avcc.len() && !avcc[offset..].is_empty() { let sps_nalu = &avcc[offset..offset + sps_len]; let rbsp = h264_parser::nal::ebsp_to_rbsp(&sps_nalu[1..]); - if let Ok(sps) = h264_parser::Sps::parse(&rbsp) { - width = sps.width; - height = sps.height; + match h264_parser::Sps::parse(&rbsp) { + Ok(sps) => { + width = sps.width; + height = sps.height; + } + Err(e) => { + tracing::debug!(error = %e, "failed to parse SPS for dimensions"); + } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rs/moq-mux/src/import/avc1.rs` around lines 53 - 65, The SPS parsing failures in avc1.rs are currently swallowed (when calling h264_parser::nal::ebsp_to_rbsp and h264_parser::Sps::parse), leaving width/height at 0; update the block that processes sps_nalu so that parse errors are logged at debug level (include the sps_nalu length and error details). Specifically, inside the branch that builds rbsp from sps_nalu and calls h264_parser::Sps::parse, add a debug log on Err from ebsp_to_rbsp and on Err from Sps::parse (referencing sps_nalu, rbsp conversion failure, and the parse error) so failures are visible while preserving the existing fallback behavior that leaves width/height unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@dev/pub/justfile`:
- Around line 133-138: The cleanup() trap is missing EXIT and currently masks
the original exit status; update the trap to include EXIT (trap cleanup SIGINT
SIGTERM EXIT) and modify cleanup() to preserve the original exit code (save $?
into a local variable at start of cleanup, kill the backgrounded jobs/ffmpeg as
it already does, then exit with the saved status) so a crashed Python server or
port error doesn't leave ffmpeg orphaned and the original error code isn't lost;
locate the cleanup() function and the trap line and make these changes, and
ensure the backgrounded ffmpeg invocation referenced in the diff remains
targeted by kill $(jobs -p).
---
Nitpick comments:
In `@dev/drone/src/video.rs`:
- Around line 35-43: The loop draining commands currently ignores a Disconnected
result from cmd_rx.try_recv(), so if the sender is dropped the loop will
silently spin; update the try_recv handling on cmd_rx to match on Err(e) and
explicitly detect and handle TryRecvError::Disconnected (e.g., log a warning via
your logger or break/stop the loop) while preserving the current behavior for
TryRecvError::Empty, and keep the existing flow that converts names with
game::GameAction::from_str and calls game.apply_action(action).
- Around line 100-106: In the Drop implementation for EncoderHandle, do not
ignore the Result from thread.join(); instead match on the join outcome and log
any panic payload. In the impl Drop for EncoderHandle (fn drop), replace the
unused let _ = thread.join() with code that matches thread.join() -> Ok(_) =>
(), Err(payload) => { extract payload as &str or String via
downcast_ref::<&str>() / downcast_ref::<String>() and log it (e.g. log::error!
or eprintln!) with contextual text like "encoder thread panicked" }. Ensure you
reference EncoderHandle, the Drop::drop method, and the call to thread.join()
when making the change.
- Around line 180-186: Add a short clarifying comment above the cloning line
explaining that yuv_hd.clone() is required because burn_label(&mut hd_frame,
"720p") mutates the frame and we must preserve the original yuv_hd for
subsequent downscaling/SD processing; reference the symbols yuv_hd.clone(),
burn_label, and hd_encoder.encode_yuv so the intent and necessity of the mutable
copy are clear to future readers.
In `@rs/moq-mux/src/import/avc1.rs`:
- Around line 53-65: The SPS parsing failures in avc1.rs are currently swallowed
(when calling h264_parser::nal::ebsp_to_rbsp and h264_parser::Sps::parse),
leaving width/height at 0; update the block that processes sps_nalu so that
parse errors are logged at debug level (include the sps_nalu length and error
details). Specifically, inside the branch that builds rbsp from sps_nalu and
calls h264_parser::Sps::parse, add a debug log on Err from ebsp_to_rbsp and on
Err from Sps::parse (referencing sps_nalu, rbsp conversion failure, and the
parse error) so failures are visible while preserving the existing fallback
behavior that leaves width/height unchanged.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ab2b4251-5c11-404a-8457-1521c008385a
📒 Files selected for processing (12)
dev/drone/justfiledev/drone/src/game.rsdev/drone/src/index.tsdev/drone/src/sensor.rsdev/drone/src/video.rsdev/pub/justfiledev/web/justfiledoc/setup/demo/drone.mddoc/setup/demo/web.mddoc/setup/dev.mdrs/moq-mux/src/import/avc1.rsrs/moq-relay/README.md
✅ Files skipped from review due to trivial changes (6)
- doc/setup/demo/web.md
- doc/setup/dev.md
- rs/moq-relay/README.md
- dev/drone/justfile
- dev/web/justfile
- doc/setup/demo/drone.md
🚧 Files skipped from review as they are similar to previous changes (3)
- dev/drone/src/sensor.rs
- dev/drone/src/index.ts
- dev/drone/src/game.rs
Comment on lines
+133
to
+138
| cleanup() { | ||
| echo "Shutting down..." | ||
| kill $(jobs -p) 2>/dev/null || true | ||
| exit 0 | ||
| } | ||
| trap cleanup SIGINT SIGTERM |
Contributor
There was a problem hiding this comment.
Inconsistent cleanup: missing EXIT trap may leave ffmpeg orphaned.
Unlike the hls recipe (line 86) which traps SIGINT SIGTERM EXIT, this recipe only traps SIGINT SIGTERM. If the Python server exits unexpectedly (port conflict, crash), the backgrounded ffmpeg process (line 153) will continue running as an orphan.
Additionally, the exit 0 inside cleanup() masks any error exit status.
Suggested fix for consistent cleanup
cleanup() {
echo "Shutting down..."
kill $(jobs -p) 2>/dev/null || true
- exit 0
}
-trap cleanup SIGINT SIGTERM
+trap cleanup SIGINT SIGTERM EXIT📝 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
| cleanup() { | |
| echo "Shutting down..." | |
| kill $(jobs -p) 2>/dev/null || true | |
| exit 0 | |
| } | |
| trap cleanup SIGINT SIGTERM | |
| cleanup() { | |
| echo "Shutting down..." | |
| kill $(jobs -p) 2>/dev/null || true | |
| } | |
| trap cleanup SIGINT SIGTERM EXIT |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@dev/pub/justfile` around lines 133 - 138, The cleanup() trap is missing EXIT
and currently masks the original exit status; update the trap to include EXIT
(trap cleanup SIGINT SIGTERM EXIT) and modify cleanup() to preserve the original
exit code (save $? into a local variable at start of cleanup, kill the
backgrounded jobs/ffmpeg as it already does, then exit with the saved status) so
a crashed Python server or port error doesn't leave ffmpeg orphaned and the
original error code isn't lost; locate the cleanup() function and the trap line
and make these changes, and ensure the backgrounded ffmpeg invocation referenced
in the diff remains targeted by kill $(jobs -p).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced Mar 31, 2026
Merged
Merged
Merged
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
robo→dronethroughout (crate, package, paths, docs)dev/pubmodule, add flat justfile shortcuts (just drone,just pub)What's in the game
just drone 3spawns 3 drones)Test plan
just drone— starts relay + drone + viewer, video plays in browserjust drone 3— three drone cards appear in gridjust dev— web demo still works (pub module extracted correctly)just checkpasses🤖 Generated with Claude Code