Skip to content

Repo review, 1 Sep 2026: ranked improvements for the driver and for cloners #23

Description

@ThinkOffApp

Review of main at 1538087 (1 Sep 2026), asked by @Petrus in the room. Written by @claudemb with a full-tree sweep agent; every claim below was re-checked by hand against the code before filing. Ranked by value to the two users this repo has: the driver in the seat, and the cloner with a fresh Pi (273 stars, 17 forks, zero external issues or discussion comments so far, which says the cloners are not getting far enough to ask).

The verdict first: the in-car loop (voice in, grounded answer, voice out, OBD, manual RAG, model swap) is real and well engineered. listen.py, models.py, elm327.py, the auth gate in webchat.py and the two-layer private-HA check in mercedesme.py are careful work. What loses value is everything around that loop: a fresh clone cannot start, three daemons post through a script that is not in the repo, the offline promise in the README is not wired, and the hourly self-update can restart the car mid-answer.

Tier 1: a fresh clone does not run

  1. Two config paths, nothing creates the second one. install.sh:25-30 seeds /etc/carwatch/config.json, and config.py:14 plus mercedesme.py:84 read it. But agent.py:32, presence.py:26, listen.py:94,122, selfstate.py:216, obd_probe.py:24 and four sites in webchat.py (2344, 2454, 2578, 3164) read ~/.carwatch/config.json, which no script writes. Result on a fresh Pi after following the README: carwatch-agent exits, the dash has no handle, the listener has no key. Fix: one loader (config.py already is one), used everywhere, with install.sh writing exactly that file.

  2. Room posting depends on ~/post-as-gle.py, which is not in the repo. obdwatch.py:41, listen.py:520,538, reach.sh:22, scripts/pair-watch.sh:27. On any machine but the reference Pi the subprocess fails, is caught, prints post failed, and every engine milestone, every voice transcript and the tunnel URL silently never reach the room. carwatch/room.py (RoomClient) already exists and is only used once, by the dash chat view. Move the poster into the package and use it from all five sites. This is the exact failure ask.py:3-8 was written to end.

  3. The owner is hardcoded. agent.py:172 owner = "petrus" and presence.py:28 USER_ID = "@petrus". A cloner's car never answers its own owner in the room and heartbeats to the wrong dashboard. Both belong in config.

  4. install.sh stops half way. It installs wireless-tools poppler-utils bluez rsync (install.sh:44) and guides the .gguf download. It does not run apt-get update, does not install alsa-utils, ffmpeg (needed by voiceroom.py:76) or bluez-alsa, does not create ~/.carwatch, does not generate the dash token, and says nothing about the whisper and piper models. "Zero dependencies, clone and it runs" (README:41) is true of the Python and false of the product. Either finish the installer or make the README list the eleven external binaries honestly.

Tier 2: promises in the README that the code does not keep

  1. The offline outbox is not wired. README:211-213 says every post lands in a persistent on-disk outbox first. outbox.py is complete and tested, and is imported by nothing but the tests. agent._post (agent.py:127) posts straight to HTTP; last_seen is advanced at :427 before the answer is posted, and it is only rewound when the model is still loading (:438, :463), never when the post fails. A question asked in a tunnel is lost. Wire Outbox into _post and into the OBD and voice posters, and rewind on post failure.

  2. Hourly self-update restarts the car mid-drive and mid-answer. update.sh:13 does git reset --hard FETCH_HEAD and :153 restarts chat, agent, presence and obd every hour with no "nothing changed" exit, no brain lock, no speed check, no rollback. models.py:216-224 already shows the right pattern (hold voicestate.brain_lock() across the restart, roll back on failed start). Reuse it, skip the restart while speed_kmh > 0 or an answer is in flight, and keep the previous commit for rollback. A bad push to main currently reaches every car within the hour.

  3. Two brain paths that will drift. agent._think (agent.py:176-394) and webchat.answer (webchat.py:1065-1190) assemble different fact sets, one streams and one does not. The dash and the room can already give differently grounded answers to the same question. One think() in one module, both callers.

  4. Dead code that violates the grounding rule. commands.py:135-139 would tell a user "No OBD dongle installed yet, arrives in phase 4" while obdwatch streams live readings. It is unreachable today (only tests import Commands), as are TripTracker (trips.py), Wolfbox (wolfbox.py) and candecode.py. Roughly half of tests/test_carwatch.py tests modules nothing runs. Wire them into a daemon or delete them with their tests. FEATURES.md:10-22 still lists trips and the dashcam pipeline as implemented; README:252 correctly says the dashcam pipeline is not wired, and FEATURES.md:41 lists OBD health as planned while it is the flagship shipped feature. Generate one status table from one source.

Tier 3: hygiene that costs little

  1. Privacy: both licence plates are committed. profiles/eclass.json:11 and profiles/gle.json:14, next to "Helsinki" and "Berlin". Release notes v0.4.0 say "plate scrubbed from code" (commit 274618e); d156017 re-added them on 27 Aug. Petrus chose unblurred photos, so this is his call, but the release-notes claim and the file disagree, and a plate plus city plus name in a public repo is a lookup. Move plate to the private config, ship "" in profiles. Also bench-results/codex-pi-speech.wav (378 KB voice recording) is untracked and not gitignored; add bench-results/ to .gitignore.

  2. No CI, no logging, no contributor scaffolding. No .github/ directory, so the 34 passing tests (3.4 s, stdlib unittest, verified today) never run on push. logging is imported by zero modules; 121 print( calls and 139 except Exception: handlers (issue elm327.py swallows errors with bare except Exception (4 sites) #20 covers four of them) mean a field failure leaves no trace with a level or a timestamp. No CONTRIBUTING.md, SECURITY.md or issue template, for a repo that invites "here's mine on a different car". One workflow file running python3 -m unittest discover -s tests is an hour of work.

  3. Open security issues are still open and compound each other. Dashboard token travels in the URL query string #12 (token in URL), /api/update pulls and runs code, gated only by the dashboard token #13 (/api/update is remote code execution behind that token), Systemd units ship zero hardening (NoNewPrivileges, ProtectSystem, ProtectHome) #18 (no systemd hardening), Hardcoded home-LAN HA address in mercedesme.py breaks forks — and points at a stale IP #19 (hardcoded home HA IP at mercedesme.py:86 and webchat.py:110). Fixing Dashboard token travels in the URL query string #12 with a one-time ?t= to cookie exchange and a separate credential for /api/update removes most of the risk in an afternoon.

  4. Unbounded growth on the SD card. ~/.carwatch/obd-probe.log appends forever (update.sh:96), can-logs/rec-*.log and probe-results/deep-*.json are never pruned (deepscan.py:158, obdwatch.py:128-140), Outbox has no cap (outbox.py:33). No WatchdogSec, MemoryMax or OOMPolicy in any unit. A Pi that has driven for a year will find out the hard way.

  5. webchat.py is 3519 lines, 1053 of them base64 image blobs and about 330 of inline HTML, CSS and JS. Moving assets to carwatch/static/ served by the same handler keeps the zero-dependency rule and makes the dashboard reviewable.

What I would do first

Items 1 to 4 in one PR ("a fresh Pi runs"), then 5 and 6 in a second ("the promises hold while driving"). Each is a day or less. Everything in Tier 3 can be split across the team by file. I can take 1 to 4 myself if wanted; the update.sh guard (6) should be tested against tests/fake_elm327.py per docs/REVIEW_GATE.md before it goes to the car.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions