refactor(build): nest static-site generator under cook build web

[dubadub]

Summary

• Restructure cook build as a parent command dispatching to named subcommands; introduce cook build web for the existing static-site generator.
• Carves out room for future build targets (e.g. printable cookbooks) without further reshaping the CLI surface.
• Updates docs (README.md, docs/build.md) and integration tests to the new invocation.

Breaking change: cook build ... invocations must now specify the target, e.g. cook build web instead of cook build.

Test plan

• cargo fmt --check
• cargo clippy --all-targets (clean)
• cargo test --test build (14/14 pass)
• cook build --help shows the new web subcommand
• cook build web --help shows the original options (output dir, --base-path, --base-url, --lang)

[claude]

Code Review: refactor(build): nest static-site generator under cook build web

Overview

This PR restructures cook build from a flat command into a parent command with subcommands, introducing cook build web for the existing static-site generator. The refactor is clean and the motivation (making room for future build targets) is sound.

---

Code Quality

Strengths:

• The Clap Subcommand + Args split is idiomatic and consistent with the rest of the codebase
• run() → run_web() dispatch is minimal and readable
• Every test in tests/build.rs (14 of them) has been updated consistently — no missed call sites
• Docs in both README.md and docs/build.md are updated correctly and consistently

Potential concern — BuildArgs::get_base_path():

impl BuildArgs {
    pub fn get_base_path(&self) -> Option<Utf8PathBuf> {
        match &self.command {
            BuildCommand::Web(args) => args.base_path.clone(),
        }
    }
}

This leaks WebBuildArgs internals through BuildArgs. If a future cook build pdf subcommand doesn't have a base_path, this match arm will need to return None and callers may not expect that. Worth checking where this method is called — if the only caller is run_web() (now private), the method could be removed from BuildArgs entirely and accessed directly on WebBuildArgs. If it's called from main.rs or Context setup before dispatch, keeping it on BuildArgs makes sense but should be documented.

---

Missing Test

There's no test verifying that cook build (no subcommand) exits with a helpful error or shows usage. Clap handles this automatically, but adding a quick assertion would prevent a regression if the subcommand config changes:

#[test]
fn build_without_subcommand_shows_help() {
    let mut cmd = Command::cargo_bin("cook").unwrap();
    cmd.args(["build"]).assert().failure();
}

---

Breaking Change Handling

The PR correctly documents this as a breaking change. Per project conventions (CLAUDE.md: "Avoid backwards-compatibility hacks"), no alias shim is needed — this is the right call. Just make sure the release notes / CHANGELOG call this out clearly so users upgrading aren't silently broken in scripts.

---

Minor Nit

The long_about on the Build variant in args.rs:

#[command(long_about = "Build artifacts (static website, etc.) from your recipe collection")]

The etc. feels a bit vague for a help string. Something like "Build artifacts from your recipe collection (e.g. static website)" or just relying on the doc-comment above would read more cleanly.

---

Summary

Area	Status
Correctness	✅ Clean dispatch, no logic changes
Conventions	✅ Matches existing command/module patterns
Docs	✅ README and docs/build.md updated
Tests	✅ All 14 tests updated; one edge case worth adding
Security	✅ No concerns
Breaking change	✅ Documented; intentional and appropriate

The get_base_path() point is the only thing worth resolving before merge — everything else is minor. Overall a well-scoped, clean refactor.

[claude]

Code Review: refactor(build): nest static-site generator under \cook build web``

Overview

Clean, minimal refactoring that promotes cook build to a parent command dispatching to named subcommands, with web being the first (and currently only) target. This opens the door for future build targets (e.g. cookbooks) without touching the CLI surface again.

---

✅ What's done well

• Minimal blast radius — the refactor is surgical; only src/build/mod.rs and src/args.rs change meaningfully on the Rust side.
• Consistent test updates — all 14 integration tests in tests/build.rs updated uniformly; snapshot files kept in sync.
• Documentation is thorough — both README.md and docs/build.md are updated with the new invocation; comment in builders.rs updated too.
• Breaking change is clearly called out in the PR description.

---

🔍 Concerns & Suggestions

1. BuildArgs::get_base_path() is a leaky abstraction

impl BuildArgs {
    pub fn get_base_path(&self) -> Option<Utf8PathBuf> {
        match &self.command {
            BuildCommand::Web(args) => args.base_path.clone(),
        }
    }
}

This method leaks a WebBuildArgs-specific field (base_path) up to the parent BuildArgs. As new subcommands are added (e.g., cook build book), each will require a new match arm — likely returning None if it doesn't have a base_path. That's manageable but easy to forget.

Suggestion: Either move this method to WebBuildArgs and have callers reach through the command enum, or document clearly in the method that it's a forwarding helper that must be extended for each new subcommand. If main.rs calls args.get_base_path() to configure the Context before dispatch, consider moving that configuration inside run_web() so the parent doesn't need to know about subcommand-specific fields at all.

---

2. No test for cook build --help (the parent's help)

build_command_help_works was changed from testing cook build --help to cook build web --help. Now there is no test verifying that the parent cook build --help actually lists the web subcommand. This is the primary UX change users will notice.

Suggestion: Add a test like:

#[test]
fn build_parent_help_lists_web_subcommand() {
    let mut cmd = Command::cargo_bin("cook").unwrap();
    let output = cmd.args(["build", "--help"]).assert().success();
    let stdout = String::from_utf8_lossy(&output.get_output().stdout);
    assert!(stdout.contains("web"), "Expected 'web' subcommand in build --help");
}

---

3. Breaking change — no migration path for existing scripts

cook build ... now fails with a clap error rather than running. There's no alias, deprecation warning, or redirect. This is intentional (and the right call for a breaking change), but worth confirming:

• Does the project have any CI scripts, Makefiles, or GitHub Actions workflows that call cook build without web? A quick grep -r "cook build" . in the repo is worth running before merging.
• The release notes / CHANGELOG should call this out prominently given downstream users may have CI pipelines using the old invocation.

---

Minor / Nitpicks

• The long_about on the Build variant in args.rs mentions "More targets (e.g. cookbooks) may be added in future releases." — this is fine as intent documentation, but if no cookbook target lands soon it'll age poorly. Consider "More targets may be added in future releases." without the example, or leave it as-is if a cookbook target is actually planned.
• The comment // (the common case: \cook build web` with default `_site` next to recipes)` in the regression test is a nice touch.

---

Summary

The refactor is clean and the intent is clear. The two items worth addressing before merge:

1. Add a test for cook build --help listing the web subcommand (prevents silent regression if the dispatch wiring breaks).
2. Verify no internal scripts still use the old cook build invocation before shipping the breaking change.

The get_base_path() concern is lower priority — it works today and is easy to address when the second subcommand lands.