logoalt Hacker News

WD-42yesterday at 12:06 PM1 replyview on HN

Pretty much. I plan on using this program for a long time. I don’t want a codebase that looks like something out of a H.P. Lovecraft novel when I have to fix something in the future.


Replies

igraviousyesterday at 1:08 PM

Here's an analysis of your codebase by Claude Opus 4.6 and Kimi 2.5 -- I don't code in Rust or GTK so I can't vouch for how spot on this is but I hope it's not too far off the mark and helps you improve your code. (Claude is wrapping both analyses together.)

  Strong agreement                                                                                                                                                                                                             
                                                                                                                                                                                                                               
  - Panicking error handling — both flag unwrap()/expect() on fallible operations (secrets, HTTP client, GStreamer). Kimi specifically calls out get_stream_uri returning String instead of Result, which I missed — good      
  catch.                                                                                                                                                                                                                       
  - Excessive cloning — both note the Jellyfin client and string cloning. Kimi's phrasing ("host.to_string() on every request construction") is more specific than mine.
  - Application struct overloaded — both agree it holds too many responsibilities. Kimi suggests splitting into LibraryService, PlaybackService, etc. I focused more on the RefCell exposure.
  - Silent error suppression — both flag let _ = sender.send(...) and unwrap_or_default() patterns.
  - Code duplication in UI — both identify the repetitive list/detail/sorter patterns. Kimi specifically calls out call_on_visible_page widget comparisons, which I grouped under the broader "5+ copies of factory setup"
  observation.
  - Missing HTTP timeouts — Kimi flags this explicitly. I noted it indirectly via the buffer_unordered without timeout, but Kimi is clearer.

  Things Kimi caught that I didn't emphasise

  - get_stream_uri returning String instead of Result — a genuine API design issue I glossed over.
  - String-based signal names being error-prone with no compile-time checking — fair point, though it's idiomatic GTK4/Rust.
  - glycin potentially unused — I didn't check dependency usage.
  - Position polling not accounting for seeks — a practical UX observation I missed.
  - /proc/sys/kernel/hostname read being cached in OnceLock — Kimi approves the caching but notes it could be done once at startup. Minor but valid.

  Things I caught that Kimi missed

  - Actual bugs — queue reorder off-by-one, playlist DnD index error, toast format string "HTTP {} error" never substituted, double-wrapped Result in stream_info.rs, reversed comment in shuffle logic. These are concrete
  defects, not just style issues.
  - Concurrency issues in depth — the Cell<bool> shuffle state race, futures::Mutex inside glib::spawn_future_local deadlock risk, queue mutation race between remove_all() and extend_from_slice().
  - Signal handler accumulation on widget reuse in ListView factories — a slow memory leak.
  - Main-thread blocking from synchronous library filtering in detail views.
  - Image cache with no eviction — unbounded disk growth.
  - bytes_to_texture panicking inside a Result-returning function — defeats the entire error contract.
show 1 reply