Repository navigation
cleanup: ~1.3k lines of kept-dead modules, phantom Makefile targets, 7 TODOs, docs drift #10
Description
Activity
Status: #13 covers the Makefile phantom targets and docs-drift sections. While checking the other targets, its builder fixed a third broken one:
run-server/run-server-realnever passed thedummy/dirigerasubcommand, somake run-serverjust printed clap usage and exited 2.Residual found during #13 (adding it here, same subsystem)
-
Makefile:107-121,181,190: every test/bench/coverage target setsV1BECTL_GATEWAY_TYPE/V1BECTL_DUMMY_SCENARIO, and no Rust code reads either variable (workspace-wide grep: 0 hits outside the Makefile). Sotest-dummy,test-real,test-scenario SCENARIO=…andtest-scenariosall run the samecargo test --workspace. The scenario matrix is for show. Options: (a) strip the env vars and fold these intotest, or (b) make the scenario selectable for real once test: the core crates (state, sync, api, server, cli, tui) have zero tests #8'sDummyGatewaytests exist. I lean (a) now and (b) as part of test: the core crates (state, sync, api, server, cli, tui) have zero tests #8. -
make test=cargo test --workspaceincludesv1bectl_gtk, which doesn't build innix develop(ci: v1bectl_gtk can't build in the dev shell, v1bectl_web fails clippy (44 errors), flake unchecked #9:dbus-1missing). So the documentedmake testfails in the documented shell. Either mirror CI's--exclude v1bectl_gtk --exclude v1bectl_webor fix the devShell (depends on the gtk question on code-climate: This project could use some maintenance #5).
-
Update on the two held items (read-only investigation, spot-checked on
main@ c9f292c)1.
max_per_second(sync.rs:26): vestigial. The rate limiter it belongs to doesn't limit anything, but writes are still throttled.RateLimiter::acquire()(lib/v1bectl_sync/src/sync.rs:38-41) takes a semaphore permit and drops it right away when the function returns. So it's neither a per-second limit nor, in practice, a concurrency cap. The comment above the field ("the limit is enforced via the semaphore's permit count") is wrong, andSyncConfig.gateway_rate_limit = 10("10 req/s",:169) has no effect.- The throttle that does work is coalescing:
queue_sync(:255-267) keeps only the latest pending state per device in aHashMap, andstart_buffer_worker(:424-428) drains it every 50 ms. A dragged slider produces at most about 20 PATCHes/s per device. That's the behaviour the localtest_rate_limit.shexpects. - So deleting the field is safe. The better cleanup is to delete
RateLimiterentirely, or make it real by wiring the drain interval togateway_rate_limit. Moved from "hold" to the mechanical list: - Remove
RateLimiter(or makegateway_rate_limitactually drive the 50 ms drain interval), and fix the misleading comment.
2. TUI
main.rs:580TODO: a latent bug, currently unreachable.ApiResponse::DeviceState(lib/v1bectl_api/src/axum_server.rs:100-102) carries onlystate, with no device id. The handler hasdevice_idin scope (:308-317) and doesn't include it.- The TUI therefore updates the first device of the same type (
v1bectl_tui/src/main.rs:578-597). With two lights, a state response for light B would overwrite light A's row. - It's unreachable today because the TUI never sends
GetDeviceState; only the CLI (websocket_client.rs:105) and web (sensor_card.rs:31) do, and they match by request, not by row. - Add
device_idtoApiResponse::DeviceState(a small wire change: CBOR, manual schema) and match on it in the TUI. Low priority, but cheap to do together with test: the core crates (state, sync, api, server, cli, tui) have zero tests #8's API round-trip tests, since those will pin the response shape anyway.
Shipped: #13 (Docker targets and
unreliable_networkremoved,run-server/run-server-realfixed, crate names in CLAUDE.md/ARCHITECTURE.md corrected) is on main @ 5980bb3. Still open here: the dead-module deletes (web ones held on #5), removingRateLimiter, the inertV1BECTL_*Makefile env vars,make testincluding gtk,ApiResponse::DeviceStatemissingdevice_id, and the 7 TODOs.Shipped: #25 removed the web dead modules. Still open here: the CLI
client.rs/http_client.rsand TUIdashboard.rs(asked on #5), removingRateLimiter(after #23, same file), the inertV1BECTL_*Makefile env vars andmake test's old gtk exclude,ApiResponse::DeviceStatedevice_id, and the code TODOs.Shipped: #48 is on main @
bfec218. The Makefile no longer sets the inertV1BECTL_GATEWAY_TYPE/V1BECTL_DUMMY_SCENARIOvars,test-real/test-scenario(s)are gone, andtest/clippynow run the same commands as CI's native job. There are newtest-web/clippy-webtargets for the web job.RateLimiterwas already removed by #41.Still open here (re-swept on
bfec218):- Delete the CLI
client.rs/http_client.rsand the TUIdashboard.rs? This is still waiting on the question on code-climate: This project could use some maintenance #5. -
ApiResponse::DeviceStatehas nodevice_id(v1bectl_tui/src/main.rs:583). This goes with test: the core crates (state, sync, api, server, cli, tui) have zero tests #8 item 3, after 🔥 FIX - resync WS clients and virtual groups after falling behind the bus (#15) #47. - The 6 other code TODOs:
dirigera.rs:442(rgb from HSV),sync.rs:1397(state timestamp),light_group_linear.rs:121(inverse brightness map),v1bectl_server/src/main.rs:416(scene controller creation),screen_renderer.rs:173(template substitution),websocket_reconnect.rs:918(pure CBOR).
- Delete the CLI
Shipped: #52 (@
8a34c1b) addsdevice_idtoApiResponse::DeviceStateand fixes the TUI match (v1bectl_tui/src/main.rs, the old:583TODO is gone). Still open here: the dead-modules question (now alsotcp_server.rs, see #5), and 6 code TODOs:dirigera.rs:442,sync.rs:1397,light_group_linear.rs:121,v1bectl_server/src/main.rs:416,screen_renderer.rs:173,websocket_reconnect.rs(pure CBOR).A closer look at two of the remaining TODOs (on
07b7bb9):sync.rs:1583get_state_timestampalways returns "now", soConflictResolution::TimestampWinscan't work: both sides compare the time of the check. Nothing selects it (the only references are insync.rs), so this is dead code, not a live bug. Recommendation: remove theTimestampWinsvariant together withget_state_timestamp, instead of adding timestamps toDeviceStateValue. The sync test lane is busy with sync/virtual: after #50 — group writes bypass the engine, the expiry-race remainder, parallel group pushes + rate limit, events.rs doc nits #55, so this goes after it.screen_renderer.rs:173"Template substitution" has no syntax defined anywhere. The trollshell widget also renderstext "…"templates verbatim (v1bectl_widget/src/main.rs:562). This needs a design call before anyone builds it. @annikahannig, do you wanttextto support placeholders, something liketext "Living room: {temperature_living_room.temperature}°"? If yes, the web UI and the widget should share one parser.
- added a commit that references this issue
on Sep 28, 2026 Shipped: #59 (@
c5b3a3b) removesConflictResolution::TimestampWinsandget_state_timestamp. Thesync.rsTODO is gone. Code TODOs left:dirigera.rs:442(rgb from HSV, after #30/#37),light_group_linear.rs:121(inverse brightness map),v1bectl_server/src/main.rs:416(scene controller creation),screen_renderer.rs:173(template syntax, a design question above),websocket_reconnect.rs(pure CBOR).- added a commit that references this issue
on Sep 29, 2026 Shipped: #63 (@
d061692). Scene controllers fromvirtual_devices/*.tomlare now created and registered, and themain.rsTODO is gone. The loader was moved unchanged into a testableload_virtual_devices, and there are fixture-driven server tests. The format gaps and follow-ups are in #64. Code TODOs left:dirigera.rs:442(rgb from HSV, after #30/#37),light_group_linear.rs:121(being built now),screen_renderer.rs:173(template syntax, a design question),websocket_reconnect.rs(pure CBOR).Shipped: #65 (@
4b6eba1). ALightGroupLinearnow inverts each member's[min, max]map before averaging, so a group set to 50 reads back 50 instead of drifting to 77. Dimming a member at the wall re-derives to the level that member implies. After review, the echo-skip guard tests run over a curvedLightGroupagain (a skip-removed mutant turns 8 tests red). Follow-ups are in #66, including the same drift fix forLightGroup. Code TODOs left:dirigera.rs:442(rgb from HSV, after #30/#37),screen_renderer.rs:173(template syntax, a design question), andwebsocket_reconnect.rs(pure CBOR). Also still waiting on @annikahannig: the dead-modules question (#5).
Split out of #5. Measured on
main@ c9f292c.Dead modules kept under
#[allow(dead_code)](~1.3k lines)v1bectl_cli/src/client.rs(88) +http_client.rs(100):main.rs:1-10calls them "supported, not-yet-wired transports"v1bectl_tui/src/dashboard.rs(525):main.rs:1-4says it's "superseded by the scrollview dashboard … retained as reference/fallback"v1bectl_web/src/websocket_simple.rs(348): superseded bywebsocket_reconnect.rs, but both are still declared.lib.rs:2labels the simple one "NEW RECONNECTING WEBSOCKET", which is backwardsv1bectl_web"Legacy modules (keep for now)" (main.rs:12-17):pages/,context/,favorites,hooks,router, now behindcyber_appThese are also the bulk of the 44 web clippy errors in #9. Git history keeps them, so they don't need to live in the tree.
Dead fields that hide unfinished features
lib/v1bectl_sync/src/sync.rs:26max_per_secondis never read. Is the per-device rate limit actually enforced? (test_rate_limit.shexists, so it was meant to be.) Needs a check, not just a delete.v1bectl_tui/src/main.rs:132-140brightness_slider,color_temp_slider,show_create_virtual,virtual_device_name: the "TUI virtual device creation not fully implemented" item in CLAUDE.mdlib/v1bectl_api/src/server.rs:5port,lib/v1bectl_virtual/src/button_controller.rs:38-45,scene_controller.rs:36,lib/v1bectl_gateway/src/dirigera.rs:29,111,113Makefile targets that don't do what they say
Makefile:198-204docker-build/docker-runrundocker build ., but there's noDockerfilein the repoMakefile:212runstest-scenario SCENARIO=unreliable_network, butlib/v1bectl_virtual/src/dummy.rs:107maps"unreliable_network"straight toload_basic_home_scenario_static. It's a silent alias with no unreliabilityCode TODOs (7, the full sweep)
v1bectl_server/src/main.rs:408Implement scene controller creationlib/v1bectl_virtual/src/light_group_linear.rs:117Inverse-map member brightness → group brightness (a group's reported brightness drifts from its members)lib/v1bectl_gateway/src/dirigera.rs:433rgb_color: None, convert from HSV if availablelib/v1bectl_sync/src/sync.rs:988Extract timestamp from state if availablev1bectl_tui/src/main.rs:580"Need device_id in the state response to match properly". Possibly a real mismatch bug, so check it firstv1bectl_web/src/screen_renderer.rs:173Template substitutionv1bectl_web/src/websocket_reconnect.rs:308CBOR→JSON "for now", switch to pure CBORDocs drift
CLAUDE.md:34-44crate list omitsv1bectl_widget(README.md:64 has it) and uses flat names for thelib/cratesdocs/ARCHITECTURE.md:5,15usev1bectl-server/v1bectl-cli; the crates arev1bectl_server/v1bectl_cliCLAUDE.md:70lists "Virtual device system ✅", but in practice it's broken end-to-end (bug(virtual): virtual-device writes publish no DeviceEvents — echo-driven clients see stale state and latched toggles #1, bug(dummy): virtual_bedroom_lights references member devices the dummy scenario never creates — group control always errors #2)Recommendation
One mechanical PR for the deletes, the Makefile fixes and the docs. Hold the
max_per_secondand TUI:580items until someone has looked at them, since they may be bugs rather than dead code. The feature TODOs stay as this checklist until someone picks one up.Severity: Low. Maintenance drag, no user-facing impact, except possibly the unread rate-limit field.