Narrow the build script's module graph (#513)

build.rs

@@ -16,56 +16,49 @@ use time::{OffsetDateTime, format_description::well_known::Iso8601};
 
 const FALLBACK_DATE: &str = "1970-01-01";
 
-// The build script recompiles these library modules as its own crate so that
-// `cli::Cli::command()` (used for man-page generation) can be constructed. Only
-// a small slice of each module's public API is reachable from this binary, so
-// the compiler reports the remainder as unused. Those items are not dead: their
-// real call sites live in the library crate and are covered by its tests, where
-// dead-code and unused-import analysis applies normally. Each shared module
-// therefore carries an `#[expect]` for exactly the lints it triggers here, in
-// preference to anchoring the symbols with artificial references.
-#[expect(
-    dead_code,
-    unused_imports,
-    reason = "shared library source; the unreached API is exercised by the library crate"
-)]
-#[path = "src/cli/mod.rs"]
-mod cli;
+// The build script recompiles a slice of the library as its own crate so that
+// `cli::Cli::command()` (used for man-page generation) can be constructed, and
+// so that the localization audit can read the declared key registry.
+//
+// The slice is named file by file rather than by pulling in `src/cli/mod.rs`,
+// because that would drag the whole `cli` subtree — configuration discovery,
+// merging, diagnostics, localised value parsing — into this crate, where none
+// of it is reachable. Recompiling only what is reachable keeps rustc's
+// unused-item analysis meaningful here instead of requiring module-wide
+// `#[expect(dead_code)]` suppressions that would also mask genuinely dead
+// library code.
+//
+// The library modules below are laid out to keep this slice small:
+// `src/cli/command.rs` holds definitions only, with runtime behaviour in
+// `src/cli/preferences.rs` and `src/cli/parser.rs`; matching logic is split out
+// of `src/host_pattern.rs` into `src/host_matching.rs`. Adding a dependency on
+// anything outside this slice will surface here as a compile error, which is
+// the intended signal.
+#[path = "src/cli"]
+mod cli {
+    //! The Clap schema slice of `src/cli`, mirroring `src/cli/mod.rs`.
+
+    #[path = "config.rs"]
+    pub mod config;
+    #[path = "validation.rs"]
+    mod validation;
+
+    #[path = "command.rs"]
+    mod command;
+
+    pub use command::Cli;
+    pub use config::{AccessibilityPolicy, ColourPolicy, EmojiPolicy, ProgressPolicy};
+}
 
 #[path = "src/cli_localization.rs"]
 mod cli_localization;
 
-#[expect(
-    dead_code,
-    reason = "shared library source; the unreached API is exercised by the library crate"
-)]
-#[path = "src/cli_l10n.rs"]
-mod cli_l10n;
-
-#[expect(
-    dead_code,
-    reason = "shared library source; the unreached API is exercised by the library crate"
-)]
 #[path = "src/host_pattern.rs"]
 mod host_pattern;
 
 #[path = "src/localization/mod.rs"]
 mod localization;
 
-#[expect(
-    dead_code,
-    reason = "shared library source; the unreached API is exercised by the library crate"
-)]
-#[path = "src/output_mode.rs"]
-mod output_mode;
-
-#[expect(
-    dead_code,
-    reason = "shared library source; the unreached API is exercised by the library crate"
-)]
-#[path = "src/theme.rs"]
-mod theme;
-
 mod build_l10n_audit;
 
 fn manual_date() -> String {
@@ -114,11 +107,12 @@ fn write_man_page(data: &[u8], dir: &Path, page_name: &str) -> std::io::Result<P
 }
 
 fn emit_rerun_directives() {
-    println!("cargo:rerun-if-changed=src/cli/mod.rs");
+    // Only the modules this script actually compiles need to trigger a rerun.
+    println!("cargo:rerun-if-changed=src/cli/command.rs");
     println!("cargo:rerun-if-changed=src/cli/config.rs");
-    println!("cargo:rerun-if-changed=src/cli/merge.rs");
-    println!("cargo:rerun-if-changed=src/cli/parser.rs");
-    println!("cargo:rerun-if-changed=src/cli/parsing.rs");
+    println!("cargo:rerun-if-changed=src/cli/validation.rs");
+    println!("cargo:rerun-if-changed=src/host_pattern.rs");
+    println!("cargo:rerun-if-changed=src/cli_localization.rs");
     println!("cargo:rerun-if-env-changed=CARGO_PKG_VERSION");
     println!("cargo:rerun-if-env-changed=CARGO_PKG_NAME");
     println!("cargo:rerun-if-env-changed=CARGO_BIN_NAME");

docs/developers-guide.md

@@ -377,6 +377,39 @@ unit tests, and `rstest-bdd` release-help scenarios.
 `src/cli/config_path_precedence_tests.rs` is the canonical exhaustive
 state-enumeration example.
 
+## The build script's module slice
+
+`build.rs` recompiles part of the library as its own crate: it needs
+`cli::Cli::command()` for man-page generation and the key registry in
+`src/localization/keys.rs` for the Fluent audit. Rather than declaring
+`src/cli/mod.rs` and inheriting the whole subtree, it declares an inline `cli`
+module naming exactly three files — `src/cli/command.rs`, `src/cli/config.rs`,
+and `src/cli/validation.rs`.
+
+That slice is a maintained boundary, not an accident:
+
+- `src/cli/command.rs` holds Clap definitions only. Runtime behaviour on `Cli`
+  belongs in `src/cli/preferences.rs`, and the localization-aware parsing entry
+  point belongs in `src/cli/parser.rs`.
+- `src/cli/validation.rs` holds the shared limits and error constructor that
+  `src/cli/config.rs` needs, so neither file has to reach up into
+  `src/cli/mod.rs`.
+- `src/host_pattern.rs` covers pattern syntax; matching a concrete hostname
+  against a parsed pattern lives in `src/host_matching.rs`, which the build
+  script does not compile.
+
+Keeping the slice narrow is what lets rustc's unused-item analysis run normally
+inside the build-script crate. Widening it — for example by making
+`src/cli/command.rs` depend on the merge or discovery layers — reintroduces
+unreachable items and, with them, the module-wide `#[expect(dead_code)]`
+suppressions that issue #513 removed. Those suppressions also masked genuinely
+dead code: an unused `pub` item in `src/cli/config.rs` is reported by the
+build-script crate but not by the library, because the library exports that
+module publicly.
+
+A dependency added outside the slice surfaces as a build-script compile error.
+Prefer moving the new code into a sibling module over widening the slice.
+
 ## Local build acceleration
 
 Debug builds and tests can optionally use the [`mold`] linker and the Cranelift

docs/netsuke-design.md

@@ -2429,7 +2429,9 @@ the targets listed in the `defaults` section of the manifest are built.
 
 ### 8.4 Design Decisions
 
-The parser-facing `Cli` type is now defined in `src/cli/parser.rs`, while
+The parser-facing `Cli` type is now defined in `src/cli/command.rs`, with the
+localization-aware parsing entry point in `src/cli/parser.rs` and the runtime
+preference accessors in `src/cli/preferences.rs`, while
 layered configuration lives in a dedicated `CliConfig` struct derived with
 OrthoConfig in `src/cli/config.rs`. The top-level `src/cli/mod.rs` module
 re-exports that public CLI surface. This separation keeps parsing,

src/cli/command.rs

@@ -0,0 +1,212 @@
+//! The Clap-derived command tree.
+//!
+//! This module owns the runtime-visible [`Cli`] struct and every associated
+//! Clap definition ([`InteractionArgs`], [`BuildArgs`], [`GraphArgs`],
+//! [`Commands`]). It holds definitions only: no parsing entry point, no
+//! localisation, and no runtime behaviour.
+//!
+//! **Pipeline position:** schema layer, below [`super::parser`].
+//!
+//! The narrow dependency surface is deliberate. `build.rs` recompiles this
+//! module (plus [`super::config`] and [`super::validation`]) to obtain
+//! `Cli::command()` for man-page generation; anything reachable from here is
+//! also compiled by the build script, so behaviour that the man page does not
+//! need belongs in a sibling module instead.
+
+use clap::{Args, Parser, Subcommand};
+use serde::{Deserialize, Serialize};
+use std::path::PathBuf;
+
+use super::config::CliConfig;
+use super::{AccessibilityPolicy, ColourPolicy, EmojiPolicy, ProgressPolicy};
+use crate::host_pattern::HostPattern;
+
+/// A modern, friendly build system that uses YAML and Jinja, powered by Ninja.
+#[derive(Debug, Parser, Serialize, Deserialize)]
+#[command(author, version, about, long_about = None)]
+pub struct Cli {
+    /// Path to the Netsuke manifest file to use.
+    #[arg(
+        short,
+        long,
+        value_name = "FILE",
+        default_value_os_t = CliConfig::default_manifest_path()
+    )]
+    pub file: PathBuf,
+
+    /// Run as if started in this directory.
+    ///
+    /// This affects manifest lookup, output paths, and config discovery.
+    #[arg(short = 'C', long, value_name = "DIR")]
+    pub directory: Option<PathBuf>,
+
+    /// Path to a configuration file, bypassing automatic discovery.
+    #[arg(long, value_name = "FILE")]
+    #[serde(skip)]
+    pub config: Option<PathBuf>,
+
+    /// Set the number of parallel build jobs.
+    ///
+    /// Values must be between 1 and 64.
+    #[arg(short, long, value_name = "N")]
+    pub jobs: Option<usize>,
+
+    /// Enable verbose diagnostic logging and completion timing summaries.
+    #[arg(short, long)]
+    pub verbose: bool,
+
+    /// Locale tag for CLI copy (for example: en-US, es-ES).
+    #[arg(long, value_name = "LOCALE")]
+    pub locale: Option<String>,
+
+    /// Additional URL schemes allowed for the `fetch` helper.
+    #[arg(long = "fetch-allow-scheme", value_name = "SCHEME")]
+    pub fetch_allow_scheme: Vec<String>,
+
+    /// Hostnames that are permitted when default deny is enabled.
+    ///
+    /// Supports wildcards such as `*.example.com`.
+    #[arg(long = "fetch-allow-host", value_name = "HOST")]
+    pub fetch_allow_host: Vec<HostPattern>,
+
+    /// Hostnames that are always blocked, even when allowed elsewhere.
+    ///
+    /// Supports wildcards such as `*.example.com`.
+    #[arg(long = "fetch-block-host", value_name = "HOST")]
+    pub fetch_block_host: Vec<HostPattern>,
+
+    /// Deny all hosts by default; only allow the declared allowlist.
+    #[arg(long = "fetch-default-deny")]
+    pub fetch_default_deny: bool,
+
+    /// Emit machine-readable JSON output.
+    #[arg(long)]
+    pub json: bool,
+
+    /// Interaction policy flags.
+    #[command(flatten)]
+    pub interaction: InteractionArgs,
+
+    /// Select the colour policy for terminal output.
+    #[arg(long, value_name = "POLICY", default_value_t)]
+    pub color: ColourPolicy,
+
+    /// Select the emoji policy for terminal output.
+    #[arg(long, value_name = "POLICY", default_value_t)]
+    pub emoji: EmojiPolicy,
+
+    /// Select the progress-rendering policy.
+    #[arg(long, value_name = "POLICY", default_value_t)]
+    pub progress: ProgressPolicy,
+
+    /// Select the accessible-output policy.
+    #[arg(long, value_name = "POLICY", default_value_t)]
+    pub accessibility: AccessibilityPolicy,
+
+    /// Default build targets used when none are specified on the CLI.
+    #[arg(long = "default-target", value_name = "TARGET")]
+    pub default_targets: Vec<String>,
+
+    /// Optional subcommand to execute; defaults to `build` when omitted.
+    #[serde(skip)]
+    #[command(subcommand)]
+    pub command: Option<Commands>,
+}
+
+impl Cli {
+    /// Apply the default command if none was specified.
+    #[must_use]
+    pub fn with_default_command(mut self) -> Self {
+        if self.command.is_none() {
+            self.command = Some(Commands::Build(BuildArgs::default()));
+        }
+        self
+    }
+}
+
+impl Default for Cli {
+    fn default() -> Self {
+        Self {
+            file: CliConfig::default_manifest_path(),
+            directory: None,
+            config: None,
+            jobs: None,
+            verbose: false,
+            locale: None,
+            fetch_allow_scheme: Vec::new(),
+            fetch_allow_host: Vec::new(),
+            fetch_block_host: Vec::new(),
+            fetch_default_deny: false,
+            json: false,
+            interaction: InteractionArgs::default(),
+            color: ColourPolicy::Auto,
+            emoji: EmojiPolicy::Auto,
+            progress: ProgressPolicy::Auto,
+            accessibility: AccessibilityPolicy::Auto,
+            default_targets: Vec::new(),
+            command: None,
+        }
+        .with_default_command()
+    }
+}
+
+/// Arguments controlling whether Netsuke may read interactive input.
+#[derive(Debug, Args, PartialEq, Eq, Clone, Serialize, Deserialize)]
+pub struct InteractionArgs {
+    /// Never read interactive input.
+    #[arg(long, default_value_t = true)]
+    pub no_input: bool,
+}
+
+impl Default for InteractionArgs {
+    fn default() -> Self {
+        Self { no_input: true }
+    }
+}
+
+/// Arguments accepted by the `build` command.
+#[derive(Debug, Args, PartialEq, Eq, Clone, Serialize, Deserialize, Default)]
+pub struct BuildArgs {
+    /// A list of specific targets to build.
+    #[serde(default)]
+    pub targets: Vec<String>,
+}
+
+/// Arguments accepted by the `graph` command.
+///
+/// `html` and `output` are per-invocation flags and are intentionally excluded
+/// from `OrthoConfig` layering (`#[serde(skip)]`); layering them through a
+/// configuration file would silently change the artefact destination.
+#[derive(Debug, Args, PartialEq, Eq, Clone, Serialize, Deserialize, Default)]
+pub struct GraphArgs {
+    /// Render the graph as a self-contained HTML page instead of DOT.
+    #[arg(long)]
+    #[serde(skip)]
+    pub html: bool,
+
+    /// Write the graph artefact to FILE. Use `-` for stdout.
+    #[arg(long, value_name = "FILE")]
+    #[serde(skip)]
+    pub output: Option<PathBuf>,
+}
+
+/// Available top-level commands for Netsuke.
+#[derive(Debug, Subcommand, PartialEq, Eq, Clone, Serialize, Deserialize)]
+#[serde(rename_all = "kebab-case")]
+pub enum Commands {
+    /// Build specified targets (or default targets if none are given).
+    Build(BuildArgs),
+
+    /// Remove build artefacts and intermediate files.
+    Clean,
+
+    /// Display the build dependency graph in DOT format for visualisation.
+    Graph(GraphArgs),
+
+    /// Generate the Ninja manifest without invoking Ninja.
+    Generate {
+        /// Write the generated Ninja manifest to FILE instead of stdout.
+        #[arg(long, value_name = "FILE")]
+        output: Option<PathBuf>,
+    },
+}

src/cli/config.rs

@@ -11,7 +11,7 @@ use std::fmt;
 use std::path::PathBuf;
 use std::str::FromStr;
 
-use super::validation_error;
+use super::validation::validation_error;
 use crate::host_pattern::HostPattern;
 
 /// Required non-interactive execution setting.
@@ -275,7 +275,7 @@ impl CliConfig {
     }
 }
 
-const MAX_JOBS: usize = super::MAX_JOBS;
+const MAX_JOBS: usize = super::validation::MAX_JOBS;
 
 const fn jobs_out_of_bounds(jobs: usize) -> bool {
     jobs == 0 || jobs > MAX_JOBS

src/cli/diag.rs

@@ -11,8 +11,8 @@ use ortho_config::{OrthoError, OrthoResult};
 use serde_json::Value;
 use std::sync::Arc;
 
+use super::command::Cli;
 use super::discovery::{EnvProvider, StdEnvProvider, collect_diag_file_layers_with_env};
-use super::parser::Cli;
 
 const JSON_ENV_VAR: &str = "NETSUKE_JSON";