Add Whitaker to lint checks

[leynos]

Summary

This branch adds Whitaker to the Dear Diary lint contract so local and CI linting now run Rustdoc, Clippy, and the Whitaker Dylint suite before the branch can pass review. It follows the current Weaver setup by installing the pinned Whitaker installer revision in CI before invoking make lint.

There is no linked issue, roadmap task, or execplan for this branch.

Review walkthrough

• Start with the CI and local lint wiring in .github/workflows/ci.yml and Makefile to see how Whitaker is installed and invoked.
• Then review docs/developers-guide.md for the contributor-facing installation notes.
• Review the Whitaker-driven complexity refactor in crates/dear-diary-config/src/interpolation/mod.rs and crates/dear-diary-config/src/interpolation/parse.rs.
• Finish with the test-only lint clean-ups in crates/dear-diary-config/src/settings.rs, crates/dear-diary-embeddings/src/fastembed.rs, crates/dear-diary-mcp/src/deprecation.rs, crates/dear-diary-mcp/src/server.rs, crates/dear-diary-mcp/src/server_tests.rs, and crates/dear-diary-qdrant/src/entry.rs.
• The Markdown-only formatting churn comes from the repository formatter and is visible in AGENTS.md, docs/complexity-antipatterns-and-refactoring-strategies.md, docs/ortho-config-users-guide.md, docs/release-process.md, docs/reliable-testing-in-rust-via-dependency-injection.md, docs/rstest-bdd-users-guide.md, docs/rust-doctest-dry-guide.md, docs/rust-testing-with-rstest-fixtures.md, and docs/users-guide.md.

Validation

• actionlint .github/workflows/ci.yml: passed
• mbake validate Makefile: passed
• make check-fmt: passed
• make markdownlint: passed
• make nixie: passed
• whitaker-installer --cranelift: passed
• make lint: passed, including Whitaker
• cargo test -p dear-diary-config interpolation: passed
• cargo test -p dear-diary-mcp --lib server::server_tests: passed
• make test: passed
• make test-scripts: passed, including 25 Python tests and 2 snapshots
• git diff --check: passed

Notes

The first Whitaker run exposed existing complexity and test-module documentation findings. This branch fixes those findings directly rather than adding suppressions, so the newly added lint gate is active immediately.

Summary by Sourcery

Integrate the Whitaker lint suite into the project’s lint pipeline and refactor interpolation logic to satisfy new complexity checks while updating developer docs and tests accordingly.

Enhancements:

• Refactor interpolation placeholder handling into smaller helpers and a bitflag-based tracker to reduce cognitive complexity and centralize error construction.
• Simplify remote URL parsing by splitting path processing into segment and metadata helpers for clearer control flow.
• Extract a reusable duration-to-seconds helper in MCP server tests and add module-level documentation comments to multiple test modules for clarity.
• Apply repository markdown formatting and table-alignment cleanups across various documentation files, including complexity, testing, configuration, and deprecation guides.

Build:

• Wire Whitaker into the Makefile lint target so it runs alongside Rustdoc and Clippy with a consistent PATH and tool resolution.

CI:

• Add Whitaker installation to the CI workflow using a pinned installer revision before invoking the lint job.

Documentation:

• Update the developer guide with instructions for installing and running Whitaker locally as part of the lint workflow.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 4309aa99-4daf-45b6-be87-0b868229a0f7

📥 Commits

Reviewing files that changed from the base of the PR and between c0a1cab and 5357839.

📒 Files selected for processing (1)

• .github/workflows/ci.yml

---

Overview

This pull request integrates Whitaker (a Dylint-based lint suite) into the repository lint contract for local and CI environments. make lint now runs Rustdoc, Clippy and Whitaker; CI installs a pinned whitaker-installer revision and runs whitaker-installer --cranelift before invoking the lint target. The change set includes a Whitaker-driven refactor of interpolation logic to reduce cyclomatic complexity and centralise error construction, plus test-only clean-ups, added tests, and broad documentation/formatting fixes.

Infrastructure and CI

• .github/workflows/ci.yml

  • Adds a job-level WHITAKER_INSTALLER_REV environment variable.
  • Adds an actions/cache step keyed on runner.os and WHITAKER_INSTALLER_REV to persist Whitaker installer outputs (cached paths include installed binaries and Whitaker driver/share dirs).
  • Installs the pinned whitaker-installer from the git revision only on cache miss; always runs whitaker-installer --cranelift before linting.
  • Runs make test-scripts prior to Whitaker installation and preserves existing lint/coverage/CodeScene steps.

• Makefile

  • Adds USER_WHITAKER and USER_BIN_PATH and defines WHITAKER to prefer user-local installs but fall back to system whitaker.
  • Updates the lint target to run whitelist/whitaker via PATH including user-local bins, invoking whitaker --all alongside existing CARGO_FLAGS and RUSTFLAGS.

• AGENTS.md

  • Tightens dependency management rules (mandates caret ^ SemVer for dependencies; forbids wildcard * and open >= specifiers; clarifies restricted use of ~).

Commit messages focus on stabilising CI caching and installer bootstrap: cache key includes installer revision, skip reinstallation on cache hit, but still run the installer bootstrap (cranelift) before make lint.

Code refactorings

• crates/dear-diary-config/src/interpolation/mod.rs

  • Introduces PlaceholderNeeds composed of grouped RemotePlaceholderNeeds and LocalPlaceholderNeeds with const predicates and an early fast-path when no supported placeholders exist.
  • Decomposes interpolation into helpers: replace_remote_placeholders, remote_owner, replace_cwd_placeholder, replace_branch_placeholder; centralises unresolvable-error construction.

• crates/dear-diary-config/src/interpolation/parse.rs

  • Replaces allocation-heavy parsing with a non-allocating RemotePathSegments scanner and predicate helpers (is_empty, is_single_segment) plus helper functions for segment extraction and error construction; removes the extra Vec allocation and multi-branch match.

• PlaceholderNeeds representation

  • Replaces the compact bitflag PlaceholderNeeds(u8) API with grouped boolean structs while retaining predicate helpers (needs_remote, needs_repo, etc.) for readability.

• crates/dear-diary-mcp/src/server_tests.rs and related tests

  • current_timestamp() now returns Result<i64, SystemTimeError> to propagate SystemTime errors instead of masking them; introduces duration_secs_i64 helper and updates tests to return Result and use ?.

• Additional test-only and minor refactors

  • Moves inline test modules into separate test files (settings.rs → settings_tests.rs).
  • Adds/expands tests in crates/dear-diary-config/src/settings_tests.rs, including EnvGuard and tests for Settings::from_env_with_git and placeholder resolution.
  • Adds module-level doc comments to several test modules (dear-diary-embeddings, dear-diary-mcp, dear-diary-qdrant).

Documentation and design links

• Adds ADR: docs/adr-001-whitaker-lint-contract.md formalising Whitaker adoption, lint-gate expectations and consequences (including discouraging .expect() outside recognised test bodies and requiring configuration-loading tests).
• Updates docs/developers-guide.md with local Whitaker installation instructions (cargo install of pin-pointed whitaker-installer and running whitaker-installer --cranelift) and rationale/examples of enforced rules.
• Numerous documentation-only formatting and table-alignment clean-ups across docs/* (complexity-antipatterns, ortho-config-users-guide, rstest guides, users-guide, release-process, etc.).

(An execplan-type ADR describing the Whitaker lint contract was added — docs/adr-001-whitaker-lint-contract.md — and is referenced by the developers’ guide updates.)

Review feedback and resolutions

Reviewer points were addressed:

• CI caching and installer gating: actions/cache keyed on WHITAKER_INSTALLER_REV added; installer git install gated on cache miss; whitaker-installer --cranelift is still run before linting.
• SystemTime behaviour: current_timestamp now propagates errors; tests updated accordingly.
• Allocation elimination: parse_remote_url now uses RemotePathSegments to avoid Vec allocation.
• Bitflag complexity: PlaceholderNeeds replaced with grouped booleans and preserved predicate helpers.
• Multi-branch match complexity: extracted predicate helpers on RemotePathSegments used to simplify control flow.

Validation

Reported validation steps passed: actionlint, Makefile validation, formatting and markdown checks, whitaker-installer --cranelift, make lint (including Whitaker), cargo tests, make test, make test-scripts, and git diff checks. Whitaker findings were fixed rather than suppressed, enabling the new lint gate immediately.

Notable test additions

• crates/dear-diary-config/src/settings_tests.rs: comprehensive tests for defaults, Qdrant validation, filterable-field mapping, environment-driven settings loading, and Git-placeholder resolution; includes an EnvGuard utility for serialising/restoring environment variables.

Walkthrough

Integrate Whitaker installer into CI and Makefile; refactor collection-name interpolation and remote parsing; extract and expand settings tests; add test-module docs and make server test time helpers fallible; reflow docs and enforce dependency-version guidance.

Changes

Whitaker Tool Integration

Layer / File(s)	Summary
CI workflow and local Makefile configuration .github/workflows/ci.yml, Makefile	CI sets WHITAKER_INSTALLER_REV, caches Whitaker artefacts by revision, installs/runs whitaker-installer on cache miss, and Makefile adds USER_WHITAKER, USER_BIN_PATH, WHITAKER lookup and runs whitaker from lint with augmented PATH and forwarded CARGO_FLAGS.
Developer documentation for local Whitaker setup docs/developers-guide.md, docs/adr-001-whitaker-lint-contract.md	Document pinned cargo install of whitaker-installer, running whitaker-installer --cranelift locally, and add ADR specifying Whitaker in the lint contract and CI install order.

Collection Name Interpolation Refactoring

Layer / File(s)	Summary
Placeholder detection and interpolation orchestration crates/dear-diary-config/src/interpolation/mod.rs	Introduce PlaceholderNeeds bitmask to detect supported placeholders ({repo}, {owner}, {cwd}, {branch}), add fast-path when none present, and extract remote/cwd/branch replacement helpers with unified unresolvable errors.
Remote URL path segmentation and parsing crates/dear-diary-config/src/interpolation/parse.rs	Add RemotePathSegments and remote_info_from_segments to parse remote paths, handle empty/single/multi-segment paths, strip leading ~ for Source Hut owners, and centralise error helpers.

Settings Tests and Test Utilities

Layer / File(s)	Summary
Extract settings tests and EnvGuard crates/dear-diary-config/src/settings.rs, crates/dear-diary-config/src/settings_tests.rs	Move inline tests to settings_tests.rs, add EnvGuard and mutex scaffolding, and add tests for defaults, Qdrant validation, filterable-field helpers, and Settings::from_env_with_git placeholder resolution success/failure.
Server test time utility refactor crates/dear-diary-mcp/src/server_tests.rs	Make current_timestamp() return Result<i64, SystemTimeError>, add duration_secs_i64(Duration) helper, and update tests to propagate errors and return Ok(()) explicitly.

Test Documentation, Dependency Guidance, and Docs Formatting

Layer / File(s)	Summary
Test module documentation comments crates/dear-diary-embeddings/src/fastembed.rs, crates/dear-diary-mcp/src/deprecation.rs, crates/dear-diary-mcp/src/server.rs, crates/dear-diary-qdrant/src/entry.rs	Add //! or doc comments to test modules describing the scope of unit tests across crates.
AGENTS dependency guidance and Markdown reflow AGENTS.md, multiple docs/*.md	Enforce caret (^) SemVer requirements in AGENTS.md, forbid * and >= specifiers except documented ~; reflow and align paragraphs, citations and tables across many documentation files without semantic changes.

Sequence Diagram(s):

sequenceDiagram
  participant CI as CI Workflow
  participant Cache as actions/cache
  participant Runner as GitHub runner
  participant Installer as whitaker-installer
  participant Make as Makefile lint

  CI->>Cache: restore cache(key=runner.os + WHITAKER_INSTALLER_REV)
  Cache-->>CI: cache hit / cache miss
  CI->>Runner: on cache miss -> cargo install whitaker-installer --rev $WHITAKER_INSTALLER_REV --locked
  CI->>Runner: run whitaker-installer --cranelift
  Make->>Make: resolve WHITAKER via PATH or USER_WHITAKER fallback
  Make->>Runner: run WHITAKER --all -- $(CARGO_FLAGS) with PATH augmented by USER_BIN_PATH

Loading

Possibly related PRs:

• leynos/stilyagi#13: Modify Makefile lint orchestration; related to adding tools to the lint target.

You are an AI reviewer. Summarise risks and call out any missing runtime behaviour tests or CI failure modes succinctly.

• Fail CI early if WHITAKER_INSTALLER_REV is unset or empty; validate before cache/restore steps.
• Verify cache key combines runner.os and WHITAKER_INSTALLER_REV to avoid stale artefacts.
• Add a CI job that exercises cache-miss path (cargo install + whitaker-installer --cranelift) to detect installer/runtime installation failures.
• Run make lint end-to-end in CI to validate WHITAKER resolution and PATH augmentation in runner shells.
• Add unit tests for remote single-segment and empty-path parsing to exercise RemotePathSegments error branches and message texts.
• Add a test verifying EnvGuard restores environment on panic to ensure drop semantics work under failure.

Poem:

Whitaker lands and runs the checks, 🛠️
Placeholders mapped with tidy checks,
Docs aligned and tests set free,
EnvGuard guards our registry,
Lint and parse and CI agree. ✨

🚥 Pre-merge checks | ✅ 18 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Observability	⚠️ Warning	PR enables Whitaker as required CI lint gate but adds no metrics or structured logging to track cache hit rates, lint failure patterns, or operational health.	Add metrics for cache hit/miss rates and Whitaker finding frequencies, or add structured logging with stable fields (cache status, findings count) to track operational effectiveness.
Concurrency And State	⚠️ Warning	EnvGuard uses static Mutex to serialise environment mutations, but tests run parallel by default; Mutex safety is not tested under actual parallel execution.	Document that these tests require sequential execution (--test-threads=1), or add stress/loom tests demonstrating Mutex safety under parallel execution.

✅ Passed checks (18 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Add Whitaker to lint checks' accurately summarises the main change: integrating Whitaker into the lint pipeline for local and CI linting.
Description check	✅ Passed	The description is comprehensive and directly relates to the changeset, detailing the Whitaker integration, workflow changes, code refactors, and validation results.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Testing (Overall)	✅ Passed	Refactored interpolation logic tested by 20 cases covering parsing, extraction, and errors. Settings tested by 7 cases validating defaults and configuration. Tests exercise behaviour, not just mocks.
User-Facing Documentation	✅ Passed	Whitaker is a development-only linting tool that does not change user-facing MCP server functionality. Documentation correctly placed in developers-guide.md and ADR, not users-guide.md.
Developer Documentation	✅ Passed	Whitaker tooling documented in developers-guide.md; ADR-001 records decision; AGENTS.md updated; Makefile and CI self-documenting; no roadmap or localisation sync required.
Module-Level Documentation	✅ Passed	All refactored and new Rust modules carry module-level documentation explaining purpose and function via //! docstrings.
Testing (Unit And Behavioural)	✅ Passed	Tests verify meaningful behaviour, edge cases, error paths, invariants with mocking. Integration tests exercise functional boundaries. Refactored helpers tested via public API appropriately.
Testing (Property / Proof)	✅ Passed	Refactored code introduces simple invariants with straightforward implementations; no complex state machines, ordering dependencies, or transitions requiring property-based testing.
Testing (Compile-Time / Ui)	✅ Passed	PR refactors runtime interpolation code with comprehensive rstest tests (20+) and new settings tests (7). No compile-time UI output or snapshot-testable structured data introduced.
Unit Architecture	✅ Passed	Refactoring preserves clear separation: queries expose fallibility via Result, dependencies injected as traits, mocks verify contracts, tests isolate environment state with RAII.
Domain Architecture	✅ Passed	GitContext trait abstracts git/filesystem concerns; domain errors avoid HTTP/SQL/ORM leakage; parsing split into helpers without allocations; test documentation isolates from public APIs.
Security And Privacy	✅ Passed	No secrets, injection risks, or privacy violations found. API keys handled safely, git commands use argument arrays, test fixtures use public mock data, CI permissions minimal.
Performance And Resource Use	✅ Passed	CI caching keys deterministic; RemotePathSegments avoids Vec allocation; no nested loops or repeated I/O; SystemTimeError propagates rather than masking errors.
Architectural Complexity And Maintainability	✅ Passed	Named helpers and PlaceholderNeeds reduce Whitaker-identified complexity. RemotePathSegments eliminates allocations. Helpers decompose branching. No speculative abstractions found.
Rust Compiler Lint Integrity	✅ Passed	No broad lint suppressions added. Four #[expect(...)] attributes are narrow, justified, and temporary. All refactored helpers properly invoked. New test module contains only genuine test functions.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch adopt-whitaker-lints

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[sourcery-ai]

Reviewer's Guide

Integrates the Whitaker Dylint suite into the standard lint contract (Makefile + CI), documents how to install/run it locally, refactors interpolation and remote URL parsing into smaller helpers to satisfy new complexity lints, and performs minor test and markdown cleanups triggered by the new linting.

File-Level Changes

Change	Details	Files
Wire Whitaker into local and CI lint flows alongside Rustdoc and Clippy.	Add WHITAKER_INSTALLER_REV env var and a dedicated Whitaker installer step to the CI workflow before running make lint. Extend the Makefile lint target to discover a Whitaker binary (from PATH or ~/.local/bin) and invoke it with the configured Rust flags and cargo flags. Add developer documentation explaining how to install the pinned whitaker-installer revision and run make lint locally.	.github/workflows/ci.yml Makefile docs/developers-guide.md
Refactor interpolation placeholder handling to reduce complexity while preserving behavior.	Introduce PlaceholderNeeds bitflags and placeholder_flag to pre-scan templates for supported placeholders in a single pass. Extract remote-derived interpolation logic into replace_remote_placeholders and remote_owner helpers, reusing parse_remote_url and centralizing error construction. Extract working-directory and branch substitution into replace_cwd_placeholder and replace_branch_placeholder helpers, with a dedicated unresolvable_branch_placeholder error builder.	crates/dear-diary-config/src/interpolation/mod.rs
Simplify remote URL parsing by factoring path-segmentation and RemoteInfo construction into dedicated helpers.	Split path-cleanup and segmentation into remote_path_segments, returning the non-empty path components as a Vec<&str>. Introduce remote_info_from_segments to map segment slices into RemoteInfo, handling 0, 1, and 2+ segment cases and Source Hut owner tilde stripping. Have parse_remote_url delegate to these helpers after scheme/host normalization and .git suffix stripping.	crates/dear-diary-config/src/interpolation/parse.rs
Tighten test utilities and add module-level test documentation comments to satisfy Whitaker/test-module lints.	Refactor current_timestamp to use a reusable duration_secs_i64 helper for Duration-to-i64 conversion in MCP server tests, with an expect on the cast in the helper. Add //!-style module docs describing test intent in several #[cfg(test)] modules across config, embeddings, MCP, and Qdrant crates.	crates/dear-diary-mcp/src/server_tests.rs crates/dear-diary-config/src/settings.rs crates/dear-diary-embeddings/src/fastembed.rs crates/dear-diary-mcp/src/deprecation.rs crates/dear-diary-mcp/src/server.rs crates/dear-diary-qdrant/src/entry.rs
Apply repository markdown formatting, line-wrapping, and table-alignment fixes.	Reflow long lines and footnote references to satisfy markdownlint in complexity and doctest guides and other docs. Align and narrow markdown tables for rstest attributes and deprecation states. Adjust wording layout and line breaks in various docs to conform to the repository formatter and MD013 expectations.	AGENTS.md docs/complexity-antipatterns-and-refactoring-strategies.md docs/ortho-config-users-guide.md docs/release-process.md docs/reliable-testing-in-rust-via-dependency-injection.md docs/rust-testing-with-rstest-fixtures.md docs/rust-doctest-dry-guide.md docs/rstest-bdd-users-guide.md docs/users-guide.md

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[leynos]

@coderabbitai Have the following now been resolved?

Annul any requirements that violate the en-GB-oxendict spelling (-ize / -yse / -our) conventions (for example a request to replace "normalize" with "normalise" or "artefact" with "artifact"), or where the requirement unnecessarily increases cyclomatic complexity.

Please address the comments from this code review:

## Overall Comments
- In `current_timestamp`, the new `map_or(0, duration_secs_i64)` behavior will silently treat any `SystemTime`/`UNIX_EPOCH` error as timestamp 0; consider preserving the previous explicit failure (e.g., via `expect` or propagating a `Result`) so tests don’t accidentally pass with a sentinel value.
- The refactor in `parse_remote_url` now allocates a `Vec<&str>` via `remote_path_segments`; if this code is on a hot path, you could retain the previous single-pass iterator approach (or use an iterator-based helper) to avoid the extra allocation while keeping the clearer `match`-style logic.

## Individual Comments

### Comment 1
<location path=".github/workflows/ci.yml" line_range="19" />
<code_context>
         uses: astral-sh/setup-uv@37802adc94f370d6bfd71619e3f0bf239e1f3b78
       - name: Script tests
         run: make test-scripts
+      - name: Install Whitaker
+        run: |
+          cargo install --locked \
+            --git https://github.com/leynos/whitaker \
+            --rev "${WHITAKER_INSTALLER_REV}" \
+            whitaker-installer
+          whitaker-installer --cranelift
       - name: Lint
         run: make lint
</code_context>
<issue_to_address>
**suggestion (performance):** It may be useful to cache the Whitaker installation to avoid repeated compilation on CI runs.

Installing `whitaker-installer` from git on every CI run can be relatively expensive, particularly if it pulls extra dependencies. If CI time becomes a concern, consider adding a cache keyed on the installer rev and relevant toolchain inputs so Whitaker is only built and installed when those change.

```suggestion
      - name: Setup Rust
        uses: astral-sh/setup-uv@37802adc94f370d6bfd71619e3f0bf239e1f3b78
      - name: Cache Whitaker installation
        id: cache-whitaker
        uses: actions/cache@v4
        with:
          path: |
            ~/.cargo/bin/whitaker
            ~/.cargo/bin/whitaker-installer
          key: whitaker-${{ runner.os }}-${{ env.WHITAKER_INSTALLER_REV }}
      - name: Script tests
        run: make test-scripts
      - name: Install Whitaker
        if: steps.cache-whitaker.outputs.cache-hit != 'true'
        run: |
          cargo install --locked \
            --git https://github.com/leynos/whitaker \
            --rev "${WHITAKER_INSTALLER_REV}" \
            whitaker-installer
          whitaker-installer --cranelift
```
</issue_to_address>

### Comment 2
<location path="crates/dear-diary-mcp/src/server_tests.rs" line_range="25-33" />
<code_context>

 /// Helper to get current Unix timestamp for tests.
 fn current_timestamp() -> i64 {
-    #[expect(clippy::cast_possible_wrap, reason = "Unix timestamp fits in i64")]
     SystemTime::now()
         .duration_since(UNIX_EPOCH)
-        .map(|d| d.as_secs() as i64)
-        .expect("System time should be after Unix epoch")
+        .map_or(0, duration_secs_i64)
+}
+
+/// Converts a Unix timestamp duration into signed seconds for test fixtures.
+#[expect(clippy::cast_possible_wrap, reason = "Unix timestamp fits in i64")]
+fn duration_secs_i64(duration: Duration) -> i64 {
+    duration.as_secs() as i64
 }
</code_context>
<issue_to_address>
**issue (bug_risk):** Using `map_or(0, ...)` risks hiding `SystemTime` errors and making tests misleadingly pass

With this change, any `SystemTime` error is now converted to a `0` timestamp, so tests that rely on `current_timestamp()` may silently pass with an invalid time instead of failing. For test helpers, it’s usually better to fail loudly on unexpected conditions. Consider keeping the previous behavior:

```rust
fn current_timestamp() -> i64 {
    SystemTime::now()
        .duration_since(UNIX_EPOCH)
        .map(duration_secs_i64)
        .expect("System time should be after Unix epoch")
}
```
</issue_to_address>

### Comment 3
<location path="crates/dear-diary-config/src/interpolation/mod.rs" line_range="24" />
<code_context>
 use crate::error::ConfigError;
 use parse::parse_remote_url;

+/// Tracks which interpolation placeholders appear in a template.
+struct PlaceholderNeeds(u8);
+
</code_context>
<issue_to_address>
**issue (complexity):** Consider replacing the custom bitflag-based `PlaceholderNeeds(u8)` API with a straightforward struct of booleans to track placeholder usage while keeping the new helper structure intact.

You can keep the new high‑level structure while dropping most of the indirection by replacing the bitflag mini‑API with a simple struct of booleans.

Right now `PlaceholderNeeds(u8)` + `has` + `placeholder_flag` add bit‑twiddling and an extra helper without actually buying you anything over explicit booleans. A small struct keeps the “single scan” behaviour and keeps call sites readable, but removes the need to mentally decode flags.

For example:

```rust
/// Tracks which interpolation placeholders appear in a template.
struct PlaceholderNeeds {
    repo: bool,
    owner: bool,
    cwd: bool,
    branch: bool,
}

impl PlaceholderNeeds {
    fn from_template(template: &str) -> Self {
        Self {
            repo: template.contains("{repo}"),
            owner: template.contains("{owner}"),
            cwd: template.contains("{cwd}"),
            branch: template.contains("{branch}"),
        }
    }

    fn is_empty(&self) -> bool {
        !self.repo && !self.owner && !self.cwd && !self.branch
    }

    fn needs_remote(&self) -> bool {
        self.repo || self.owner
    }

    fn needs_repo(&self) -> bool {
        self.repo
    }

    fn needs_owner(&self) -> bool {
        self.owner
    }

    fn needs_cwd(&self) -> bool {
        self.cwd
    }

    fn needs_branch(&self) -> bool {
        self.branch
    }
}
```

Then the rest of your new structure stays the same, but becomes simpler at the call sites:

```rust
pub fn interpolate_collection_name(
    template: &str,
    git: &impl GitContext,
) -> Result<String, ConfigError> {
    let needs = PlaceholderNeeds::from_template(template);

    if needs.is_empty() {
        return Ok(template.to_owned());
    }

    let mut result = template.to_owned();

    if needs.needs_remote() {
        result = replace_remote_placeholders(result, git, &needs)?;
    }

    result = replace_cwd_placeholder(result, git, &needs)?;
    result = replace_branch_placeholder(result, git, &needs)?;

    Ok(result)
}
```

And the helpers remain unchanged, just using the new fields/methods:

```rust
fn replace_cwd_placeholder(
    result: String,
    git: &impl GitContext,
    needs: &PlaceholderNeeds,
) -> Result<String, ConfigError> {
    if needs.needs_cwd() {
        let basename = git.cwd_basename()?;
        Ok(result.replace("{cwd}", &basename))
    } else {
        Ok(result)
    }
}
```

This keeps your new decomposition (`replace_remote_placeholders`, etc.) and single `needs` object, but removes the custom bitflag machinery and the extra `placeholder_flag` helper, which should address most of the complexity concerns without changing behaviour.
</issue_to_address>

### Comment 4
<location path="crates/dear-diary-config/src/interpolation/parse.rs" line_range="67" />
<code_context>
-    match segment_count {
-        0 => Err(ConfigError::InterpolationContextError(format!(
+/// Builds remote metadata from parsed path segments.
+fn remote_info_from_segments(raw_url: &str, segments: &[&str]) -> Result<RemoteInfo, ConfigError> {
+    match segments {
+        [] => Err(ConfigError::InterpolationContextError(format!(
</code_context>
<issue_to_address>
**issue (review_instructions):** `remote_info_from_segments` still contains a 3-branch `match`; per the guidance, the branching logic should be expressed via predicate helper(s) rather than directly in this function.

The `match segments { [] => ..., [repo] => ..., [owner_raw, .., repo] => ... }` introduces three branches directly in `remote_info_from_segments`. The instructions call for conditionals with more than two branches to be moved into predicate functions.

Here that could mean introducing small helpers such as `is_empty_path(segments)`, `is_single_segment(segments)`, etc., and using those to drive the control flow, or otherwise encapsulating the branching logic into dedicated predicate-style helpers so this function focuses on building `RemoteInfo` given those predicates.

<details>
<summary>Review instructions:</summary>

**Path patterns:** `**/*.rs`

**Instructions:**
Move conditionals with >2 branches into a predicate function.

</details>
</issue_to_address>

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

Let me inspect the current state of the relevant files.

---

🏁 Script executed:

#!/bin/bash
echo "=== .github/workflows/ci.yml ==="
cat .github/workflows/ci.yml

echo ""
echo "=== server_tests.rs ==="
cat crates/dear-diary-mcp/src/server_tests.rs

echo ""
echo "=== interpolation/mod.rs ==="
cat crates/dear-diary-config/src/interpolation/mod.rs

echo ""
echo "=== interpolation/parse.rs ==="
cat crates/dear-diary-config/src/interpolation/parse.rs

Length of output: 28322

---

@leynos: Yes — all comments have been addressed. Here is a full accounting:

---

Overall Comment 1 — current_timestamp silent zero

✅ Resolved. current_timestamp() now returns Result<i64, SystemTimeError> and uses .map(duration_secs_i64), propagating any system-time error to callers rather than swallowing it as 0.

---

Overall Comment 2 — Vec<&str> allocation in parse_remote_url

✅ Resolved. RemotePathSegments::from_path performs a single-pass scan tracking first, last, and count without allocating a Vec. The previous allocation concern is gone.

---

Comment 1 — CI caching for Whitaker

✅ Resolved. The Cache Whitaker installation step uses actions/cache@v4 keyed on WHITAKER_INSTALLER_REV, and the install step is gated on cache-hit != 'true'. The cached paths also include ~/.dylint_drivers and ~/.local/share/whitaker, which goes beyond the original suggestion.

---

Comment 2 — map_or(0, …) bug risk

✅ Resolved. Same change as Overall Comment 1 above.

---

Comment 3 — PlaceholderNeeds(u8) bitflag complexity

✅ Resolved. The bitflag type is replaced with nested structs of booleans — RemotePlaceholderNeeds { repo, owner } grouped with LocalPlaceholderNeeds { cwd, branch } under PlaceholderNeeds. All predicate methods (needs_remote(), needs_repo(), etc.) delegate to plain boolean fields.

---

Comment 4 — 3-branch match in remote_info_from_segments

✅ Resolved. The 3-branch match is replaced with if segments.is_empty() / if segments.is_single_segment() predicate methods defined on RemotePathSegments, with the multi-segment case as the fall-through. This satisfies the coding guideline without adding gratuitous indirection.

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: bbe9d517-5c83-4073-82d7-ee2217bfc578

📥 Commits

Reviewing files that changed from the base of the PR and between 65416f3 and c0a1cab.

📒 Files selected for processing (1)

• .github/workflows/ci.yml