Skip to content

Use cooklang-find's list_menus_for_date for the web UI today's menu#360

Merged
dubadub merged 5 commits into
mainfrom
feat/menus-for-date
Jun 24, 2026
Merged

Use cooklang-find's list_menus_for_date for the web UI today's menu#360
dubadub merged 5 commits into
mainfrom
feat/menus-for-date

Conversation

@dubadub

@dubadub dubadub commented Jun 24, 2026

Copy link
Copy Markdown
Member

Summary

  • Bump cooklang-find 0.5.0 → 0.6, which adds list_menus_for_date(base_dirs, date).
  • Rewrite find_todays_menu (src/server/handlers/menus.rs) to delegate the menu-by-date scan to the library instead of building the menu list and fully parsing every menu to regex-extract a section date. chrono stays in cookcli for computing/displaying "today".
  • Drop the now-unused tree parameter from find_todays_menu and update its single caller in src/server/builders.rs.

Notes

  • Date matching is slightly wider (intentional): the old code required a parenthesized date ((2026-06-24)); the library matches the date as a substring anywhere in a section header, so = 2026-06-24 Dinner now matches too. A strict superset of the old behavior. Documented in the spec.
  • Scope is today only; no template/UI or endpoint changes. collect_menus, extract_date, and the other menu handlers are untouched.

Test Plan

  • cargo build — clean, no warnings
  • cargo clippy --all-targets -- -D warnings — clean
  • cargo test — full suite green, plus 3 new tests for find_todays_menu (parenthesized match, bare-date match, no-match)
  • Manual: run cook server against a folder with a .menu containing a section dated today; confirm the home page shows today's menu

Design + plan: docs/superpowers/specs/2026-06-24-menus-for-date-integration-design.md, docs/superpowers/plans/2026-06-24-menus-for-date-integration.md

@claude

claude Bot commented Jun 24, 2026

Copy link
Copy Markdown

Code Review

Overall this is a clean, well-scoped change that simplifies find_todays_menu significantly. A few things worth flagging:


Dependency footprint (worth discussing upstream)

cooklang-find 0.6.0 now depends on uniffi unconditionally. The lock file gains ~20 new packages:

uniffi, uniffi_bindgen, uniffi_build, uniffi_checksum_derive, uniffi_core,
uniffi_macros, uniffi_meta, uniffi_testing, uniffi_udl, weedle2, goblin,
cargo_metadata, cargo-platform, bincode, fs-err, paste, scroll, scroll_derive,
static_assertions, toml 0.5.11

uniffi is a foreign-function interface generator (for iOS/Android bindings). Having it as a hard dependency of a CLI tool adds substantial compile time and binary size with no runtime benefit. Worth filing an issue/PR on cooklang-find to make uniffi a feature-gated optional dependency, or confirming the intent.


Double directory scan (mild performance regression)

The old code received the already-built RecipeTree via the tree parameter and reused it. The new code calls list_menus_for_date which does its own filesystem scan, so every home-page load now triggers two scans: one from build_tree and one inside find_todays_menu. For typical recipe collections this is imperceptible, but it is a step backward. If performance ever becomes a concern here, one option is to pass a pre-built list of menu entries from the tree to avoid the second scan.


strip_prefix fallback silently produces absolute paths

let relative = full_path
    .strip_prefix(base_path)
    .unwrap_or(full_path.as_ref());

If strip_prefix fails (e.g. list_menus_for_date returns an entry whose path doesn't start with base_path), menu_path will be an absolute filesystem path. That would silently produce broken links in the web UI. This probably never happens in practice since the library is called with base_path, but a more defensive option would be to return None or log a warning on prefix mismatch instead of falling back to the full path.


Test assertions are minimal

Each test only asserts on menu_path. Consider also asserting on menu_name (e.g. "week") and maybe date_display to lock in the full contract of the returned struct.


Minor nits

  • entry.name().clone() — the .clone() is necessary when name() returns a reference, which is fine. If you're ever on a newer API that returns Option<String> by value, it becomes a no-op but harmless.
  • unwrap_or_default() on the list_menus_for_date result silently swallows scan errors. This matches the original continue-on-error approach and is appropriate for UI code, but a tracing::warn! on the error path could help with debugging in the field.

What's good

  • The core change is correct and well-motivated — delegating the date-scan to the library removes ~30 lines of bespoke parsing and regex.
  • The behavioral difference (bare date headers now match) is documented in the spec, the PR description, and the doc comment.
  • Three unit tests cover the match/no-match/new-format cases cleanly.
  • tree removal is handled properly: the parameter is dropped, the caller is updated, and the tree variable in builders.rs remains used for its other purposes.

@dubadub dubadub merged commit 2da1fe7 into main Jun 24, 2026
6 checks passed
@dubadub dubadub deleted the feat/menus-for-date branch June 24, 2026 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant