# CLAUDE.md compliance — implementation summary

Plan: `docs/plans/transient-baking-clock.md` (four chunks). All four completed.

## Chunk 1 — Comments stripped

314 C# comment lines and 53 web comment lines removed across ~40 files (see plan for the
file list). Exemptions honored: `Novelly.ServiceDefaults/Extensions.cs` (Aspire scaffold),
`vite-env.d.ts` (required TS directive). One file the original audit missed and this pass
caught: `src/Novelly.Web/vite.config.ts` had a two-line comment on the dev proxy — removed.

Added `scripts/ci/check-no-comments.sh`, wired into `scripts/ci/prepush.sh` before the
build. Grep-based trip-wire (excludes `Data/Migrations/`, the two exempted files,
`node_modules/dist/bin/obj`, and skips `https://` matches) — verified it catches real
violations and passes on the cleaned tree.

## Chunk 2 — `else` removed, `INovelDbContext` folded

- Added `AddRequiredTextErrors` / `AddOptionalTextErrors` / `AddUnclearableTextErrors` to
  `ValidationResultExtensions` and pointed all `*Contracts.cs` validators at them, collapsing
  ~18 `if/else if` blocks into one-line calls.
- Converted the 7 flagged service-level `else` sites to early return, ternary, or
  `logger.Log(level, ...)` with a ternary `LogLevel` argument (used for the several
  "two log calls, different level" sites — a value pick, not a branch).
- `INovelDbContext` moved to the bottom of `NovelDbContext.cs`; `INovelDbContext.cs` deleted.

## Chunk 3 — Logging: Warning on rejection/not-found paths

- ~20 rejection/not-found sites flipped from `LogInformation` to `LogWarning` across
  `ProjectService`, `ChapterService`, `BeatService`, `CharacterService`,
  `CharacterArcService`, `TagService`, `OpenQuestionService`, `NovelAgentService`.
- Agent tool outcomes (`NovelAgentService`, `ImportAgentService`) now log at Warning when
  `outcome.IsError`, Information otherwise.
- `ToolNotFound` / `ImportToolNotFound` records changed from a single pre-formatted
  `Message` string to `(Entity, Id)`, with `Message` as a computed property — the tool-not-found
  log now carries `{Entity}`/`{EntityId}` as separate structured properties instead of an
  interpolated string.
- `ThrowIfInvalid` gained an `ILogger` overload that logs Warning with the failing field
  names before throwing; every service's `Validate(...).ThrowIfInvalid()` call now passes
  `logger`, so a validation rejection reaching a service directly (agent tools, MCP, tests)
  is no longer silent.
- Added missing Debug end-logs: `CharacterArcService.EnsureChapterIsInSameProjectAsync` /
  `NextSortOrderAsync`, `BeatService.ResolveCharactersAsync` / `NextSortOrderAsync`,
  `OpenQuestionService.ValidateAssociationsAsync`, `NovelAgentService.AppendMessageAsync` /
  `StartConversation` / `FindConversationAsync`, `ImportAgentService.RunOneTurnAsync`.
- Added the missing entry Information log to `ImportAgentService.RunAsync`.
- `NovelApiClient` (MCP) now takes `ILogger<NovelApiClient>` via DI; the swallowed
  `HttpRequestException` now logs Error, and the two swallowed `JsonException` catches
  (`Prettify`, `TryReadProblemDetail`, both changed from `static` to instance methods) now
  log Warning instead of silently falling back.
- `ImportPaths.ResolveRoot` no longer drops the original exception — it's passed as
  `innerException` on the rethrown `ArgumentException`. (Left un-logged: `ImportPaths` is a
  static utility with no `ILogger`; threading one through would be a bigger change than this
  pass's scope, and the exception is still visible to whichever caller has a logger.)
- `Program.cs`'s exception-handler `else` collapsed into one `logger.Log(level, ...)` call
  driven by the 500-vs-handled ternary.
- `LoggingTests.cs`: the one test that asserted the *old* (rule-violating) behavior —
  "missing chapter logs at Information, not Warning" — renamed and rewritten to assert the
  corrected Warning-level behavior. All other tests were unaffected.

## Chunk 4 — Coverage config, gate, and two new test files

- Added `[ExcludeFromCodeCoverage]` to `Program` and `NovellyServiceRegistration`
  (composition roots, not unit-testable).
- Added `coverlet.msbuild` alongside the existing `coverlet.collector` package reference.
  **Correction to the plan**: a `.runsettings` file with `<Threshold>` under the
  `XPlat Code Coverage` *collector* does not actually fail the run — verified directly
  (`dotnet test --collect:"XPlat Code Coverage" --settings .runsettings` exited 0 at 54.8%
  coverage, well under the configured 70% floor). Deleted the `.runsettings` file rather than
  ship a threshold that silently does nothing. `coverlet.msbuild`'s `/p:Threshold` *does*
  enforce (verified: exits 1 below the threshold, 0 at or above it), so `prepush.sh` now gates
  through MSBuild properties instead.
- Added `ChapterServiceTests.cs` and `CharacterServiceTests.cs` (previously the two largest
  services with no owning test file). Both use the existing `ServiceTestFixture` /
  `TestDatabase` pattern, BDD-named, covering create/patch-semantics/delete, the
  cross-project relationship rejection, and cascade behavior (chapter delete takes its beats;
  character delete detaches from beats without deleting them).
- Grouped the two genuinely-adjacent bare `Assert.That` pairs in `ImportServiceTests.cs`
  (`:96-97`, `:108-109`) into `Assert.Multiple`. Left the other flagged sites alone — each
  interleaves an act step between asserts, so grouping them would obscure ordering rather
  than clarify it.

**Coverage floor set to 60, not 70.** Real `Novelly.Api` line coverage after the two new
test files is 60.53%, not 70% — the gap is concentrated in code the plan explicitly put out
of scope: every `*Endpoints.cs` file and the two endpoint filters
(`RequestLoggingEndpointFilter`, `ValidationEndpointFilter`) have zero tests, since there's
no `WebApplicationFactory`/`HttpClient` integration-test setup in the project yet. Setting
the gate to 70 today would fail every push until that follow-up work lands, which is a
worse outcome than a lower floor that (a) still catches regressions and (b) is honest about
where the codebase actually stands. Endpoint/integration tests remain the deliberately
deferred next chunk — building the `WebApplicationFactory` harness and covering the five
endpoint groups would very likely clear 70 on its own.

## Verification

- `dotnet build Novelly.slnx -c Debug` — succeeds (pre-existing nullable warnings only,
  unrelated to this change).
- `dotnet test tests/Novelly.Api.Tests/Novelly.Api.Tests.csproj -c Debug` — 114/114 pass
  (100 existing + 14 new, one existing test rewritten as noted above).
- `npm --prefix src/Novelly.Web run build` — succeeds.
- `scripts/ci/check-no-comments.sh` — passes on the cleaned tree, was verified to fail before
  the `vite.config.ts` fix.
- `bash scripts/ci/prepush.sh` — full run end to end, exits 0 (build, comment guard,
  coverage-gated tests, web build).

**Not run**: the app itself (Aspire host / API / MCP stdio) was not exercised live in this
pass — the changes here are logging levels, validation-message plumbing, and test additions,
not endpoint or protocol-shape changes, so build+test coverage was judged sufficient. Worth a
live smoke pass before merging if the reviewer wants to see the corrected log levels in the
Aspire dashboard.