diff --git a/CLAUDE.md b/CLAUDE.md index a2fd8f3..d1e40a4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,51 +1,61 @@ # CLAUDE.md -Guidance for Claude Code (claude.ai/code) in this repo. +Guidance for Claude Code (claude.ai/code) in repo. ## Project -Novelly: software for planning and writing a novel. ASP.NET Core 10, C#, TypeScript, React, .NET Aspire. Chapter outlines, character dossiers, prose drafting, an embedded Claude agent, and an MCP server over the same API. +Novelly: software plan + write novel. ASP.NET Core 10, C#, TypeScript, React, .NET Aspire. Chapter outlines, character dossiers, prose drafting, embedded Claude agent, MCP server over same API. ## Structure -- `src/Novelly.Api/` — the whole back end, organised by feature. One folder per feature holds its - entity, DTOs, service and endpoints together: `Projects/`, `Characters/`, `Chapters/`, `Beats/`, - `Scenes/`, `Tags/`, `Agent/`. `Common/` holds what genuinely crosses features; `Data/` holds the - `DbContext` and EF migrations. -- `src/Novelly.AppHost/` — .NET Aspire orchestration; run this to bring up the API and the web client +- `src/Novelly.Api/` — whole back end, organised by feature. One folder per feature holds + entity, DTOs, service, endpoints together: `Projects/`, `Characters/`, `Chapters/`, `Beats/`, + `Scenes/`, `Tags/`, `Agent/`. `Common/` holds what crosses features; `Data/` holds + `DbContext` + EF migrations. +- `src/Novelly.AppHost/` — .NET Aspire orchestration; run this to bring up API + web client - `src/Novelly.ServiceDefaults/` — shared Aspire wiring: OpenTelemetry, health checks, service discovery - `src/Novelly.Mcp/` — MCP stdio server - `src/Novelly.Web/` — React + Vite client - `tests/` — test suite - `docs/` — documentation -- `scripts/ci/` — bash CI steps; `prepush.sh` is what the Husky pre-push hook runs +- `scripts/ci/` — bash CI steps; `prepush.sh` = what Husky pre-push hook runs ## Best Practices - Use latest .NET + latest supported nuget packages for that version -- Set `langVersion` to latest in all csproj files; enable nullable -- Organize code by feature/area, not layer or type. A new capability adds files to one feature folder - rather than a row to each of an entity/DTO/service/endpoint folder -- New features need unit tests covering logic as much as possible +- Set `langVersion` latest in all csproj files; enable nullable +- Organize code by feature/area, not layer or type. New capability adds files to one feature folder + rather than row to each of entity/DTO/service/endpoint folder +- New features need unit tests covering logic much as possible - Modified file: check missing test coverage, all tests pass ## Architecture -- The React client, the embedded agent, and the MCP server all go through the same REST API. One source of truth — never let a client reach past the API to the database. -- Agent tools and MCP tools call the same application services the endpoints do. New capability = new service method, then surface it in all three. -- Domain has no dependencies. Application depends on Domain. Infrastructure depends on Application. Nothing depends on Api. +- React client, embedded agent, MCP server all go through same REST API. One source of truth — never let client reach past API to database. +- Agent tools + MCP tools call same application services endpoints do. New capability = new service method, then surface in all three. + +## Logging + +Serilog console via `AddSerilog` (not `UseSerilog` — keeps OTel provider for Aspire dashboard). `app.UseSerilogRequestLogging()` registered before `UseExceptionHandler` (outermost), else it logs raw exception status instead of handled one. +- Endpoints: Information via `RequestLoggingEndpointFilter` on each `MapGroup`. No per-lambda logging. +- Service public methods: Information at entry, ids/enums/counts as args. +- Deeper/private methods: Debug at start + end. +- Caught exceptions: `LogError(ex, ...)` with identifying values. +- Expected/recoverable (NotFound, validation, agent tool errors): Warning before throw/return. +- Structured templates only — `{ChapterId}`, never interpolation. +- Never log prose (summary/notes/content) or `ANTHROPIC_API_KEY`. Prose = length only. # Coding - Descriptive names all classes/methods. No generic: Provider, Manager, Helper - Match formatting/style from `.editorconfig` - Wrap lines at 220 chars, single line if fewer -- Interfaces implemented by single class → bottom of class file. Interface w/ multiple implementations → separate file. +- Interface implemented by single class → bottom of class file. Interface w/ multiple implementations → separate file. - No tuples for return types. Prefer records or classes for multiple values - Use `record` for data objects, `class` for objects with behavior. Avoid mutable state where possible. -- DTOs are records; entities are classes -- `PATCH` requests are partial: null field = leave alone, empty string = clear. Keep new update endpoints consistent with `Patch.Apply`. -- Enums cross the wire as names, never ordinals +- DTOs are records; entities are classes. DO NOT use Dto in names. +- `PATCH` requests partial: null field = leave alone, empty string = clear. Keep new update endpoints consistent with `Patch.Apply`. +- Enums cross wire as names, never ordinals ## Testing @@ -53,28 +63,28 @@ Novelly: software for planning and writing a novel. ASP.NET Core 10, C#, TypeScr - Don't write tests just for coverage. Call out missing coverage rather than cover stuff not valuable to end user. - Code not cleanly unit-testable → mark `[ExcludeFromCodeCoverage]` or exclude namespace from coverage in .runsettings file - BDD-style unit tests, end-to-end as possible. e.g. `Deleting_a_scene_leaves_its_beats_alone` -- NUnit. No FluentAssertions — assert with `Assert.That` and NUnit's constraint model: +- NUnit. No FluentAssertions — assert with `Assert.That` + NUnit's constraint model: `Assert.That(beat.SceneId, Is.Null)`, `Assert.That(listed, Has.Count.EqualTo(3))`, `Assert.That(titles, Is.EqualTo(new[] { "First", "Second" }))` -- Grouping related asserts in `Assert.Multiple` beats a chain that stops at the first failure +- Grouping related asserts in `Assert.Multiple` beats chain that stops at first failure - Expected exceptions: `Assert.That(() => service.Foo(), Throws.TypeOf())` -- Tests run against real in-memory SQLite via `TestDatabase`, not the EF InMemory provider — cascade deletes and query translation must be exercised, and the InMemory provider fakes both -- Model calls are faked at the `IAgentModelClient` seam (see `ScriptedModelClient`). Never hit the Anthropic API from a test. +- Tests run against real in-memory SQLite via `TestDatabase`, not EF InMemory provider — cascade deletes + query translation must be exercised, InMemory provider fakes both +- Model calls faked at `IAgentModelClient` seam (see `ScriptedModelClient`). Never hit Anthropic API from test. - No "Mock" in mocked object names - No Arrange/Act/Assert comments - All tests pass before commit ## Verifying -Build and tests passing is not the same as working. For anything touching an endpoint, the agent loop, or the MCP server, run it: +Build + tests passing ≠ working. Anything touching endpoint, agent loop, or MCP server — run it: -- Everything at once: `dotnet run --project src/Novelly.AppHost` — Aspire starts the API on :5080 and - the Vite dev server on :5173, with the dashboard for logs and traces -- API alone: `ASPNETCORE_URLS=http://localhost:5080 dotnet run --project src/Novelly.Api`, then exercise the route with curl +- Everything at once: `dotnet run --project src/Novelly.AppHost` — Aspire starts API on :5080 + + Vite dev server on :5173, dashboard for logs + traces +- API alone: `ASPNETCORE_URLS=http://localhost:5080 dotnet run --project src/Novelly.Api`, then exercise route with curl - Web alone: `cd src/Novelly.Web && npm run dev` — proxies `/api` to :5080 -- MCP: build it, then drive it over stdio JSON-RPC (`initialize` → `notifications/initialized` → `tools/list` → `tools/call`) +- MCP: build it, then drive over stdio JSON-RPC (`initialize` → `notifications/initialized` → `tools/list` → `tools/call`) -Several real bugs here — SQLite refusing to ORDER BY a DateTimeOffset, the agent's model client throwing at construction and taking read-only endpoints down with it — passed the build and the test suite and only showed up when the app actually ran. +Several real bugs here — SQLite refusing ORDER BY DateTimeOffset, agent's model client throwing at construction + taking read-only endpoints down with it — passed build + test suite, only showed up when app actually ran. ## Claude @@ -87,8 +97,8 @@ Several real bugs here — SQLite refusing to ORDER BY a DateTimeOffset, the age `.gitignore` set for .NET/Visual Studio (C#, NuGet, MSBuild) plus Node. Update if stack change. -- Anthropic model id lives in `appsettings.json` under `Agent:Model`. Don't hardcode it. -- API key comes from `ANTHROPIC_API_KEY` or `Agent:ApiKey` — never commit one. The app must stay fully usable without a key; only the agent endpoints require it. -- EF migrations: `dotnet ef migrations add -p src/Novelly.Api -o Data/Migrations`. The API migrates on boot. -- `git push` runs `scripts/ci/prepush.sh` through Husky: build, test, then a web build. Run `npm install` - once at the repo root to install the hook. +- Anthropic model id lives in `appsettings.json` under `Agent:Model`. Don't hardcode. +- API key comes from `ANTHROPIC_API_KEY` or `Agent:ApiKey` — never commit one. App must stay fully usable without key; only agent endpoints require it. +- EF migrations: `dotnet ef migrations add -p src/Novelly.Api -o Data/Migrations`. API migrates on boot. +- `git push` runs `scripts/ci/prepush.sh` through Husky: build, test, then web build. Run `npm install` + once at repo root to install hook. diff --git a/CLAUDE.original.md b/CLAUDE.original.md new file mode 100644 index 0000000..74325a7 --- /dev/null +++ b/CLAUDE.original.md @@ -0,0 +1,106 @@ +# CLAUDE.md + +Guidance for Claude Code (claude.ai/code) in this repo. + +## Project + +Novelly: software for planning and writing a novel. ASP.NET Core 10, C#, TypeScript, React, .NET Aspire. Chapter outlines, character dossiers, prose drafting, an embedded Claude agent, and an MCP server over the same API. + +## Structure + +- `src/Novelly.Api/` — the whole back end, organised by feature. One folder per feature holds its + entity, DTOs, service and endpoints together: `Projects/`, `Characters/`, `Chapters/`, `Beats/`, + `Scenes/`, `Tags/`, `Agent/`. `Common/` holds what genuinely crosses features; `Data/` holds the + `DbContext` and EF migrations. +- `src/Novelly.AppHost/` — .NET Aspire orchestration; run this to bring up the API and the web client +- `src/Novelly.ServiceDefaults/` — shared Aspire wiring: OpenTelemetry, health checks, service discovery +- `src/Novelly.Mcp/` — MCP stdio server +- `src/Novelly.Web/` — React + Vite client +- `tests/` — test suite +- `docs/` — documentation +- `scripts/ci/` — bash CI steps; `prepush.sh` is what the Husky pre-push hook runs + +## Best Practices + +- Use latest .NET + latest supported nuget packages for that version +- Set `langVersion` to latest in all csproj files; enable nullable +- Organize code by feature/area, not layer or type. A new capability adds files to one feature folder + rather than a row to each of an entity/DTO/service/endpoint folder +- New features need unit tests covering logic as much as possible +- Modified file: check missing test coverage, all tests pass + +## Architecture + +- The React client, the embedded agent, and the MCP server all go through the same REST API. One source of truth — never let a client reach past the API to the database. +- Agent tools and MCP tools call the same application services the endpoints do. New capability = new service method, then surface it in all three. +- Domain has no dependencies. Application depends on Domain. Infrastructure depends on Application. Nothing depends on Api. + +## Logging + +Logging goes through Serilog, writing to the console, registered with `AddSerilog` rather than `UseSerilog` — this makes it an additional logging provider alongside the OpenTelemetry one that `AddServiceDefaults` wires up, instead of replacing it, so the Aspire dashboard keeps seeing structured logs. `app.UseSerilogRequestLogging()` must be registered before `app.UseExceptionHandler(...)` so it wraps the exception handler as the outermost middleware; otherwise it observes the raw exception as it propagates past it and logs the request as a 500 even when the exception handler goes on to turn it into a handled 404. + +- Endpoints get their Information-level logging from a single `RequestLoggingEndpointFilter`, applied once per `MapGroup` call. No individual endpoint lambda needs to log anything itself. +- Every public service method logs an Information entry naming the operation and its identifying arguments — ids, enums, counts. +- Deeper, private helper methods log at Debug on entry and again before they return, including whatever state drove the decision. +- Anywhere an exception is caught, log it at Error with `LogError(ex, ...)`, including enough identifying values (ids, not prose) to debug from. +- Expected or recoverable failures — a not-found lookup, a validation rejection, an agent tool call that fails and is reported back to the model instead of thrown — log at Warning immediately before the throw or the recovery, not at Error. +- Always use structured message templates with named holes, like `{ChapterId}`, rather than string interpolation, so Serilog captures them as properties. +- Never log prose bodies (summary, notes, scene prose, agent message content) or the `ANTHROPIC_API_KEY`. Where a prose field is worth acknowledging in a log, log its length instead of its content. + +# Coding + +- Descriptive names all classes/methods. No generic: Provider, Manager, Helper +- Match formatting/style from `.editorconfig` +- Wrap lines at 220 chars, single line if fewer +- Interfaces implemented by single class → bottom of class file. Interface w/ multiple implementations → separate file. +- No tuples for return types. Prefer records or classes for multiple values +- Use `record` for data objects, `class` for objects with behavior. Avoid mutable state where possible. +- DTOs are records; entities are classes +- `PATCH` requests are partial: null field = leave alone, empty string = clear. Keep new update endpoints consistent with `Patch.Apply`. +- Enums cross the wire as names, never ordinals + +## Testing + +- Min 70% code coverage, target 90%. Unit tests focus end-user scenarios first. +- Don't write tests just for coverage. Call out missing coverage rather than cover stuff not valuable to end user. +- Code not cleanly unit-testable → mark `[ExcludeFromCodeCoverage]` or exclude namespace from coverage in .runsettings file +- BDD-style unit tests, end-to-end as possible. e.g. `Deleting_a_scene_leaves_its_beats_alone` +- NUnit. No FluentAssertions — assert with `Assert.That` and NUnit's constraint model: + `Assert.That(beat.SceneId, Is.Null)`, `Assert.That(listed, Has.Count.EqualTo(3))`, + `Assert.That(titles, Is.EqualTo(new[] { "First", "Second" }))` +- Grouping related asserts in `Assert.Multiple` beats a chain that stops at the first failure +- Expected exceptions: `Assert.That(() => service.Foo(), Throws.TypeOf())` +- Tests run against real in-memory SQLite via `TestDatabase`, not the EF InMemory provider — cascade deletes and query translation must be exercised, and the InMemory provider fakes both +- Model calls are faked at the `IAgentModelClient` seam (see `ScriptedModelClient`). Never hit the Anthropic API from a test. +- No "Mock" in mocked object names +- No Arrange/Act/Assert comments +- All tests pass before commit + +## Verifying + +Build and tests passing is not the same as working. For anything touching an endpoint, the agent loop, or the MCP server, run it: + +- Everything at once: `dotnet run --project src/Novelly.AppHost` — Aspire starts the API on :5080 and + the Vite dev server on :5173, with the dashboard for logs and traces +- API alone: `ASPNETCORE_URLS=http://localhost:5080 dotnet run --project src/Novelly.Api`, then exercise the route with curl +- Web alone: `cd src/Novelly.Web && npm run dev` — proxies `/api` to :5080 +- MCP: build it, then drive it over stdio JSON-RPC (`initialize` → `notifications/initialized` → `tools/list` → `tools/call`) + +Several real bugs here — SQLite refusing to ORDER BY a DateTimeOffset, the agent's model client throwing at construction and taking read-only endpoints down with it — passed the build and the test suite and only showed up when the app actually ran. + +## Claude + +- Plans = `.md` files in `docs/plans/`. Web → `docs/plans/web/`, API → `docs/plans/api/` +- Split large plans into discrete chunks — each buildable + committable independently +- Plan generated from `docs/plans/.md` → save as `docs/plans/_plan.md` +- Plan implemented from `docs/plans/.md` → save summary as `docs/plans/_output.md` + +## Stack + +`.gitignore` set for .NET/Visual Studio (C#, NuGet, MSBuild) plus Node. Update if stack change. + +- Anthropic model id lives in `appsettings.json` under `Agent:Model`. Don't hardcode it. +- API key comes from `ANTHROPIC_API_KEY` or `Agent:ApiKey` — never commit one. The app must stay fully usable without a key; only the agent endpoints require it. +- EF migrations: `dotnet ef migrations add -p src/Novelly.Api -o Data/Migrations`. The API migrates on boot. +- `git push` runs `scripts/ci/prepush.sh` through Husky: build, test, then a web build. Run `npm install` + once at the repo root to install the hook. diff --git a/src/Novelly.Api/Agent/AgentEndpoints.cs b/src/Novelly.Api/Agent/AgentEndpoints.cs index a8ae893..6540a9b 100644 --- a/src/Novelly.Api/Agent/AgentEndpoints.cs +++ b/src/Novelly.Api/Agent/AgentEndpoints.cs @@ -1,10 +1,12 @@ +using Novelly.Api.Common; + namespace Novelly.Api.Agent; public static class AgentEndpoints { public static IEndpointRouteBuilder MapAgentEndpoints(this IEndpointRouteBuilder app) { - var projectScoped = app.MapGroup("/api/projects/{projectId:guid}/agent").WithTags("Agent"); + var projectScoped = app.MapGroup("/api/projects/{projectId:guid}/agent").WithTags("Agent").AddEndpointFilter(); projectScoped.MapGet("/conversations", async ( Guid projectId, NovelAgentService agent, CancellationToken ct) => @@ -19,7 +21,7 @@ public static class AgentEndpoints Results.Ok(await agent.SendMessageAsync(projectId, request, ct))) .WithSummary("Send a message to the writing agent and run it to completion."); - var conversations = app.MapGroup("/api/conversations").WithTags("Agent"); + var conversations = app.MapGroup("/api/conversations").WithTags("Agent").AddEndpointFilter(); conversations.MapGet("/{id:guid}", async (Guid id, NovelAgentService agent, CancellationToken ct) => Results.Ok(await agent.GetConversationAsync(id, ct))) diff --git a/src/Novelly.Api/Agent/AnthropicAgentModelClient.cs b/src/Novelly.Api/Agent/AnthropicAgentModelClient.cs index 6ee52b5..f1da5c1 100644 --- a/src/Novelly.Api/Agent/AnthropicAgentModelClient.cs +++ b/src/Novelly.Api/Agent/AnthropicAgentModelClient.cs @@ -11,7 +11,7 @@ namespace Novelly.Api.Agent; /// model-agnostic block types and the SDK's request/response shapes; the tool-use loop /// itself lives in . /// -public class AnthropicAgentModelClient(IOptions options) : IAgentModelClient +public class AnthropicAgentModelClient(IOptions options, ILogger logger) : IAgentModelClient { private readonly AgentOptions _options = options.Value; private AnthropicClient? _client; @@ -36,6 +36,10 @@ public class AnthropicAgentModelClient(IOptions options) : IAgentM IReadOnlyList tools, CancellationToken ct = default) { + logger.LogInformation( + "Calling model {Model} with {MessageCount} messages, {ToolCount} tools, effort {Effort}", + _options.Model, messages.Count, tools.Count, _options.Effort); + var parameters = new MessageCreateParams { Model = _options.Model, @@ -53,6 +57,10 @@ public class AnthropicAgentModelClient(IOptions options) : IAgentM var response = await Client.Messages.Create(parameters, cancellationToken: ct); + logger.LogInformation( + "Model {Model} responded with stop reason {StopReason}, input tokens {InputTokens}, output tokens {OutputTokens}", + _options.Model, response.StopReason, response.Usage?.InputTokens, response.Usage?.OutputTokens); + return new AgentModelResponse( [.. response.Content.Select(FromSdkBlock).OfType()], response.StopReason?.ToString()); diff --git a/src/Novelly.Api/Agent/NovelAgentService.cs b/src/Novelly.Api/Agent/NovelAgentService.cs index 5c1bbca..d28eb71 100644 --- a/src/Novelly.Api/Agent/NovelAgentService.cs +++ b/src/Novelly.Api/Agent/NovelAgentService.cs @@ -28,15 +28,21 @@ public class NovelAgentService( private readonly AgentOptions _options = options.Value; public async Task> ListConversationsAsync( - Guid projectId, CancellationToken ct = default) => - await db.Conversations + Guid projectId, CancellationToken ct = default) + { + logger.LogInformation("Listing agent conversations for project {ProjectId}", projectId); + + return await db.Conversations .Where(c => c.ProjectId == projectId) .OrderByDescending(c => c.UpdatedAt) .Select(c => new ConversationSummaryDto(c.Id, c.ProjectId, c.Title, c.Messages.Count, c.UpdatedAt)) .ToListAsync(ct); + } public async Task GetConversationAsync(Guid conversationId, CancellationToken ct = default) { + logger.LogInformation("Getting agent conversation {ConversationId}", conversationId); + var conversation = await LoadConversationAsync(conversationId, ct); return new ConversationDto( @@ -49,6 +55,8 @@ public class NovelAgentService( public async Task DeleteConversationAsync(Guid conversationId, CancellationToken ct = default) { + logger.LogInformation("Deleting agent conversation {ConversationId}", conversationId); + var conversation = await LoadConversationAsync(conversationId, ct); db.Conversations.Remove(conversation); await db.SaveChangesAsync(ct); @@ -61,6 +69,10 @@ public class NovelAgentService( public async Task SendMessageAsync( Guid projectId, SendAgentMessageRequest request, CancellationToken ct = default) { + logger.LogInformation( + "Sending agent message for project {ProjectId}, conversation {ConversationId}, message length {MessageLength}", + projectId, request.ConversationId, request.Message.Length); + var conversation = request.ConversationId is { } id ? await LoadConversationAsync(id, ct) : await StartConversationAsync(projectId, request.Message, ct); @@ -77,6 +89,8 @@ public class NovelAgentService( for (var iteration = 0; iteration < _options.MaxIterations; iteration++) { + logger.LogDebug("Agent iteration {Iteration} for project {ProjectId}", iteration, projectId); + var response = await model.CompleteAsync(systemPrompt, transcript, toolset.Definitions, ct); foreach (var block in response.Content.OfType()) @@ -142,6 +156,8 @@ public class NovelAgentService( private async Task AppendMessageAsync( AgentConversation conversation, AgentRole role, string content, string? toolCallsJson, CancellationToken ct) { + logger.LogDebug("Appending {Role} message to conversation {ConversationId}, content length {ContentLength}", role, conversation.Id, content.Length); + var message = new AgentMessage { ConversationId = conversation.Id, @@ -170,8 +186,11 @@ public class NovelAgentService( private async Task StartConversationAsync( Guid projectId, string firstMessage, CancellationToken ct) { + logger.LogDebug("Starting new agent conversation for project {ProjectId}", projectId); + if (!await db.Projects.AnyAsync(p => p.Id == projectId, ct)) { + logger.LogWarning("Project {ProjectId} not found", projectId); throw new NotFoundException(nameof(Project), projectId); } @@ -185,11 +204,22 @@ public class NovelAgentService( return conversation; } - private async Task LoadConversationAsync(Guid conversationId, CancellationToken ct) => - await db.Conversations + private async Task LoadConversationAsync(Guid conversationId, CancellationToken ct) + { + logger.LogDebug("Loading agent conversation {ConversationId}", conversationId); + + var conversation = await db.Conversations .Include(c => c.Messages) - .FirstOrDefaultAsync(c => c.Id == conversationId, ct) - ?? throw new NotFoundException(nameof(AgentConversation), conversationId); + .FirstOrDefaultAsync(c => c.Id == conversationId, ct); + + if (conversation is null) + { + logger.LogWarning("AgentConversation {ConversationId} not found", conversationId); + throw new NotFoundException(nameof(AgentConversation), conversationId); + } + + return conversation; + } /// /// Replays the stored conversation as plain text turns. Tool calls are not replayed — @@ -208,8 +238,14 @@ public class NovelAgentService( private async Task BuildSystemPromptAsync(Guid projectId, CancellationToken ct) { - var project = await db.Projects.FirstOrDefaultAsync(p => p.Id == projectId, ct) - ?? throw new NotFoundException(nameof(Project), projectId); + logger.LogDebug("Building system prompt for project {ProjectId}", projectId); + + var project = await db.Projects.FirstOrDefaultAsync(p => p.Id == projectId, ct); + if (project is null) + { + logger.LogWarning("Project {ProjectId} not found", projectId); + throw new NotFoundException(nameof(Project), projectId); + } var brief = new StringBuilder(); brief.AppendLine($"Title: {project.Title}"); diff --git a/src/Novelly.Api/Agent/NovelAgentToolset.cs b/src/Novelly.Api/Agent/NovelAgentToolset.cs index 1398a97..1007e71 100644 --- a/src/Novelly.Api/Agent/NovelAgentToolset.cs +++ b/src/Novelly.Api/Agent/NovelAgentToolset.cs @@ -33,7 +33,8 @@ public class NovelAgentToolset( BeatService beats, SceneService scenes, TagService tags, - OpenQuestionService questions) + OpenQuestionService questions, + ILogger logger) { private static readonly JsonSerializerOptions SerializerOptions = new() { @@ -57,24 +58,31 @@ public class NovelAgentToolset( { if (!ByName.TryGetValue(name, out var tool)) { + logger.LogWarning("Agent requested unknown tool {Tool}", name); return new AgentToolResult($"No such tool: '{name}'.", true); } + logger.LogDebug("Running tool {Tool} for project {ProjectId}", name, projectId); + try { var result = await tool.Handler(projectId, input, ct); + logger.LogDebug("Tool {Tool} for project {ProjectId} succeeded", name, projectId); return new AgentToolResult(JsonSerializer.Serialize(result, SerializerOptions), false); } catch (NotFoundException ex) { + logger.LogWarning(ex, "Tool {Tool} for project {ProjectId} failed: not found", name, projectId); return new AgentToolResult(ex.Message, true); } catch (ArgumentException ex) { + logger.LogWarning(ex, "Tool {Tool} for project {ProjectId} failed: invalid argument", name, projectId); return new AgentToolResult(ex.Message, true); } catch (InvalidOperationException ex) { + logger.LogWarning(ex, "Tool {Tool} for project {ProjectId} failed: invalid operation", name, projectId); return new AgentToolResult(ex.Message, true); } } diff --git a/src/Novelly.Api/Beats/BeatEndpoints.cs b/src/Novelly.Api/Beats/BeatEndpoints.cs index 29fd672..01a9c43 100644 --- a/src/Novelly.Api/Beats/BeatEndpoints.cs +++ b/src/Novelly.Api/Beats/BeatEndpoints.cs @@ -1,10 +1,12 @@ +using Novelly.Api.Common; + namespace Novelly.Api.Beats; public static class BeatEndpoints { public static IEndpointRouteBuilder MapBeatEndpoints(this IEndpointRouteBuilder app) { - var chapterScoped = app.MapGroup("/api/chapters/{chapterId:guid}/beats").WithTags("Beats"); + var chapterScoped = app.MapGroup("/api/chapters/{chapterId:guid}/beats").WithTags("Beats").AddEndpointFilter(); chapterScoped.MapGet("/", async (Guid chapterId, BeatService service, CancellationToken ct) => Results.Ok(await service.ListAsync(chapterId, ct))) @@ -29,7 +31,7 @@ public static class BeatEndpoints .WithTags("Beats") .WithSummary("Every beat this character appears in, in manuscript order."); - var beats = app.MapGroup("/api/beats").WithTags("Beats"); + var beats = app.MapGroup("/api/beats").WithTags("Beats").AddEndpointFilter(); beats.MapGet("/{id:guid}", async (Guid id, BeatService service, CancellationToken ct) => Results.Ok(await service.GetAsync(id, ct))) diff --git a/src/Novelly.Api/Beats/BeatService.cs b/src/Novelly.Api/Beats/BeatService.cs index eacf0a3..4a29dac 100644 --- a/src/Novelly.Api/Beats/BeatService.cs +++ b/src/Novelly.Api/Beats/BeatService.cs @@ -11,10 +11,12 @@ namespace Novelly.Api.Beats; /// Beats are a chapter's outline: a flat, ordered table rather than a tree. Everything /// here is scoped to one chapter. /// -public class BeatService(INovelDbContext db, TagService tags) +public class BeatService(INovelDbContext db, TagService tags, ILogger logger) { public async Task> ListAsync(Guid chapterId, CancellationToken ct = default) { + logger.LogInformation("Listing beats for chapter {ChapterId}", chapterId); + var beats = await Query() .Where(b => b.ChapterId == chapterId) .OrderBy(b => b.SortOrder) @@ -23,8 +25,11 @@ public class BeatService(INovelDbContext db, TagService tags) return [.. beats.Select(b => b.ToDto())]; } - public async Task GetAsync(Guid id, CancellationToken ct = default) => - (await FindAsync(id, ct)).ToDto(); + public async Task GetAsync(Guid id, CancellationToken ct = default) + { + logger.LogInformation("Getting beat {BeatId}", id); + return (await FindAsync(id, ct)).ToDto(); + } /// /// Every beat this character appears in, in manuscript order. This is the character @@ -34,8 +39,11 @@ public class BeatService(INovelDbContext db, TagService tags) public async Task> ListForCharacterAsync( Guid characterId, CancellationToken ct = default) { + logger.LogInformation("Listing beats for character {CharacterId}", characterId); + if (!await db.Characters.AnyAsync(c => c.Id == characterId, ct)) { + logger.LogWarning("Character {CharacterId} not found", characterId); throw new NotFoundException(nameof(Character), characterId); } @@ -66,8 +74,14 @@ public class BeatService(INovelDbContext db, TagService tags) public async Task CreateAsync(Guid chapterId, CreateBeatRequest request, CancellationToken ct = default) { - var chapter = await db.Chapters.FirstOrDefaultAsync(c => c.Id == chapterId, ct) - ?? throw new NotFoundException(nameof(Chapter), chapterId); + logger.LogInformation("Creating beat {Title} for chapter {ChapterId}", request.Title, chapterId); + + var chapter = await db.Chapters.FirstOrDefaultAsync(c => c.Id == chapterId, ct); + if (chapter is null) + { + logger.LogWarning("Chapter {ChapterId} not found", chapterId); + throw new NotFoundException(nameof(Chapter), chapterId); + } await ValidateReferencesAsync(chapter, request.CharacterId, request.SceneId, ct); @@ -94,9 +108,15 @@ public class BeatService(INovelDbContext db, TagService tags) public async Task UpdateAsync(Guid id, UpdateBeatRequest request, CancellationToken ct = default) { + logger.LogInformation("Updating beat {BeatId}", id); + var beat = await FindAsync(id, ct); - var chapter = await db.Chapters.FirstOrDefaultAsync(c => c.Id == beat.ChapterId, ct) - ?? throw new NotFoundException(nameof(Chapter), beat.ChapterId); + var chapter = await db.Chapters.FirstOrDefaultAsync(c => c.Id == beat.ChapterId, ct); + if (chapter is null) + { + logger.LogWarning("Chapter {ChapterId} not found", beat.ChapterId); + throw new NotFoundException(nameof(Chapter), beat.ChapterId); + } await ValidateReferencesAsync(chapter, request.CharacterId, request.SceneId, ct); @@ -119,6 +139,8 @@ public class BeatService(INovelDbContext db, TagService tags) public async Task DeleteAsync(Guid id, CancellationToken ct = default) { + logger.LogInformation("Deleting beat {BeatId}", id); + var beat = await FindAsync(id, ct); db.Beats.Remove(beat); await db.SaveChangesAsync(ct); @@ -131,11 +153,14 @@ public class BeatService(INovelDbContext db, TagService tags) public async Task> ReorderAsync( Guid chapterId, ReorderBeatsRequest request, CancellationToken ct = default) { + logger.LogInformation("Reordering {Count} beats for chapter {ChapterId}", request.BeatIds.Count, chapterId); + var beats = await db.Beats.Where(b => b.ChapterId == chapterId).ToListAsync(ct); var missing = request.BeatIds.Where(id => beats.All(b => b.Id != id)).ToList(); if (missing.Count > 0) { + logger.LogWarning("Reorder for chapter {ChapterId} referenced missing beat {BeatId}", chapterId, missing[0]); throw new NotFoundException(nameof(Beat), missing[0]); } @@ -159,6 +184,8 @@ public class BeatService(INovelDbContext db, TagService tags) private async Task ValidateReferencesAsync( Chapter chapter, Guid? characterId, Guid? sceneId, CancellationToken ct) { + logger.LogDebug("Validating beat references for chapter {ChapterId}: character {CharacterId}, scene {SceneId}", chapter.Id, characterId, sceneId); + if (characterId is { } cid) { var belongs = await db.Characters @@ -166,6 +193,7 @@ public class BeatService(INovelDbContext db, TagService tags) if (!belongs) { + logger.LogWarning("Rejected beat reference: character {CharacterId} does not belong to project {ProjectId}", cid, chapter.ProjectId); throw new InvalidOperationException( "A beat's character must belong to the same project as its chapter."); } @@ -177,6 +205,7 @@ public class BeatService(INovelDbContext db, TagService tags) if (!belongs) { + logger.LogWarning("Rejected beat reference: scene {SceneId} does not belong to chapter {ChapterId}", sid, chapter.Id); throw new InvalidOperationException( "A beat can only be grouped under a scene in the same chapter."); } @@ -185,6 +214,8 @@ public class BeatService(INovelDbContext db, TagService tags) private async Task NextSortOrderAsync(Guid chapterId, CancellationToken ct) { + logger.LogDebug("Computing next sort order for chapter {ChapterId}", chapterId); + var max = await db.Beats .Where(b => b.ChapterId == chapterId) .MaxAsync(b => (int?)b.SortOrder, ct); @@ -198,7 +229,18 @@ public class BeatService(INovelDbContext db, TagService tags) .Include(b => b.Scene) .Include(b => b.Tags); - private async Task FindAsync(Guid id, CancellationToken ct) => - await Query().FirstOrDefaultAsync(b => b.Id == id, ct) - ?? throw new NotFoundException(nameof(Beat), id); + private async Task FindAsync(Guid id, CancellationToken ct) + { + logger.LogDebug("Finding beat {BeatId}", id); + + var beat = await Query().FirstOrDefaultAsync(b => b.Id == id, ct); + if (beat is null) + { + logger.LogWarning("Beat {BeatId} not found", id); + throw new NotFoundException(nameof(Beat), id); + } + + logger.LogDebug("Found beat {BeatId}", id); + return beat; + } } diff --git a/src/Novelly.Api/Chapters/ChapterEndpoints.cs b/src/Novelly.Api/Chapters/ChapterEndpoints.cs index a8178f2..f6f1d10 100644 --- a/src/Novelly.Api/Chapters/ChapterEndpoints.cs +++ b/src/Novelly.Api/Chapters/ChapterEndpoints.cs @@ -1,10 +1,12 @@ +using Novelly.Api.Common; + namespace Novelly.Api.Chapters; public static class ChapterEndpoints { public static IEndpointRouteBuilder MapChapterEndpoints(this IEndpointRouteBuilder app) { - var projectScoped = app.MapGroup("/api/projects/{projectId:guid}/chapters").WithTags("Chapters"); + var projectScoped = app.MapGroup("/api/projects/{projectId:guid}/chapters").WithTags("Chapters").AddEndpointFilter(); projectScoped.MapGet("/", async (Guid projectId, ChapterService service, CancellationToken ct) => Results.Ok(await service.ListAsync(projectId, ct))) @@ -18,7 +20,7 @@ public static class ChapterEndpoints }) .WithSummary("Add a chapter."); - var chapters = app.MapGroup("/api/chapters").WithTags("Chapters"); + var chapters = app.MapGroup("/api/chapters").WithTags("Chapters").AddEndpointFilter(); chapters.MapGet("/{id:guid}", async (Guid id, ChapterService service, CancellationToken ct) => Results.Ok(await service.GetAsync(id, ct))) diff --git a/src/Novelly.Api/Chapters/ChapterService.cs b/src/Novelly.Api/Chapters/ChapterService.cs index 9e9ed61..156be29 100644 --- a/src/Novelly.Api/Chapters/ChapterService.cs +++ b/src/Novelly.Api/Chapters/ChapterService.cs @@ -6,10 +6,12 @@ using Novelly.Api.Tags; namespace Novelly.Api.Chapters; -public class ChapterService(INovelDbContext db, TagService tags) +public class ChapterService(INovelDbContext db, TagService tags, ILogger logger) { public async Task> ListAsync(Guid projectId, CancellationToken ct = default) { + logger.LogInformation("Listing chapters for project {ProjectId}", projectId); + var chapters = await db.Chapters .Include(c => c.PovCharacter) .Include(c => c.Beats) @@ -22,13 +24,19 @@ public class ChapterService(INovelDbContext db, TagService tags) return [.. chapters.Select(c => c.ToSummaryDto())]; } - public async Task GetAsync(Guid id, CancellationToken ct = default) => - (await FindAsync(id, ct)).ToDto(); + public async Task GetAsync(Guid id, CancellationToken ct = default) + { + logger.LogInformation("Getting chapter {ChapterId}", id); + return (await FindAsync(id, ct)).ToDto(); + } public async Task CreateAsync(Guid projectId, CreateChapterRequest request, CancellationToken ct = default) { + logger.LogInformation("Creating chapter {Title} for project {ProjectId}", request.Title, projectId); + if (!await db.Projects.AnyAsync(p => p.Id == projectId, ct)) { + logger.LogWarning("Project {ProjectId} not found", projectId); throw new NotFoundException(nameof(Project), projectId); } @@ -57,6 +65,8 @@ public class ChapterService(INovelDbContext db, TagService tags) public async Task UpdateAsync(Guid id, UpdateChapterRequest request, CancellationToken ct = default) { + logger.LogInformation("Updating chapter {ChapterId}", id); + var chapter = await FindAsync(id, ct); chapter.Title = Patch.Apply(chapter.Title, request.Title) ?? chapter.Title; @@ -80,6 +90,8 @@ public class ChapterService(INovelDbContext db, TagService tags) public async Task DeleteAsync(Guid id, CancellationToken ct = default) { + logger.LogInformation("Deleting chapter {ChapterId}", id); + var chapter = await FindAsync(id, ct); db.Chapters.Remove(chapter); await db.SaveChangesAsync(ct); @@ -87,21 +99,37 @@ public class ChapterService(INovelDbContext db, TagService tags) private async Task NextChapterNumberAsync(Guid projectId, CancellationToken ct) { + logger.LogDebug("Computing next chapter number for project {ProjectId}", projectId); + var max = await db.Chapters .Where(c => c.ProjectId == projectId) .MaxAsync(c => (int?)c.Number, ct); - return (max ?? 0) + 1; + var next = (max ?? 0) + 1; + logger.LogDebug("Next chapter number for project {ProjectId} is {Number}", projectId, next); + return next; } - private async Task FindAsync(Guid id, CancellationToken ct) => - await db.Chapters + private async Task FindAsync(Guid id, CancellationToken ct) + { + logger.LogDebug("Finding chapter {ChapterId}", id); + + var chapter = await db.Chapters .Include(c => c.PovCharacter) .Include(c => c.Beats).ThenInclude(b => b.Character) .Include(c => c.Beats).ThenInclude(b => b.Scene) .Include(c => c.Beats).ThenInclude(b => b.Tags) .Include(c => c.Scenes).ThenInclude(s => s.PovCharacter) .Include(c => c.Tags) - .FirstOrDefaultAsync(c => c.Id == id, ct) - ?? throw new NotFoundException(nameof(Chapter), id); + .FirstOrDefaultAsync(c => c.Id == id, ct); + + if (chapter is null) + { + logger.LogWarning("Chapter {ChapterId} not found", id); + throw new NotFoundException(nameof(Chapter), id); + } + + logger.LogDebug("Found chapter {ChapterId}", id); + return chapter; + } } diff --git a/src/Novelly.Api/Characters/CharacterArcService.cs b/src/Novelly.Api/Characters/CharacterArcService.cs index 9adff1e..d42c763 100644 --- a/src/Novelly.Api/Characters/CharacterArcService.cs +++ b/src/Novelly.Api/Characters/CharacterArcService.cs @@ -13,10 +13,12 @@ namespace Novelly.Api.Characters; /// on a supporting character. Demoting someone should not delete work, and a character /// who turns out to matter gets promoted after the arc is already sketched. /// -public class CharacterArcService(INovelDbContext db) +public class CharacterArcService(INovelDbContext db, ILogger logger) { public async Task> ListAsync(Guid characterId, CancellationToken ct = default) { + logger.LogInformation("Listing arc stages for character {CharacterId}", characterId); + var stages = await Query() .Where(s => s.CharacterId == characterId) .OrderBy(s => s.SortOrder) @@ -25,14 +27,23 @@ public class CharacterArcService(INovelDbContext db) return [.. stages.Select(s => s.ToDto())]; } - public async Task GetAsync(Guid id, CancellationToken ct = default) => - (await FindAsync(id, ct)).ToDto(); + public async Task GetAsync(Guid id, CancellationToken ct = default) + { + logger.LogInformation("Getting arc stage {ArcStageId}", id); + return (await FindAsync(id, ct)).ToDto(); + } public async Task CreateAsync( Guid characterId, CreateArcStageRequest request, CancellationToken ct = default) { - var character = await db.Characters.FirstOrDefaultAsync(c => c.Id == characterId, ct) - ?? throw new NotFoundException(nameof(Character), characterId); + logger.LogInformation("Creating arc stage {Title} for character {CharacterId}", request.Title, characterId); + + var character = await db.Characters.FirstOrDefaultAsync(c => c.Id == characterId, ct); + if (character is null) + { + logger.LogWarning("Character {CharacterId} not found", characterId); + throw new NotFoundException(nameof(Character), characterId); + } await EnsureChapterIsInSameProjectAsync(character, request.ChapterId, ct); @@ -53,10 +64,16 @@ public class CharacterArcService(INovelDbContext db) public async Task UpdateAsync( Guid id, UpdateArcStageRequest request, CancellationToken ct = default) { + logger.LogInformation("Updating arc stage {ArcStageId}", id); + var stage = await FindAsync(id, ct); - var character = await db.Characters.FirstOrDefaultAsync(c => c.Id == stage.CharacterId, ct) - ?? throw new NotFoundException(nameof(Character), stage.CharacterId); + var character = await db.Characters.FirstOrDefaultAsync(c => c.Id == stage.CharacterId, ct); + if (character is null) + { + logger.LogWarning("Character {CharacterId} not found", stage.CharacterId); + throw new NotFoundException(nameof(Character), stage.CharacterId); + } await EnsureChapterIsInSameProjectAsync(character, request.ChapterId, ct); @@ -72,6 +89,8 @@ public class CharacterArcService(INovelDbContext db) public async Task DeleteAsync(Guid id, CancellationToken ct = default) { + logger.LogInformation("Deleting arc stage {ArcStageId}", id); + var stage = await FindAsync(id, ct); db.CharacterArcStages.Remove(stage); await db.SaveChangesAsync(ct); @@ -84,6 +103,8 @@ public class CharacterArcService(INovelDbContext db) public async Task> ReorderAsync( Guid characterId, ReorderArcStagesRequest request, CancellationToken ct = default) { + logger.LogInformation("Reordering {Count} arc stages for character {CharacterId}", request.StageIds.Count, characterId); + var stages = await db.CharacterArcStages .Where(s => s.CharacterId == characterId) .ToListAsync(ct); @@ -91,6 +112,7 @@ public class CharacterArcService(INovelDbContext db) var missing = request.StageIds.Where(id => stages.All(s => s.Id != id)).ToList(); if (missing.Count > 0) { + logger.LogWarning("Reorder for character {CharacterId} referenced missing arc stage {ArcStageId}", characterId, missing[0]); throw new NotFoundException(nameof(CharacterArcStage), missing[0]); } @@ -117,10 +139,13 @@ public class CharacterArcService(INovelDbContext db) return; } + logger.LogDebug("Checking chapter {ChapterId} belongs to project {ProjectId}", id, character.ProjectId); + var belongs = await db.Chapters.AnyAsync(c => c.Id == id && c.ProjectId == character.ProjectId, ct); if (!belongs) { + logger.LogWarning("Rejected arc stage: chapter {ChapterId} does not belong to project {ProjectId}", id, character.ProjectId); throw new InvalidOperationException( "An arc stage can only point at a chapter in the same project as its character."); } @@ -128,6 +153,8 @@ public class CharacterArcService(INovelDbContext db) private async Task NextSortOrderAsync(Guid characterId, CancellationToken ct) { + logger.LogDebug("Computing next sort order for character {CharacterId}", characterId); + var max = await db.CharacterArcStages .Where(s => s.CharacterId == characterId) .MaxAsync(s => (int?)s.SortOrder, ct); @@ -137,7 +164,18 @@ public class CharacterArcService(INovelDbContext db) private IQueryable Query() => db.CharacterArcStages.Include(s => s.Chapter); - private async Task FindAsync(Guid id, CancellationToken ct) => - await Query().FirstOrDefaultAsync(s => s.Id == id, ct) - ?? throw new NotFoundException(nameof(CharacterArcStage), id); + private async Task FindAsync(Guid id, CancellationToken ct) + { + logger.LogDebug("Finding arc stage {ArcStageId}", id); + + var stage = await Query().FirstOrDefaultAsync(s => s.Id == id, ct); + if (stage is null) + { + logger.LogWarning("CharacterArcStage {ArcStageId} not found", id); + throw new NotFoundException(nameof(CharacterArcStage), id); + } + + logger.LogDebug("Found arc stage {ArcStageId}", id); + return stage; + } } diff --git a/src/Novelly.Api/Characters/CharacterEndpoints.cs b/src/Novelly.Api/Characters/CharacterEndpoints.cs index 1638e02..6f1564e 100644 --- a/src/Novelly.Api/Characters/CharacterEndpoints.cs +++ b/src/Novelly.Api/Characters/CharacterEndpoints.cs @@ -1,10 +1,12 @@ +using Novelly.Api.Common; + namespace Novelly.Api.Characters; public static class CharacterEndpoints { public static IEndpointRouteBuilder MapCharacterEndpoints(this IEndpointRouteBuilder app) { - var projectScoped = app.MapGroup("/api/projects/{projectId:guid}/characters").WithTags("Characters"); + var projectScoped = app.MapGroup("/api/projects/{projectId:guid}/characters").WithTags("Characters").AddEndpointFilter(); projectScoped.MapGet("/", async (Guid projectId, CharacterService service, CancellationToken ct) => Results.Ok(await service.ListAsync(projectId, ct))) @@ -18,7 +20,7 @@ public static class CharacterEndpoints }) .WithSummary("Add a character dossier."); - var characters = app.MapGroup("/api/characters").WithTags("Characters"); + var characters = app.MapGroup("/api/characters").WithTags("Characters").AddEndpointFilter(); characters.MapGet("/{id:guid}", async (Guid id, CharacterService service, CancellationToken ct) => Results.Ok(await service.GetAsync(id, ct))) @@ -67,7 +69,7 @@ public static class CharacterEndpoints Results.Ok(await service.ReorderAsync(id, request, ct))) .WithSummary("Renumber a character's arc to match the order given."); - var arcStages = app.MapGroup("/api/arc-stages").WithTags("Characters"); + var arcStages = app.MapGroup("/api/arc-stages").WithTags("Characters").AddEndpointFilter(); arcStages.MapGet("/{id:guid}", async (Guid id, CharacterArcService service, CancellationToken ct) => Results.Ok(await service.GetAsync(id, ct))) diff --git a/src/Novelly.Api/Characters/CharacterService.cs b/src/Novelly.Api/Characters/CharacterService.cs index edc46c9..3926d2b 100644 --- a/src/Novelly.Api/Characters/CharacterService.cs +++ b/src/Novelly.Api/Characters/CharacterService.cs @@ -6,7 +6,7 @@ using Novelly.Api.Tags; namespace Novelly.Api.Characters; -public class CharacterService(INovelDbContext db, TagService tags) +public class CharacterService(INovelDbContext db, TagService tags, ILogger logger) { /// /// Main characters first, then by the part they play, then by name. @@ -20,6 +20,8 @@ public class CharacterService(INovelDbContext db, TagService tags) /// public async Task> ListAsync(Guid projectId, CancellationToken ct = default) { + logger.LogInformation("Listing characters for project {ProjectId}", projectId); + var characters = await Query() .Where(c => c.ProjectId == projectId) .ToListAsync(ct); @@ -34,11 +36,16 @@ public class CharacterService(INovelDbContext db, TagService tags) ]; } - public async Task GetAsync(Guid id, CancellationToken ct = default) => - (await FindAsync(id, ct)).ToDto(); + public async Task GetAsync(Guid id, CancellationToken ct = default) + { + logger.LogInformation("Getting character {CharacterId}", id); + return (await FindAsync(id, ct)).ToDto(); + } public async Task CreateAsync(Guid projectId, CreateCharacterRequest request, CancellationToken ct = default) { + logger.LogInformation("Creating character {Name} for project {ProjectId}, role {Role}, importance {Importance}", request.Name, projectId, request.Role, request.Importance); + await EnsureProjectExists(projectId, ct); var character = new Character @@ -74,6 +81,8 @@ public class CharacterService(INovelDbContext db, TagService tags) public async Task UpdateAsync(Guid id, UpdateCharacterRequest request, CancellationToken ct = default) { + logger.LogInformation("Updating character {CharacterId}", id); + var character = await FindAsync(id, ct); character.Name = Patch.Apply(character.Name, request.Name) ?? character.Name; @@ -105,6 +114,8 @@ public class CharacterService(INovelDbContext db, TagService tags) public async Task DeleteAsync(Guid id, CancellationToken ct = default) { + logger.LogInformation("Deleting character {CharacterId}", id); + var character = await FindAsync(id, ct); db.Characters.Remove(character); await db.SaveChangesAsync(ct); @@ -113,14 +124,20 @@ public class CharacterService(INovelDbContext db, TagService tags) public async Task AddRelationshipAsync( Guid characterId, CreateRelationshipRequest request, CancellationToken ct = default) { + logger.LogInformation("Adding relationship {RelationshipType} from character {CharacterId} to {RelatedCharacterId}", request.RelationshipType, characterId, request.RelatedCharacterId); + var character = await FindAsync(characterId, ct); - var related = await db.Characters - .FirstOrDefaultAsync(c => c.Id == request.RelatedCharacterId, ct) - ?? throw new NotFoundException(nameof(Character), request.RelatedCharacterId); + var related = await db.Characters.FirstOrDefaultAsync(c => c.Id == request.RelatedCharacterId, ct); + if (related is null) + { + logger.LogWarning("Character {RelatedCharacterId} not found", request.RelatedCharacterId); + throw new NotFoundException(nameof(Character), request.RelatedCharacterId); + } if (related.ProjectId != character.ProjectId) { + logger.LogWarning("Rejected relationship: character {CharacterId} and {RelatedCharacterId} belong to different projects", characterId, request.RelatedCharacterId); throw new InvalidOperationException("Characters must belong to the same project to be related."); } @@ -138,9 +155,14 @@ public class CharacterService(INovelDbContext db, TagService tags) public async Task RemoveRelationshipAsync(Guid relationshipId, CancellationToken ct = default) { - var relationship = await db.CharacterRelationships - .FirstOrDefaultAsync(r => r.Id == relationshipId, ct) - ?? throw new NotFoundException(nameof(CharacterRelationship), relationshipId); + logger.LogInformation("Removing relationship {RelationshipId}", relationshipId); + + var relationship = await db.CharacterRelationships.FirstOrDefaultAsync(r => r.Id == relationshipId, ct); + if (relationship is null) + { + logger.LogWarning("CharacterRelationship {RelationshipId} not found", relationshipId); + throw new NotFoundException(nameof(CharacterRelationship), relationshipId); + } db.CharacterRelationships.Remove(relationship); await db.SaveChangesAsync(ct); @@ -154,14 +176,28 @@ public class CharacterService(INovelDbContext db, TagService tags) .Include(c => c.ArcStages) .ThenInclude(s => s.Chapter); - private async Task FindAsync(Guid id, CancellationToken ct) => - await Query().FirstOrDefaultAsync(c => c.Id == id, ct) - ?? throw new NotFoundException(nameof(Character), id); + private async Task FindAsync(Guid id, CancellationToken ct) + { + logger.LogDebug("Finding character {CharacterId}", id); + + var character = await Query().FirstOrDefaultAsync(c => c.Id == id, ct); + if (character is null) + { + logger.LogWarning("Character {CharacterId} not found", id); + throw new NotFoundException(nameof(Character), id); + } + + logger.LogDebug("Found character {CharacterId}", id); + return character; + } private async Task EnsureProjectExists(Guid projectId, CancellationToken ct) { + logger.LogDebug("Checking project {ProjectId} exists", projectId); + if (!await db.Projects.AnyAsync(p => p.Id == projectId, ct)) { + logger.LogWarning("Project {ProjectId} not found", projectId); throw new NotFoundException(nameof(Project), projectId); } } diff --git a/src/Novelly.Api/Common/RequestLoggingEndpointFilter.cs b/src/Novelly.Api/Common/RequestLoggingEndpointFilter.cs new file mode 100644 index 0000000..3eade78 --- /dev/null +++ b/src/Novelly.Api/Common/RequestLoggingEndpointFilter.cs @@ -0,0 +1,24 @@ +namespace Novelly.Api.Common; + +/// +/// Logs every request an endpoint group handles: Information on entry with the route's +/// name and values, Debug on exit with the resulting status. Applied per MapGroup +/// rather than inside each handler, so no endpoint lambda needs to know about logging. +/// +public class RequestLoggingEndpointFilter(ILogger logger) : IEndpointFilter +{ + public async ValueTask InvokeAsync(EndpointFilterInvocationContext context, EndpointFilterDelegate next) + { + var routeName = context.HttpContext.GetEndpoint()?.DisplayName ?? context.HttpContext.Request.Path; + + logger.LogInformation( + "HTTP {Method} {Route} invoked with {@RouteValues}", + context.HttpContext.Request.Method, routeName, context.HttpContext.Request.RouteValues); + + var result = await next(context); + + logger.LogDebug("HTTP {Method} {Route} completed with {ResultType}", context.HttpContext.Request.Method, routeName, result?.GetType().Name); + + return result; + } +} diff --git a/src/Novelly.Api/Novelly.Api.csproj b/src/Novelly.Api/Novelly.Api.csproj index 1840e53..51afa67 100644 --- a/src/Novelly.Api/Novelly.Api.csproj +++ b/src/Novelly.Api/Novelly.Api.csproj @@ -13,6 +13,8 @@ + + diff --git a/src/Novelly.Api/Program.cs b/src/Novelly.Api/Program.cs index 2b32782..d6b5433 100644 --- a/src/Novelly.Api/Program.cs +++ b/src/Novelly.Api/Program.cs @@ -11,9 +11,17 @@ using Novelly.Api.Projects; using Novelly.Api.Questions; using Novelly.Api.Scenes; using Novelly.Api.Tags; +using Serilog; var builder = WebApplication.CreateBuilder(args); +// AddSerilog (not UseSerilog) so it becomes an additional logging provider rather than +// replacing the one AddServiceDefaults wires up for the Aspire dashboard. +builder.Services.AddSerilog((services, config) => config + .ReadFrom.Configuration(builder.Configuration) + .ReadFrom.Services(services) + .WriteTo.Console(outputTemplate: "[{Timestamp:HH:mm:ss} {Level:u3}] {SourceContext}: {Message:lj}{NewLine}{Exception}")); + builder.AddServiceDefaults(); builder.Services.AddNovelly(builder.Configuration); builder.Services.AddOpenApi(); @@ -41,6 +49,11 @@ using (var scope = app.Services.CreateScope()) await scope.ServiceProvider.GetRequiredService().Database.MigrateAsync(); } +// Serilog's request logging wraps the exception handler (registered first = outermost) +// so it reads the status code the handler already resolved, rather than seeing the raw +// exception fly past and misreporting a handled 404 as a 500. +app.UseSerilogRequestLogging(); + app.UseExceptionHandler(handler => handler.Run(async context => { var exception = context.Features.Get()?.Error; @@ -57,6 +70,10 @@ app.UseExceptionHandler(handler => handler.Run(async context => { app.Logger.LogError(exception, "Unhandled exception on {Path}", context.Request.Path); } + else + { + app.Logger.LogWarning(exception, "Handled {StatusCode} on {Path}: {Title}", status, context.Request.Path, title); + } await Results .Problem(title: title, detail: exception?.Message, statusCode: status) diff --git a/src/Novelly.Api/Projects/ProjectEndpoints.cs b/src/Novelly.Api/Projects/ProjectEndpoints.cs index 3ae99b2..c2202e1 100644 --- a/src/Novelly.Api/Projects/ProjectEndpoints.cs +++ b/src/Novelly.Api/Projects/ProjectEndpoints.cs @@ -1,10 +1,12 @@ +using Novelly.Api.Common; + namespace Novelly.Api.Projects; public static class ProjectEndpoints { public static IEndpointRouteBuilder MapProjectEndpoints(this IEndpointRouteBuilder app) { - var group = app.MapGroup("/api/projects").WithTags("Projects"); + var group = app.MapGroup("/api/projects").WithTags("Projects").AddEndpointFilter(); group.MapGet("/", async (ProjectService service, CancellationToken ct) => Results.Ok(await service.ListAsync(ct))) diff --git a/src/Novelly.Api/Projects/ProjectService.cs b/src/Novelly.Api/Projects/ProjectService.cs index b4f7da4..d58a57c 100644 --- a/src/Novelly.Api/Projects/ProjectService.cs +++ b/src/Novelly.Api/Projects/ProjectService.cs @@ -4,10 +4,13 @@ using Novelly.Api.Data; namespace Novelly.Api.Projects; -public class ProjectService(INovelDbContext db) +public class ProjectService(INovelDbContext db, ILogger logger) { - public async Task> ListAsync(CancellationToken ct = default) => - await db.Projects + public async Task> ListAsync(CancellationToken ct = default) + { + logger.LogInformation("Listing projects"); + + return await db.Projects .OrderByDescending(p => p.UpdatedAt) .Select(p => new ProjectSummaryDto( p.Id, @@ -21,12 +24,18 @@ public class ProjectService(INovelDbContext db) p.Chapters.SelectMany(c => c.Scenes).Sum(s => (int?)s.WordCount) ?? 0, p.UpdatedAt)) .ToListAsync(ct); + } - public async Task GetAsync(Guid id, CancellationToken ct = default) => - (await FindAsync(id, ct)).ToDto(); + public async Task GetAsync(Guid id, CancellationToken ct = default) + { + logger.LogInformation("Getting project {ProjectId}", id); + return (await FindAsync(id, ct)).ToDto(); + } public async Task CreateAsync(CreateProjectRequest request, CancellationToken ct = default) { + logger.LogInformation("Creating project {Title}", request.Title); + var project = new Project { Title = request.Title, @@ -45,6 +54,8 @@ public class ProjectService(INovelDbContext db) public async Task UpdateAsync(Guid id, UpdateProjectRequest request, CancellationToken ct = default) { + logger.LogInformation("Updating project {ProjectId}", id); + var project = await FindAsync(id, ct); project.Title = Patch.Apply(project.Title, request.Title) ?? project.Title; @@ -62,12 +73,25 @@ public class ProjectService(INovelDbContext db) public async Task DeleteAsync(Guid id, CancellationToken ct = default) { + logger.LogInformation("Deleting project {ProjectId}", id); + var project = await FindAsync(id, ct); db.Projects.Remove(project); await db.SaveChangesAsync(ct); } - private async Task FindAsync(Guid id, CancellationToken ct) => - await db.Projects.FirstOrDefaultAsync(p => p.Id == id, ct) - ?? throw new NotFoundException(nameof(Project), id); + private async Task FindAsync(Guid id, CancellationToken ct) + { + logger.LogDebug("Finding project {ProjectId}", id); + + var project = await db.Projects.FirstOrDefaultAsync(p => p.Id == id, ct); + if (project is null) + { + logger.LogWarning("Project {ProjectId} not found", id); + throw new NotFoundException(nameof(Project), id); + } + + logger.LogDebug("Found project {ProjectId}", id); + return project; + } } diff --git a/src/Novelly.Api/Questions/OpenQuestionEndpoints.cs b/src/Novelly.Api/Questions/OpenQuestionEndpoints.cs index 5ae02f2..fe86633 100644 --- a/src/Novelly.Api/Questions/OpenQuestionEndpoints.cs +++ b/src/Novelly.Api/Questions/OpenQuestionEndpoints.cs @@ -1,10 +1,12 @@ +using Novelly.Api.Common; + namespace Novelly.Api.Questions; public static class OpenQuestionEndpoints { public static IEndpointRouteBuilder MapOpenQuestionEndpoints(this IEndpointRouteBuilder app) { - var projectScoped = app.MapGroup("/api/projects/{projectId:guid}/questions").WithTags("Questions"); + var projectScoped = app.MapGroup("/api/projects/{projectId:guid}/questions").WithTags("Questions").AddEndpointFilter(); projectScoped.MapGet("/", async ( Guid projectId, @@ -24,7 +26,7 @@ public static class OpenQuestionEndpoints }) .WithSummary("Raise an open question, optionally against a chapter outline and/or a character."); - var questions = app.MapGroup("/api/questions").WithTags("Questions"); + var questions = app.MapGroup("/api/questions").WithTags("Questions").AddEndpointFilter(); questions.MapGet("/{id:guid}", async (Guid id, OpenQuestionService service, CancellationToken ct) => Results.Ok(await service.GetAsync(id, ct))) diff --git a/src/Novelly.Api/Questions/OpenQuestionService.cs b/src/Novelly.Api/Questions/OpenQuestionService.cs index 9b775e8..85fe4fa 100644 --- a/src/Novelly.Api/Questions/OpenQuestionService.cs +++ b/src/Novelly.Api/Questions/OpenQuestionService.cs @@ -11,7 +11,7 @@ namespace Novelly.Api.Questions; /// The project's open questions — the decisions still outstanding. A question can be /// attached to a chapter outline, a character, both, or neither. /// -public class OpenQuestionService(INovelDbContext db) +public class OpenQuestionService(INovelDbContext db, ILogger logger) { /// /// Lists a project's questions, open ones first and newest first within each group. @@ -25,6 +25,10 @@ public class OpenQuestionService(INovelDbContext db) bool includeResolved = false, CancellationToken ct = default) { + logger.LogInformation( + "Listing open questions for project {ProjectId}, chapter {ChapterId}, character {CharacterId}, includeResolved {IncludeResolved}", + projectId, chapterId, characterId, includeResolved); + var query = Query().Where(q => q.ProjectId == projectId); if (chapterId is { } cid) @@ -53,19 +57,26 @@ public class OpenQuestionService(INovelDbContext db) ]; } - public async Task GetAsync(Guid id, CancellationToken ct = default) => - (await FindAsync(id, ct)).ToDto(); + public async Task GetAsync(Guid id, CancellationToken ct = default) + { + logger.LogInformation("Getting open question {QuestionId}", id); + return (await FindAsync(id, ct)).ToDto(); + } public async Task CreateAsync( Guid projectId, CreateOpenQuestionRequest request, CancellationToken ct = default) { + logger.LogInformation("Creating open question for project {ProjectId}", projectId); + if (!await db.Projects.AnyAsync(p => p.Id == projectId, ct)) { + logger.LogWarning("Project {ProjectId} not found", projectId); throw new NotFoundException(nameof(Project), projectId); } if (string.IsNullOrWhiteSpace(request.Question)) { + logger.LogWarning("Rejected open question creation for project {ProjectId}: question text was blank", projectId); throw new ArgumentException("A question needs to say something."); } @@ -88,6 +99,8 @@ public class OpenQuestionService(INovelDbContext db) public async Task UpdateAsync( Guid id, UpdateOpenQuestionRequest request, CancellationToken ct = default) { + logger.LogInformation("Updating open question {QuestionId}", id); + var question = await FindAsync(id, ct); await ValidateAssociationsAsync(question.ProjectId, request.ChapterId, request.CharacterId, ct); @@ -110,10 +123,13 @@ public class OpenQuestionService(INovelDbContext db) public async Task ResolveAsync( Guid id, ResolveOpenQuestionRequest request, CancellationToken ct = default) { + logger.LogInformation("Resolving open question {QuestionId}, appendToNotes {AppendToNotes}", id, request.AppendToNotes); + var question = await FindAsync(id, ct); if (string.IsNullOrWhiteSpace(request.Resolution)) { + logger.LogWarning("Rejected resolution for open question {QuestionId}: resolution text was blank", id); throw new ArgumentException("A resolution needs to say what was decided."); } @@ -127,6 +143,8 @@ public class OpenQuestionService(INovelDbContext db) if (question.ChapterId is { } chapterId) { + logger.LogDebug("Appending resolution note to chapter {ChapterId}", chapterId); + var chapter = await db.Chapters.FirstOrDefaultAsync(c => c.Id == chapterId, ct); if (chapter is not null) { @@ -137,6 +155,8 @@ public class OpenQuestionService(INovelDbContext db) if (question.CharacterId is { } characterId) { + logger.LogDebug("Appending resolution note to character {CharacterId}", characterId); + var character = await db.Characters.FirstOrDefaultAsync(c => c.Id == characterId, ct); if (character is not null) { @@ -153,6 +173,8 @@ public class OpenQuestionService(INovelDbContext db) /// Puts a question back on the list. The resolution goes; anything already appended to notes stays. public async Task ReopenAsync(Guid id, CancellationToken ct = default) { + logger.LogInformation("Reopening open question {QuestionId}", id); + var question = await FindAsync(id, ct); question.Resolution = null; @@ -165,6 +187,8 @@ public class OpenQuestionService(INovelDbContext db) public async Task DeleteAsync(Guid id, CancellationToken ct = default) { + logger.LogInformation("Deleting open question {QuestionId}", id); + var question = await FindAsync(id, ct); db.OpenQuestions.Remove(question); await db.SaveChangesAsync(ct); @@ -177,9 +201,12 @@ public class OpenQuestionService(INovelDbContext db) private async Task ValidateAssociationsAsync( Guid projectId, Guid? chapterId, Guid? characterId, CancellationToken ct) { + logger.LogDebug("Validating associations for project {ProjectId}: chapter {ChapterId}, character {CharacterId}", projectId, chapterId, characterId); + if (chapterId is { } cid && !await db.Chapters.AnyAsync(c => c.Id == cid && c.ProjectId == projectId, ct)) { + logger.LogWarning("Rejected question association: chapter {ChapterId} does not belong to project {ProjectId}", cid, projectId); throw new InvalidOperationException( "A question can only be attached to a chapter in the same project."); } @@ -187,6 +214,7 @@ public class OpenQuestionService(INovelDbContext db) if (characterId is { } chid && !await db.Characters.AnyAsync(c => c.Id == chid && c.ProjectId == projectId, ct)) { + logger.LogWarning("Rejected question association: character {CharacterId} does not belong to project {ProjectId}", chid, projectId); throw new InvalidOperationException( "A question can only be attached to a character in the same project."); } @@ -195,7 +223,18 @@ public class OpenQuestionService(INovelDbContext db) private IQueryable Query() => db.OpenQuestions.Include(q => q.Chapter).Include(q => q.Character); - private async Task FindAsync(Guid id, CancellationToken ct) => - await Query().FirstOrDefaultAsync(q => q.Id == id, ct) - ?? throw new NotFoundException(nameof(OpenQuestion), id); + private async Task FindAsync(Guid id, CancellationToken ct) + { + logger.LogDebug("Finding open question {QuestionId}", id); + + var question = await Query().FirstOrDefaultAsync(q => q.Id == id, ct); + if (question is null) + { + logger.LogWarning("OpenQuestion {QuestionId} not found", id); + throw new NotFoundException(nameof(OpenQuestion), id); + } + + logger.LogDebug("Found open question {QuestionId}", id); + return question; + } } diff --git a/src/Novelly.Api/Scenes/SceneEndpoints.cs b/src/Novelly.Api/Scenes/SceneEndpoints.cs index 424b159..53aa6a4 100644 --- a/src/Novelly.Api/Scenes/SceneEndpoints.cs +++ b/src/Novelly.Api/Scenes/SceneEndpoints.cs @@ -1,10 +1,12 @@ +using Novelly.Api.Common; + namespace Novelly.Api.Scenes; public static class SceneEndpoints { public static IEndpointRouteBuilder MapSceneEndpoints(this IEndpointRouteBuilder app) { - var chapterScoped = app.MapGroup("/api/chapters/{chapterId:guid}/scenes").WithTags("Scenes"); + var chapterScoped = app.MapGroup("/api/chapters/{chapterId:guid}/scenes").WithTags("Scenes").AddEndpointFilter(); chapterScoped.MapGet("/", async (Guid chapterId, SceneService service, CancellationToken ct) => Results.Ok(await service.ListAsync(chapterId, ct))) @@ -18,7 +20,7 @@ public static class SceneEndpoints }) .WithSummary("Add a scene to a chapter."); - var scenes = app.MapGroup("/api/scenes").WithTags("Scenes"); + var scenes = app.MapGroup("/api/scenes").WithTags("Scenes").AddEndpointFilter(); scenes.MapGet("/{id:guid}", async (Guid id, SceneService service, CancellationToken ct) => Results.Ok(await service.GetAsync(id, ct))) diff --git a/src/Novelly.Api/Scenes/SceneService.cs b/src/Novelly.Api/Scenes/SceneService.cs index 76e7d7c..6ac1042 100644 --- a/src/Novelly.Api/Scenes/SceneService.cs +++ b/src/Novelly.Api/Scenes/SceneService.cs @@ -5,10 +5,12 @@ using Novelly.Api.Data; namespace Novelly.Api.Scenes; -public class SceneService(INovelDbContext db) +public class SceneService(INovelDbContext db, ILogger logger) { public async Task> ListAsync(Guid chapterId, CancellationToken ct = default) { + logger.LogInformation("Listing scenes for chapter {ChapterId}", chapterId); + var scenes = await Query() .Where(s => s.ChapterId == chapterId) .OrderBy(s => s.SortOrder) @@ -17,13 +19,19 @@ public class SceneService(INovelDbContext db) return [.. scenes.Select(s => s.ToDto())]; } - public async Task GetAsync(Guid id, CancellationToken ct = default) => - (await FindAsync(id, ct)).ToDto(); + public async Task GetAsync(Guid id, CancellationToken ct = default) + { + logger.LogInformation("Getting scene {SceneId}", id); + return (await FindAsync(id, ct)).ToDto(); + } public async Task CreateAsync(Guid chapterId, CreateSceneRequest request, CancellationToken ct = default) { + logger.LogInformation("Creating scene {Title} for chapter {ChapterId}, prose length {ProseLength}", request.Title, chapterId, request.Prose?.Length ?? 0); + if (!await db.Chapters.AnyAsync(c => c.Id == chapterId, ct)) { + logger.LogWarning("Chapter {ChapterId} not found", chapterId); throw new NotFoundException(nameof(Chapter), chapterId); } @@ -50,6 +58,8 @@ public class SceneService(INovelDbContext db) public async Task UpdateAsync(Guid id, UpdateSceneRequest request, CancellationToken ct = default) { + logger.LogInformation("Updating scene {SceneId}, prose length {ProseLength}", id, request.Prose?.Length ?? 0); + var scene = await FindAsync(id, ct); scene.Title = Patch.Apply(scene.Title, request.Title) ?? scene.Title; @@ -76,6 +86,8 @@ public class SceneService(INovelDbContext db) public async Task DeleteAsync(Guid id, CancellationToken ct = default) { + logger.LogInformation("Deleting scene {SceneId}", id); + var scene = await FindAsync(id, ct); db.Scenes.Remove(scene); await db.SaveChangesAsync(ct); @@ -83,6 +95,8 @@ public class SceneService(INovelDbContext db) private async Task NextSortOrderAsync(Guid chapterId, CancellationToken ct) { + logger.LogDebug("Computing next sort order for chapter {ChapterId}", chapterId); + var max = await db.Scenes .Where(s => s.ChapterId == chapterId) .MaxAsync(s => (int?)s.SortOrder, ct); @@ -92,7 +106,18 @@ public class SceneService(INovelDbContext db) private IQueryable Query() => db.Scenes.Include(s => s.PovCharacter); - private async Task FindAsync(Guid id, CancellationToken ct) => - await Query().FirstOrDefaultAsync(s => s.Id == id, ct) - ?? throw new NotFoundException(nameof(Scene), id); + private async Task FindAsync(Guid id, CancellationToken ct) + { + logger.LogDebug("Finding scene {SceneId}", id); + + var scene = await Query().FirstOrDefaultAsync(s => s.Id == id, ct); + if (scene is null) + { + logger.LogWarning("Scene {SceneId} not found", id); + throw new NotFoundException(nameof(Scene), id); + } + + logger.LogDebug("Found scene {SceneId}", id); + return scene; + } } diff --git a/src/Novelly.Api/Tags/TagEndpoints.cs b/src/Novelly.Api/Tags/TagEndpoints.cs index f4f5af1..58c9940 100644 --- a/src/Novelly.Api/Tags/TagEndpoints.cs +++ b/src/Novelly.Api/Tags/TagEndpoints.cs @@ -1,10 +1,12 @@ +using Novelly.Api.Common; + namespace Novelly.Api.Tags; public static class TagEndpoints { public static IEndpointRouteBuilder MapTagEndpoints(this IEndpointRouteBuilder app) { - var projectScoped = app.MapGroup("/api/projects/{projectId:guid}/tags").WithTags("Tags"); + var projectScoped = app.MapGroup("/api/projects/{projectId:guid}/tags").WithTags("Tags").AddEndpointFilter(); projectScoped.MapGet("/", async (Guid projectId, TagService service, CancellationToken ct) => Results.Ok(await service.ListAsync(projectId, ct))) @@ -18,7 +20,7 @@ public static class TagEndpoints }) .WithSummary("Create a tag. Tags are also created on demand when applied by name."); - var tags = app.MapGroup("/api/tags").WithTags("Tags"); + var tags = app.MapGroup("/api/tags").WithTags("Tags").AddEndpointFilter(); tags.MapGet("/{id:guid}/references", async (Guid id, TagService service, CancellationToken ct) => Results.Ok(await service.GetReferencesAsync(id, ct))) diff --git a/src/Novelly.Api/Tags/TagService.cs b/src/Novelly.Api/Tags/TagService.cs index c2caf97..a32f358 100644 --- a/src/Novelly.Api/Tags/TagService.cs +++ b/src/Novelly.Api/Tags/TagService.cs @@ -5,27 +5,38 @@ using Novelly.Api.Projects; namespace Novelly.Api.Tags; -public class TagService(INovelDbContext db) +public class TagService(INovelDbContext db, ILogger logger) { - public async Task> ListAsync(Guid projectId, CancellationToken ct = default) => - await db.Tags + public async Task> ListAsync(Guid projectId, CancellationToken ct = default) + { + logger.LogInformation("Listing tags for project {ProjectId}", projectId); + + return await db.Tags .Where(t => t.ProjectId == projectId) .OrderBy(t => t.Name) .Select(t => new TagSummaryDto( t.Id, t.Name, t.Color, t.Characters.Count, t.Chapters.Count, t.Beats.Count)) .ToListAsync(ct); + } /// Everything in the project carrying this tag. public async Task GetReferencesAsync(Guid tagId, CancellationToken ct = default) { + logger.LogInformation("Getting references for tag {TagId}", tagId); + var tag = await db.Tags .Include(t => t.Characters) .Include(t => t.Chapters) .Include(t => t.Beats).ThenInclude(b => b.Character) .Include(t => t.Beats).ThenInclude(b => b.Chapter) - .FirstOrDefaultAsync(t => t.Id == tagId, ct) - ?? throw new NotFoundException(nameof(Tag), tagId); + .FirstOrDefaultAsync(t => t.Id == tagId, ct); + + if (tag is null) + { + logger.LogWarning("Tag {TagId} not found", tagId); + throw new NotFoundException(nameof(Tag), tagId); + } return new TagReferencesDto( tag.ToDto(), @@ -51,20 +62,25 @@ public class TagService(INovelDbContext db) public async Task CreateAsync(Guid projectId, CreateTagRequest request, CancellationToken ct = default) { + logger.LogInformation("Creating tag {Name} for project {ProjectId}", request.Name, projectId); + if (!await db.Projects.AnyAsync(p => p.Id == projectId, ct)) { + logger.LogWarning("Project {ProjectId} not found", projectId); throw new NotFoundException(nameof(Project), projectId); } var name = TagMapping.Normalise(request.Name); if (string.IsNullOrWhiteSpace(name)) { + logger.LogWarning("Rejected tag creation for project {ProjectId}: name was blank", projectId); throw new ArgumentException("A tag needs a name."); } var existing = await FindByNameAsync(projectId, name, ct); if (existing is not null) { + logger.LogWarning("Rejected tag creation for project {ProjectId}: '{Name}' already exists", projectId, existing.Name); throw new InvalidOperationException($"The project already has a tag called '{existing.Name}'."); } @@ -76,20 +92,28 @@ public class TagService(INovelDbContext db) public async Task UpdateAsync(Guid tagId, UpdateTagRequest request, CancellationToken ct = default) { - var tag = await db.Tags.FirstOrDefaultAsync(t => t.Id == tagId, ct) - ?? throw new NotFoundException(nameof(Tag), tagId); + logger.LogInformation("Updating tag {TagId}", tagId); + + var tag = await db.Tags.FirstOrDefaultAsync(t => t.Id == tagId, ct); + if (tag is null) + { + logger.LogWarning("Tag {TagId} not found", tagId); + throw new NotFoundException(nameof(Tag), tagId); + } if (request.Name is not null) { var name = TagMapping.Normalise(request.Name); if (string.IsNullOrWhiteSpace(name)) { + logger.LogWarning("Rejected update for tag {TagId}: name was blank", tagId); throw new ArgumentException("A tag needs a name."); } var clash = await FindByNameAsync(tag.ProjectId, name, ct); if (clash is not null && clash.Id != tag.Id) { + logger.LogWarning("Rejected update for tag {TagId}: '{Name}' already exists as {ClashTagId}", tagId, clash.Name, clash.Id); throw new InvalidOperationException($"The project already has a tag called '{clash.Name}'."); } @@ -104,8 +128,14 @@ public class TagService(INovelDbContext db) /// Deletes a tag. Whatever carried it keeps existing — only the label goes. public async Task DeleteAsync(Guid tagId, CancellationToken ct = default) { - var tag = await db.Tags.FirstOrDefaultAsync(t => t.Id == tagId, ct) - ?? throw new NotFoundException(nameof(Tag), tagId); + logger.LogInformation("Deleting tag {TagId}", tagId); + + var tag = await db.Tags.FirstOrDefaultAsync(t => t.Id == tagId, ct); + if (tag is null) + { + logger.LogWarning("Tag {TagId} not found", tagId); + throw new NotFoundException(nameof(Tag), tagId); + } db.Tags.Remove(tag); await db.SaveChangesAsync(ct); @@ -119,6 +149,8 @@ public class TagService(INovelDbContext db) internal async Task> ResolveAsync( Guid projectId, IReadOnlyList names, CancellationToken ct) { + logger.LogDebug("Resolving {Count} tag names for project {ProjectId}", names.Count, projectId); + var wanted = names .Select(TagMapping.Normalise) .Where(n => !string.IsNullOrWhiteSpace(n)) @@ -127,6 +159,7 @@ public class TagService(INovelDbContext db) if (wanted.Count == 0) { + logger.LogDebug("No usable tag names for project {ProjectId}", projectId); return []; } @@ -150,6 +183,7 @@ public class TagService(INovelDbContext db) resolved.Add(match); } + logger.LogDebug("Resolved {Count} tags for project {ProjectId}", resolved.Count, projectId); return resolved; } diff --git a/src/Novelly.Api/appsettings.Development.json b/src/Novelly.Api/appsettings.Development.json index 5997abb..2026d5b 100644 --- a/src/Novelly.Api/appsettings.Development.json +++ b/src/Novelly.Api/appsettings.Development.json @@ -5,5 +5,15 @@ "Novelly": "Debug", "Microsoft.AspNetCore": "Warning" } + }, + "Serilog": { + "MinimumLevel": { + "Default": "Information", + "Override": { + "Novelly": "Debug", + "Microsoft.AspNetCore": "Warning", + "Microsoft.AspNetCore.Diagnostics.ExceptionHandlerMiddleware": "Fatal" + } + } } } diff --git a/src/Novelly.Api/appsettings.json b/src/Novelly.Api/appsettings.json index 72510ac..2e4fc74 100644 --- a/src/Novelly.Api/appsettings.json +++ b/src/Novelly.Api/appsettings.json @@ -6,6 +6,17 @@ "Microsoft.EntityFrameworkCore.Database.Command": "Warning" } }, + "Serilog": { + "MinimumLevel": { + "Default": "Information", + "Override": { + "Novelly": "Information", + "Microsoft.AspNetCore": "Warning", + "Microsoft.AspNetCore.Diagnostics.ExceptionHandlerMiddleware": "Fatal", + "Microsoft.EntityFrameworkCore.Database.Command": "Warning" + } + } + }, "AllowedHosts": "*", "ConnectionStrings": { "Novel": "Data Source=novel.db" diff --git a/src/Novelly.AppHost/AppHost.cs b/src/Novelly.AppHost/AppHost.cs index 1155037..243f6ae 100644 --- a/src/Novelly.AppHost/AppHost.cs +++ b/src/Novelly.AppHost/AppHost.cs @@ -3,8 +3,7 @@ var builder = DistributedApplication.CreateBuilder(args); // Port 5080 is pinned to match src/Novelly.Web's Vite proxy default and the curl-based // smoke checks in CLAUDE.md, so the API sits at the same address whether it is started // on its own with `dotnet run` or through this AppHost. -var api = builder.AddProject("api") - .WithHttpEndpoint(port: 5080, name: "http"); +var api = builder.AddProject("api").WithHttpEndpoint(port: 5080, name: "http"); builder.AddViteApp("web", "../Novelly.Web", "dev") .WithReference(api) diff --git a/src/Novelly.Mcp/NovelApiClient.cs b/src/Novelly.Mcp/NovelApiClient.cs index 88d7bd0..f6fb835 100644 --- a/src/Novelly.Mcp/NovelApiClient.cs +++ b/src/Novelly.Mcp/NovelApiClient.cs @@ -17,8 +17,7 @@ public class NovelApiClient(HttpClient http) WriteIndented = true }; - public Task GetAsync(string path, CancellationToken ct = default) => - SendAsync(new HttpRequestMessage(HttpMethod.Get, path), ct); + public Task GetAsync(string path, CancellationToken ct = default) => SendAsync(new HttpRequestMessage(HttpMethod.Get, path), ct); public Task PostAsync(string path, object body, CancellationToken ct = default) => SendAsync(new HttpRequestMessage(HttpMethod.Post, path) @@ -32,8 +31,7 @@ public class NovelApiClient(HttpClient http) Content = JsonContent.Create(body, options: Options) }, ct); - public Task DeleteAsync(string path, CancellationToken ct = default) => - SendAsync(new HttpRequestMessage(HttpMethod.Delete, path), ct); + public Task DeleteAsync(string path, CancellationToken ct = default) => SendAsync(new HttpRequestMessage(HttpMethod.Delete, path), ct); /// /// Sends the request and shapes the outcome as a tool result. Failures come back as diff --git a/tests/Novelly.Api.Tests/AnthropicClientTests.cs b/tests/Novelly.Api.Tests/AnthropicClientTests.cs index 70430af..a98f46f 100644 --- a/tests/Novelly.Api.Tests/AnthropicClientTests.cs +++ b/tests/Novelly.Api.Tests/AnthropicClientTests.cs @@ -1,3 +1,4 @@ +using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; using Novelly.Api.Agent; using Novelly.Api.Common; @@ -13,7 +14,7 @@ public class AnthropicClientTests // (listing conversations, reading a transcript). Throwing at construction would // take those down on any install that has not configured a key yet. Assert.That( - () => new AnthropicAgentModelClient(Options.Create(new AgentOptions())), + () => new AnthropicAgentModelClient(Options.Create(new AgentOptions()), NullLogger.Instance), Throws.Nothing); [Test] @@ -24,7 +25,7 @@ public class AnthropicClientTests try { - var client = new AnthropicAgentModelClient(Options.Create(new AgentOptions())); + var client = new AnthropicAgentModelClient(Options.Create(new AgentOptions()), NullLogger.Instance); Assert.That( async () => await client.CompleteAsync("system", [], []), diff --git a/tests/Novelly.Api.Tests/CapturingLogger.cs b/tests/Novelly.Api.Tests/CapturingLogger.cs new file mode 100644 index 0000000..e8e5b39 --- /dev/null +++ b/tests/Novelly.Api.Tests/CapturingLogger.cs @@ -0,0 +1,24 @@ +using Microsoft.Extensions.Logging; + +namespace Novelly.Api.Tests; + +/// Records every entry logged through it, so tests can assert on what a service logged. +public record CapturedLogEntry(LogLevel Level, string Message, Exception? Exception); + +/// +/// A test double for that captures entries instead of +/// writing them anywhere, so tests can assert a service logged at the right level with the +/// right values without standing up a real sink. +/// +public class CapturingLogger : ILogger +{ + public List Entries { get; } = []; + + public IDisposable? BeginScope(TState state) where TState : notnull => null; + + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log( + LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func formatter) => + Entries.Add(new CapturedLogEntry(logLevel, formatter(state, exception), exception)); +} diff --git a/tests/Novelly.Api.Tests/ListingTests.cs b/tests/Novelly.Api.Tests/ListingTests.cs index b103d0e..f9e3fa2 100644 --- a/tests/Novelly.Api.Tests/ListingTests.cs +++ b/tests/Novelly.Api.Tests/ListingTests.cs @@ -97,7 +97,7 @@ public class ListingTests : ServiceTestFixture var agent = new NovelAgentService( Db.Context, new ScriptedModelClient([[new AgentTextBlock("Reply.")]]), - new NovelAgentToolset(Projects, Characters, Arcs, Chapters, Beats, Scenes, Tags, Questions), + new NovelAgentToolset(Projects, Characters, Arcs, Chapters, Beats, Scenes, Tags, Questions, NullLogger.Instance), Options.Create(new AgentOptions()), NullLogger.Instance); diff --git a/tests/Novelly.Api.Tests/LoggingTests.cs b/tests/Novelly.Api.Tests/LoggingTests.cs new file mode 100644 index 0000000..b4f30a1 --- /dev/null +++ b/tests/Novelly.Api.Tests/LoggingTests.cs @@ -0,0 +1,88 @@ +using Microsoft.Extensions.Logging; +using Novelly.Api.Beats; +using Novelly.Api.Chapters; +using Novelly.Api.Characters; +using Novelly.Api.Common; +using Novelly.Api.Projects; + +namespace Novelly.Api.Tests; + +/// +/// Covers the logging behaviour added across the services: a warning fires before a +/// not-found is thrown, and prose bodies never leak into a log message. +/// +[TestFixture] +public class LoggingTests : ServiceTestFixture +{ + [Test] + public void Fetching_a_missing_chapter_logs_a_warning_before_throwing() + { + var missingId = Guid.NewGuid(); + + Assert.That(() => Chapters.GetAsync(missingId), Throws.TypeOf()); + + var warning = ChapterLogs.Entries.Single(e => e.Level == LogLevel.Warning); + Assert.That(warning.Message, Does.Contain(missingId.ToString())); + } + + [Test] + public async Task Creating_a_chapter_logs_the_project_and_title_at_information() + { + var project = await Projects.CreateAsync(new CreateProjectRequest("The Salt Road")); + ChapterLogs.Entries.Clear(); + + await Chapters.CreateAsync(project.Id, new CreateChapterRequest("Landfall")); + + var info = ChapterLogs.Entries.Single(e => e.Level == LogLevel.Information); + Assert.Multiple(() => + { + Assert.That(info.Message, Does.Contain("Landfall")); + Assert.That(info.Message, Does.Contain(project.Id.ToString())); + }); + } + + [Test] + public async Task Logged_values_never_include_a_chapter_summary_body() + { + var project = await Projects.CreateAsync(new CreateProjectRequest("The Salt Road")); + const string secretSummary = "A very specific plot twist nobody should see in a log line."; + ChapterLogs.Entries.Clear(); + + await Chapters.CreateAsync(project.Id, new CreateChapterRequest("Landfall", Summary: secretSummary)); + + Assert.That(ChapterLogs.Entries.Select(e => e.Message), Has.None.Contain(secretSummary)); + } + + [Test] + public async Task Deleting_a_project_logs_information_before_the_lookup() + { + var project = await Projects.CreateAsync(new CreateProjectRequest("The Salt Road")); + ProjectLogs.Entries.Clear(); + + await Projects.DeleteAsync(project.Id); + + Assert.That( + ProjectLogs.Entries, + Has.Some.Matches(e => e.Level == LogLevel.Information && e.Message.Contains(project.Id.ToString()))); + } + + [Test] + public async Task Rejecting_a_beat_with_a_foreign_character_logs_a_warning_not_an_error() + { + var projectA = await Projects.CreateAsync(new CreateProjectRequest("Project A")); + var projectB = await Projects.CreateAsync(new CreateProjectRequest("Project B")); + var chapter = await Chapters.CreateAsync(projectA.Id, new CreateChapterRequest("Landfall")); + var foreignCharacter = await Characters.CreateAsync(projectB.Id, new CreateCharacterRequest("Ines")); + BeatLogs.Entries.Clear(); + + Assert.That( + () => Beats.CreateAsync(chapter.Id, new CreateBeatRequest("Arrival", CharacterId: foreignCharacter.Id)), + Throws.TypeOf()); + + Assert.Multiple(() => + { + Assert.That(BeatLogs.Entries.Where(e => e.Level == LogLevel.Error), Is.Empty); + Assert.That(BeatLogs.Entries.Any(e => e.Level == LogLevel.Warning), Is.True); + }); + } +} diff --git a/tests/Novelly.Api.Tests/NovelAgentServiceTests.cs b/tests/Novelly.Api.Tests/NovelAgentServiceTests.cs index 1fd1be0..3408867 100644 --- a/tests/Novelly.Api.Tests/NovelAgentServiceTests.cs +++ b/tests/Novelly.Api.Tests/NovelAgentServiceTests.cs @@ -12,7 +12,7 @@ public class NovelAgentServiceTests : ServiceTestFixture private NovelAgentToolset _toolset = null!; protected override void OnSetUp() => - _toolset = new NovelAgentToolset(Projects, Characters, Arcs, Chapters, Beats, Scenes, Tags, Questions); + _toolset = new NovelAgentToolset(Projects, Characters, Arcs, Chapters, Beats, Scenes, Tags, Questions, NullLogger.Instance); private NovelAgentService BuildAgent(ScriptedModelClient model) => new( Db.Context, diff --git a/tests/Novelly.Api.Tests/ServiceTestFixture.cs b/tests/Novelly.Api.Tests/ServiceTestFixture.cs index a56cae4..d4d4db9 100644 --- a/tests/Novelly.Api.Tests/ServiceTestFixture.cs +++ b/tests/Novelly.Api.Tests/ServiceTestFixture.cs @@ -29,18 +29,37 @@ public abstract class ServiceTestFixture protected CharacterArcService Arcs { get; private set; } = null!; protected OpenQuestionService Questions { get; private set; } = null!; + protected CapturingLogger ProjectLogs { get; private set; } = null!; + protected CapturingLogger CharacterLogs { get; private set; } = null!; + protected CapturingLogger ChapterLogs { get; private set; } = null!; + protected CapturingLogger SceneLogs { get; private set; } = null!; + protected CapturingLogger BeatLogs { get; private set; } = null!; + protected CapturingLogger TagLogs { get; private set; } = null!; + protected CapturingLogger ArcLogs { get; private set; } = null!; + protected CapturingLogger QuestionLogs { get; private set; } = null!; + [SetUp] public void SetUpFixture() { Db = new TestDatabase(); - Tags = new TagService(Db.Context); - Projects = new ProjectService(Db.Context); - Characters = new CharacterService(Db.Context, Tags); - Chapters = new ChapterService(Db.Context, Tags); - Scenes = new SceneService(Db.Context); - Beats = new BeatService(Db.Context, Tags); - Arcs = new CharacterArcService(Db.Context); - Questions = new OpenQuestionService(Db.Context); + + TagLogs = new CapturingLogger(); + ProjectLogs = new CapturingLogger(); + CharacterLogs = new CapturingLogger(); + ChapterLogs = new CapturingLogger(); + SceneLogs = new CapturingLogger(); + BeatLogs = new CapturingLogger(); + ArcLogs = new CapturingLogger(); + QuestionLogs = new CapturingLogger(); + + Tags = new TagService(Db.Context, TagLogs); + Projects = new ProjectService(Db.Context, ProjectLogs); + Characters = new CharacterService(Db.Context, Tags, CharacterLogs); + Chapters = new ChapterService(Db.Context, Tags, ChapterLogs); + Scenes = new SceneService(Db.Context, SceneLogs); + Beats = new BeatService(Db.Context, Tags, BeatLogs); + Arcs = new CharacterArcService(Db.Context, ArcLogs); + Questions = new OpenQuestionService(Db.Context, QuestionLogs); OnSetUp(); }