diff --git a/src/Novelly.Api/Agent/AgentEndpoints.cs b/src/Novelly.Api/Agent/AgentEndpoints.cs index 67b31dd..c53a0c9 100644 --- a/src/Novelly.Api/Agent/AgentEndpoints.cs +++ b/src/Novelly.Api/Agent/AgentEndpoints.cs @@ -23,7 +23,9 @@ public static class AgentEndpoints CancellationToken ct) => { var reply = await agent.SendMessageAsync(projectId, request, ct); - return Results.Ok(new AgentTurnResponse(reply.ConversationId, reply.ToResponse())); + return reply is null + ? Results.NotFound() + : Results.Ok(new AgentTurnResponse(reply.ConversationId, reply.ToResponse())); }) .WithSummary("Send a message to the writing agent and run it to completion."); diff --git a/src/Novelly.Api/Agent/NovelAgentService.cs b/src/Novelly.Api/Agent/NovelAgentService.cs index ad073ef..acfff4c 100644 --- a/src/Novelly.Api/Agent/NovelAgentService.cs +++ b/src/Novelly.Api/Agent/NovelAgentService.cs @@ -71,9 +71,11 @@ public class NovelAgentService( /// /// Sends a message to the agent and runs it to completion, executing any tools it - /// calls along the way. Returns the assistant's final turn. + /// calls along the way. Returns the assistant's final turn. Null when no project has + /// this id, or the request names a conversation that does not exist — a lookup miss + /// is expected, not exceptional. /// - public async Task SendMessageAsync( + public async Task SendMessageAsync( Guid projectId, SendAgentMessageRequest request, CancellationToken ct = default) { Guard.Default(projectId, nameof(projectId)); @@ -84,19 +86,37 @@ public class NovelAgentService( "Sending agent message for project {ProjectId}, conversation {ConversationId}, message length {MessageLength}", projectId, request.ConversationId, request.Message.Length); - var conversation = request.ConversationId is { } id - ? await FindConversationAsync(id, ct) - // The id came from the request body, not the route — an unknown id here - // is bad input to this call, not a direct "fetch conversation" lookup. - ?? throw new NotFoundException(nameof(AgentConversation), id) - : await StartConversationAsync(projectId, request.Message, ct); + var project = await db.Projects.FirstOrDefaultAsync(p => p.Id == projectId, ct); + if (project is null) + { + logger.LogInformation("Project {ProjectId} not found", projectId); + return null; + } + + AgentConversation conversation; + if (request.ConversationId is { } id) + { + // The id came from the request body, not the route — an unknown id here + // is bad input to this call, not a direct "fetch conversation" lookup. + var found = await FindConversationAsync(id, ct); + if (found is null) + { + return null; + } + + conversation = found; + } + else + { + conversation = StartConversation(projectId, request.Message); + } // Persist the user's turn before running the loop. The tools save through the // same DbContext, so leaving this pending would entangle it with their writes — // and recording the question even if the model call fails is the behaviour we want. await AppendMessageAsync(conversation, AgentRole.User, request.Message, null, ct); - var systemPrompt = await BuildSystemPromptAsync(projectId, ct); + var systemPrompt = BuildSystemPrompt(project); var transcript = BuildTranscript(conversation); var toolCalls = new List(); var text = new StringBuilder(); @@ -197,17 +217,10 @@ public class NovelAgentService( return message; } - private async Task StartConversationAsync( - Guid projectId, string firstMessage, CancellationToken ct) + private AgentConversation StartConversation(Guid projectId, string firstMessage) { 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); - } - var conversation = new AgentConversation { ProjectId = projectId, @@ -249,17 +262,8 @@ public class NovelAgentService( [new AgentTextBlock(m.Content)])) ]; - private async Task BuildSystemPromptAsync(Guid projectId, CancellationToken ct) + private static string BuildSystemPrompt(Project project) { - 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}"); if (!string.IsNullOrWhiteSpace(project.Genre)) brief.AppendLine($"Genre: {project.Genre}"); diff --git a/src/Novelly.Api/Agent/NovelAgentToolset.cs b/src/Novelly.Api/Agent/NovelAgentToolset.cs index 787b007..f619998 100644 --- a/src/Novelly.Api/Agent/NovelAgentToolset.cs +++ b/src/Novelly.Api/Agent/NovelAgentToolset.cs @@ -83,11 +83,6 @@ public class NovelAgentToolset( 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); @@ -157,7 +152,7 @@ public class NovelAgentToolset( "Add a character dossier. Name is the only requirement — leave fields blank when " + "the writer has not decided them yet rather than inventing detail.", CharacterSchema(includeName: true, nameRequired: true).Build(), - async (projectId, input, ct) => (await characters.CreateAsync(projectId, new CreateCharacterRequest( + async (projectId, input, ct) => await OrNotFound(characters.CreateAsync(projectId, new CreateCharacterRequest( JsonInput.RequiredString(input, "name"), JsonInput.Enum(input, "role") ?? CharacterRole.Supporting, JsonInput.Enum(input, "importance") ?? CharacterImportance.Supporting, @@ -174,7 +169,7 @@ public class NovelAgentToolset( JsonInput.String(input, "arc_summary"), JsonInput.String(input, "voice"), JsonInput.String(input, "notes"), - JsonInput.Strings(input, "tags")), ct)).ToResponse()); + JsonInput.Strings(input, "tags")), ct), c => c.ToResponse(), "Project", projectId)); yield return new AgentTool( "update_character", @@ -224,16 +219,20 @@ public class NovelAgentToolset( .Str("chapter_id", "Id of the chapter the beat belongs to.", required: true) .Str("title", "Three to five words naming the beat.", required: true) .Build(), - async (_, input, ct) => (await beats.CreateAsync( - JsonInput.RequiredGuid(input, "chapter_id"), - new CreateBeatRequest( - JsonInput.RequiredString(input, "title"), - JsonInput.Int(input, "sort_order"), - JsonInput.Guid(input, "character_id"), - JsonInput.String(input, "what_happened"), - JsonInput.String(input, "whats_next"), - JsonInput.Guid(input, "scene_id"), - JsonInput.Strings(input, "tags")), ct)).ToResponse()); + async (_, input, ct) => + { + var chapterId = JsonInput.RequiredGuid(input, "chapter_id"); + return await OrNotFound(beats.CreateAsync( + chapterId, + new CreateBeatRequest( + JsonInput.RequiredString(input, "title"), + JsonInput.Int(input, "sort_order"), + JsonInput.Guid(input, "character_id"), + JsonInput.String(input, "what_happened"), + JsonInput.String(input, "whats_next"), + JsonInput.Guid(input, "scene_id"), + JsonInput.Strings(input, "tags")), ct), b => b.ToResponse(), "Chapter", chapterId); + }); yield return new AgentTool( "update_beat", @@ -278,12 +277,16 @@ public class NovelAgentToolset( .Str("chapter_id", "Id of the chapter whose beats to reorder.", required: true) .StringArray("beat_ids", "Beat ids in their new order.", required: true) .Build(), - async (_, input, ct) => (await beats.ReorderAsync( - JsonInput.RequiredGuid(input, "chapter_id"), - new ReorderBeatsRequest( - [.. (JsonInput.Strings(input, "beat_ids") ?? []) - .Select(id => Guid.TryParse(id, out var g) ? g : Guid.Empty) - .Where(g => g != Guid.Empty)]), ct)).Select(b => b.ToResponse())); + async (_, input, ct) => + { + var chapterId = JsonInput.RequiredGuid(input, "chapter_id"); + return await OrNotFound(beats.ReorderAsync( + chapterId, + new ReorderBeatsRequest( + [.. (JsonInput.Strings(input, "beat_ids") ?? []) + .Select(id => Guid.TryParse(id, out var g) ? g : Guid.Empty) + .Where(g => g != Guid.Empty)]), ct), list => list.Select(b => b.ToResponse()), "Chapter", chapterId); + }); yield return new AgentTool( "list_tags", @@ -337,7 +340,7 @@ public class NovelAgentToolset( .Int("target_word_count", "Target length in words.") .StringArray("tags", "Tags for cross-referencing. Replaces the existing tags.") .Build(), - async (projectId, input, ct) => (await chapters.CreateAsync(projectId, new CreateChapterRequest( + async (projectId, input, ct) => await OrNotFound(chapters.CreateAsync(projectId, new CreateChapterRequest( JsonInput.RequiredString(input, "title"), JsonInput.Int(input, "number"), JsonInput.String(input, "summary"), @@ -346,7 +349,7 @@ public class NovelAgentToolset( JsonInput.String(input, "notes"), JsonInput.Enum(input, "status") ?? DraftStatus.Planned, JsonInput.Int(input, "target_word_count"), - JsonInput.Strings(input, "tags")), ct)).ToResponse()); + JsonInput.Strings(input, "tags")), ct), c => c.ToResponse(), "Project", projectId)); yield return new AgentTool( "update_chapter", @@ -388,19 +391,23 @@ public class NovelAgentToolset( .Str("chapter_id", "Id of the chapter the scene belongs to.", required: true) .Str("title", "Scene title.", required: true) .Build(), - async (_, input, ct) => (await scenes.CreateAsync( - JsonInput.RequiredGuid(input, "chapter_id"), - new CreateSceneRequest( - JsonInput.RequiredString(input, "title"), - JsonInput.Int(input, "sort_order"), - JsonInput.String(input, "summary"), - JsonInput.String(input, "goal"), - JsonInput.String(input, "conflict"), - JsonInput.String(input, "outcome"), - JsonInput.Guid(input, "pov_character_id"), - JsonInput.String(input, "location"), - JsonInput.String(input, "prose"), - JsonInput.Enum(input, "status") ?? DraftStatus.Planned), ct)).ToResponse()); + async (_, input, ct) => + { + var chapterId = JsonInput.RequiredGuid(input, "chapter_id"); + return await OrNotFound(scenes.CreateAsync( + chapterId, + new CreateSceneRequest( + JsonInput.RequiredString(input, "title"), + JsonInput.Int(input, "sort_order"), + JsonInput.String(input, "summary"), + JsonInput.String(input, "goal"), + JsonInput.String(input, "conflict"), + JsonInput.String(input, "outcome"), + JsonInput.Guid(input, "pov_character_id"), + JsonInput.String(input, "location"), + JsonInput.String(input, "prose"), + JsonInput.Enum(input, "status") ?? DraftStatus.Planned), ct), s => s.ToResponse(), "Chapter", chapterId); + }); yield return new AgentTool( "update_scene", @@ -464,13 +471,17 @@ public class NovelAgentToolset( .Str("character_id", "Id of the character whose arc to add to.", required: true) .Str("title", "A short handle for the change, three to five words.", required: true) .Build(), - async (_, input, ct) => (await arcs.CreateAsync( - JsonInput.RequiredGuid(input, "character_id"), - new CreateArcStageRequest( - JsonInput.RequiredString(input, "title"), - JsonInput.Int(input, "sort_order"), - JsonInput.String(input, "description"), - JsonInput.Guid(input, "chapter_id")), ct)).ToResponse()); + async (_, input, ct) => + { + var characterId = JsonInput.RequiredGuid(input, "character_id"); + return await OrNotFound(arcs.CreateAsync( + characterId, + new CreateArcStageRequest( + JsonInput.RequiredString(input, "title"), + JsonInput.Int(input, "sort_order"), + JsonInput.String(input, "description"), + JsonInput.Guid(input, "chapter_id")), ct), s => s.ToResponse(), "Character", characterId); + }); yield return new AgentTool( "update_arc_stage", @@ -511,10 +522,14 @@ public class NovelAgentToolset( .Str("character_id", "Id of the character whose arc to reorder.", required: true) .StringArray("stage_ids", "Arc stage ids in the order wanted.", required: true) .Build(), - async (_, input, ct) => (await arcs.ReorderAsync( - JsonInput.RequiredGuid(input, "character_id"), - new ReorderArcStagesRequest( - [.. JsonInput.Strings(input, "stage_ids")?.Select(Guid.Parse) ?? []]), ct)).Select(s => s.ToResponse())); + async (_, input, ct) => + { + var characterId = JsonInput.RequiredGuid(input, "character_id"); + return await OrNotFound(arcs.ReorderAsync( + characterId, + new ReorderArcStagesRequest( + [.. JsonInput.Strings(input, "stage_ids")?.Select(Guid.Parse) ?? []]), ct), list => list.Select(s => s.ToResponse()), "Character", characterId); + }); yield return new AgentTool( "list_open_questions", @@ -543,13 +558,13 @@ public class NovelAgentToolset( .Str("chapter_id", "The chapter outline this is about, if any.") .Str("character_id", "The character this is about, if any.") .Build(), - async (projectId, input, ct) => (await questions.CreateAsync( + async (projectId, input, ct) => await OrNotFound(questions.CreateAsync( projectId, new CreateOpenQuestionRequest( JsonInput.RequiredString(input, "question"), JsonInput.String(input, "detail"), JsonInput.Guid(input, "chapter_id"), - JsonInput.Guid(input, "character_id")), ct)).ToResponse()); + JsonInput.Guid(input, "character_id")), ct), q => q.ToResponse(), "Project", projectId)); yield return new AgentTool( "resolve_open_question", diff --git a/src/Novelly.Api/Beats/BeatEndpoints.cs b/src/Novelly.Api/Beats/BeatEndpoints.cs index a638d59..49f1399 100644 --- a/src/Novelly.Api/Beats/BeatEndpoints.cs +++ b/src/Novelly.Api/Beats/BeatEndpoints.cs @@ -18,14 +18,20 @@ public static class BeatEndpoints chapterScoped.MapPost("/", async ( Guid chapterId, CreateBeatRequest request, BeatService service, CancellationToken ct) => { - var created = (await service.CreateAsync(chapterId, request, ct)).ToResponse(); + var beat = await service.CreateAsync(chapterId, request, ct); + if (beat is null) + { + return Results.NotFound(); + } + + var created = beat.ToResponse(); return Results.Created($"/api/beats/{created.Id}", created); }) .WithSummary("Add a beat to a chapter's outline."); chapterScoped.MapPost("/reorder", async ( Guid chapterId, ReorderBeatsRequest request, BeatService service, CancellationToken ct) => - Results.Ok((await service.ReorderAsync(chapterId, request, ct)).Select(b => b.ToResponse()))) + (await service.ReorderAsync(chapterId, request, ct))?.Select(b => b.ToResponse()).ToList().ToApiResult()) .WithSummary("Renumber a chapter's beats to match the order given."); app.MapGet("/api/characters/{characterId:guid}/beats", async ( diff --git a/src/Novelly.Api/Beats/BeatService.cs b/src/Novelly.Api/Beats/BeatService.cs index 778131a..5fbe8c0 100644 --- a/src/Novelly.Api/Beats/BeatService.cs +++ b/src/Novelly.Api/Beats/BeatService.cs @@ -74,7 +74,8 @@ public class BeatService( ]; } - public async Task CreateAsync(Guid chapterId, CreateBeatRequest request, CancellationToken ct = default) + /// Null when no chapter has this id — a lookup miss is expected, not exceptional. + public async Task CreateAsync(Guid chapterId, CreateBeatRequest request, CancellationToken ct = default) { Guard.Default(chapterId, nameof(chapterId)); Guard.Null(request, nameof(request)); @@ -85,8 +86,8 @@ public class BeatService( var chapter = await db.Chapters.FirstOrDefaultAsync(c => c.Id == chapterId, ct); if (chapter is null) { - logger.LogWarning("Rejected beat creation: chapter {ChapterId} not found", chapterId); - throw new NotFoundException(nameof(Chapter), chapterId); + logger.LogInformation("Rejected beat creation: chapter {ChapterId} not found", chapterId); + return null; } await ValidateReferencesAsync(chapter, request.CharacterId, request.SceneId, ct); @@ -131,10 +132,10 @@ public class BeatService( var chapter = await db.Chapters.FirstOrDefaultAsync(c => c.Id == beat.ChapterId, ct); if (chapter is null) { - // The beat's own chapter should always exist via the FK — this is an - // invariant failing, not a caller mistake, so it stays exceptional. + // The beat's own chapter should always exist via the FK — an invariant + // failing, not a caller mistake, but still not found so still just null. logger.LogError("Beat {BeatId} references chapter {ChapterId} which does not exist", id, beat.ChapterId); - throw new NotFoundException(nameof(Chapter), beat.ChapterId); + return null; } await ValidateReferencesAsync(chapter, request.CharacterId, request.SceneId, ct); @@ -178,7 +179,8 @@ public class BeatService( /// Renumbers a chapter's beats to match the order given. Sending the whole list beats /// patching sort orders one at a time, which is fiddly to get right from a drag handle. /// - public async Task> ReorderAsync( + /// Null when the chapter carries a beat id it does not own — a lookup miss is expected, not exceptional. + public async Task?> ReorderAsync( Guid chapterId, ReorderBeatsRequest request, CancellationToken ct = default) { Guard.Default(chapterId, nameof(chapterId)); @@ -192,8 +194,8 @@ public class BeatService( 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]); + logger.LogInformation("Reorder for chapter {ChapterId} referenced missing beat {BeatId}", chapterId, missing[0]); + return null; } // Listed beats take the order given; anything omitted keeps its relative position diff --git a/src/Novelly.Api/Chapters/ChapterEndpoints.cs b/src/Novelly.Api/Chapters/ChapterEndpoints.cs index bded637..4244e87 100644 --- a/src/Novelly.Api/Chapters/ChapterEndpoints.cs +++ b/src/Novelly.Api/Chapters/ChapterEndpoints.cs @@ -18,7 +18,13 @@ public static class ChapterEndpoints projectScoped.MapPost("/", async ( Guid projectId, CreateChapterRequest request, ChapterService service, CancellationToken ct) => { - var created = (await service.CreateAsync(projectId, request, ct)).ToResponse(); + var chapter = await service.CreateAsync(projectId, request, ct); + if (chapter is null) + { + return Results.NotFound(); + } + + var created = chapter.ToResponse(); return Results.Created($"/api/chapters/{created.Id}", created); }) .WithSummary("Add a chapter."); diff --git a/src/Novelly.Api/Chapters/ChapterService.cs b/src/Novelly.Api/Chapters/ChapterService.cs index 13a5c43..de3c8ab 100644 --- a/src/Novelly.Api/Chapters/ChapterService.cs +++ b/src/Novelly.Api/Chapters/ChapterService.cs @@ -2,7 +2,6 @@ using Microsoft.EntityFrameworkCore; using Novelly.Api.Common; using Novelly.Api.Common.Validation; using Novelly.Api.Data; -using Novelly.Api.Projects; using Novelly.Api.Tags; namespace Novelly.Api.Chapters; @@ -39,7 +38,8 @@ public class ChapterService( return await FindAsync(id, ct); } - public async Task CreateAsync(Guid projectId, CreateChapterRequest request, CancellationToken ct = default) + /// Null when no project has this id — a lookup miss is expected, not exceptional. + public async Task CreateAsync(Guid projectId, CreateChapterRequest request, CancellationToken ct = default) { Guard.Default(projectId, nameof(projectId)); Guard.Null(request, nameof(request)); @@ -49,8 +49,8 @@ public class ChapterService( if (!await db.Projects.AnyAsync(p => p.Id == projectId, ct)) { - logger.LogWarning("Rejected chapter creation: project {ProjectId} not found", projectId); - throw new NotFoundException(nameof(Project), projectId); + logger.LogInformation("Rejected chapter creation: project {ProjectId} not found", projectId); + return null; } var chapter = new Chapter diff --git a/src/Novelly.Api/Characters/CharacterArcService.cs b/src/Novelly.Api/Characters/CharacterArcService.cs index 2c1b74d..aa28b32 100644 --- a/src/Novelly.Api/Characters/CharacterArcService.cs +++ b/src/Novelly.Api/Characters/CharacterArcService.cs @@ -44,7 +44,8 @@ public class CharacterArcService( return await FindAsync(id, ct); } - public async Task CreateAsync( + /// Null when no character has this id — a lookup miss is expected, not exceptional. + public async Task CreateAsync( Guid characterId, CreateArcStageRequest request, CancellationToken ct = default) { Guard.Default(characterId, nameof(characterId)); @@ -56,8 +57,8 @@ public class CharacterArcService( var character = await db.Characters.FirstOrDefaultAsync(c => c.Id == characterId, ct); if (character is null) { - logger.LogWarning("Rejected arc stage creation: character {CharacterId} not found", characterId); - throw new NotFoundException(nameof(Character), characterId); + logger.LogInformation("Rejected arc stage creation: character {CharacterId} not found", characterId); + return null; } await EnsureChapterIsInSameProjectAsync(character, request.ChapterId, ct); @@ -96,10 +97,10 @@ public class CharacterArcService( var character = await db.Characters.FirstOrDefaultAsync(c => c.Id == stage.CharacterId, ct); if (character is null) { - // The stage's own character should always exist via the FK — this is an - // invariant failing, not a caller mistake, so it stays exceptional. + // The stage's own character should always exist via the FK — an invariant + // failing, not a caller mistake, but still not found so still just null. logger.LogError("Arc stage {ArcStageId} references character {CharacterId} which does not exist", id, stage.CharacterId); - throw new NotFoundException(nameof(Character), stage.CharacterId); + return null; } await EnsureChapterIsInSameProjectAsync(character, request.ChapterId, ct); @@ -136,7 +137,8 @@ public class CharacterArcService( /// Renumbers a character's arc to match the order given. Stages left out keep their /// relative position after the ones listed, exactly as beat reordering works. /// - public async Task> ReorderAsync( + /// Null when the character carries a stage id it does not own — a lookup miss is expected, not exceptional. + public async Task?> ReorderAsync( Guid characterId, ReorderArcStagesRequest request, CancellationToken ct = default) { Guard.Default(characterId, nameof(characterId)); @@ -152,8 +154,8 @@ public class CharacterArcService( 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]); + logger.LogInformation("Reorder for character {CharacterId} referenced missing arc stage {ArcStageId}", characterId, missing[0]); + return null; } var order = 1; diff --git a/src/Novelly.Api/Characters/CharacterEndpoints.cs b/src/Novelly.Api/Characters/CharacterEndpoints.cs index aba119e..e718f9f 100644 --- a/src/Novelly.Api/Characters/CharacterEndpoints.cs +++ b/src/Novelly.Api/Characters/CharacterEndpoints.cs @@ -18,7 +18,13 @@ public static class CharacterEndpoints projectScoped.MapPost("/", async ( Guid projectId, CreateCharacterRequest request, CharacterService service, CancellationToken ct) => { - var created = (await service.CreateAsync(projectId, request, ct)).ToResponse(); + var character = await service.CreateAsync(projectId, request, ct); + if (character is null) + { + return Results.NotFound(); + } + + var created = character.ToResponse(); return Results.Created($"/api/characters/{created.Id}", created); }) .WithSummary("Add a character dossier."); @@ -58,14 +64,20 @@ public static class CharacterEndpoints characters.MapPost("/{id:guid}/arc", async ( Guid id, CreateArcStageRequest request, CharacterArcService service, CancellationToken ct) => { - var created = (await service.CreateAsync(id, request, ct)).ToResponse(); + var stage = await service.CreateAsync(id, request, ct); + if (stage is null) + { + return Results.NotFound(); + } + + var created = stage.ToResponse(); return Results.Created($"/api/arc-stages/{created.Id}", created); }) .WithSummary("Add a stage to a character's arc."); characters.MapPost("/{id:guid}/arc/reorder", async ( Guid id, ReorderArcStagesRequest request, CharacterArcService service, CancellationToken ct) => - Results.Ok((await service.ReorderAsync(id, request, ct)).Select(s => s.ToResponse()))) + (await service.ReorderAsync(id, request, ct))?.Select(s => s.ToResponse()).ToList().ToApiResult()) .WithSummary("Renumber a character's arc to match the order given."); var arcStages = app.MapGroup("/api/arc-stages").WithTags("Characters") diff --git a/src/Novelly.Api/Characters/CharacterService.cs b/src/Novelly.Api/Characters/CharacterService.cs index 299f4f4..25e0e31 100644 --- a/src/Novelly.Api/Characters/CharacterService.cs +++ b/src/Novelly.Api/Characters/CharacterService.cs @@ -2,7 +2,6 @@ using Microsoft.EntityFrameworkCore; using Novelly.Api.Common; using Novelly.Api.Common.Validation; using Novelly.Api.Data; -using Novelly.Api.Projects; using Novelly.Api.Tags; namespace Novelly.Api.Characters; @@ -53,7 +52,8 @@ public class CharacterService( return await FindAsync(id, ct); } - public async Task CreateAsync(Guid projectId, CreateCharacterRequest request, CancellationToken ct = default) + /// Null when no project has this id — a lookup miss is expected, not exceptional. + public async Task CreateAsync(Guid projectId, CreateCharacterRequest request, CancellationToken ct = default) { Guard.Default(projectId, nameof(projectId)); Guard.Null(request, nameof(request)); @@ -61,7 +61,11 @@ public class CharacterService( logger.LogInformation("Creating character {Name} for project {ProjectId}, role {Role}, importance {Importance}", request.Name, projectId, request.Role, request.Importance); - await EnsureProjectExists(projectId, ct); + if (!await db.Projects.AnyAsync(p => p.Id == projectId, ct)) + { + logger.LogInformation("Rejected character creation: project {ProjectId} not found", projectId); + return null; + } var character = new Character { @@ -174,8 +178,8 @@ public class CharacterService( var related = await db.Characters.FirstOrDefaultAsync(c => c.Id == request.RelatedCharacterId, ct); if (related is null) { - logger.LogWarning("Rejected relationship: related character {RelatedCharacterId} not found", request.RelatedCharacterId); - throw new NotFoundException(nameof(Character), request.RelatedCharacterId); + logger.LogInformation("Rejected relationship: related character {RelatedCharacterId} not found", request.RelatedCharacterId); + return null; } if (related.ProjectId != character.ProjectId) @@ -239,15 +243,4 @@ public class CharacterService( 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("Rejected character creation: project {ProjectId} not found", projectId); - throw new NotFoundException(nameof(Project), projectId); - } - } } diff --git a/src/Novelly.Api/Common/NotFoundException.cs b/src/Novelly.Api/Common/NotFoundException.cs deleted file mode 100644 index 55e0993..0000000 --- a/src/Novelly.Api/Common/NotFoundException.cs +++ /dev/null @@ -1,11 +0,0 @@ -namespace Novelly.Api.Common; - -/// -/// Thrown when a service is asked for an entity that does not exist. The API translates -/// this into a 404 so services never have to know about HTTP. -/// -public class NotFoundException(string entity, Guid id) : Exception($"{entity} '{id}' was not found.") -{ - public string Entity { get; } = entity; - public Guid Id { get; } = id; -} diff --git a/src/Novelly.Api/Imports/ImportAgentToolset.cs b/src/Novelly.Api/Imports/ImportAgentToolset.cs index 9d5268a..d40fb44 100644 --- a/src/Novelly.Api/Imports/ImportAgentToolset.cs +++ b/src/Novelly.Api/Imports/ImportAgentToolset.cs @@ -8,6 +8,13 @@ using Novelly.Api.Projects; namespace Novelly.Api.Imports; +/// +/// A lookup a tool performed came back empty. Not an exception — the underlying service +/// already said so by returning null — just a value +/// recognises and turns into the same error-result shape a caught exception would produce. +/// +internal record ImportToolNotFound(string Message); + /// A tool the import agent can call, bound to a handler that runs against this run's state. internal record ImportAgentTool( string Name, @@ -71,14 +78,16 @@ public class ImportAgentToolset( try { var result = await tool.Handler(input, ct); + + if (result is ImportToolNotFound notFound) + { + logger.LogInformation("Import tool {Tool} found nothing: {Message}", name, notFound.Message); + return new AgentToolResult(notFound.Message, true); + } + logger.LogDebug("Import tool {Tool} succeeded", name); return new AgentToolResult(JsonSerializer.Serialize(result, SerializerOptions), false); } - catch (NotFoundException ex) - { - logger.LogWarning(ex, "Import tool {Tool} failed: not found", name); - return new AgentToolResult(ex.Message, true); - } catch (ArgumentException ex) { logger.LogWarning(ex, "Import tool {Tool} failed: invalid argument", name); @@ -223,25 +232,40 @@ public class ImportAgentToolset( .Str("genre", "Genre or category.") .Str("notes", "Free-form notes — the blurb, if not already set.") .Build(), - async (input, ct) => (await projects.UpdateAsync(RequireProjectId(), new UpdateProjectRequest( - JsonInput.String(input, "title"), - JsonInput.String(input, "author"), - JsonInput.String(input, "genre"), - Notes: JsonInput.String(input, "notes")), ct) - ?? throw new NotFoundException(nameof(Project), RequireProjectId())).ToResponse()); + async (input, ct) => + { + var projectId = RequireProjectId(); + var updated = await projects.UpdateAsync(projectId, new UpdateProjectRequest( + JsonInput.String(input, "title"), + JsonInput.String(input, "author"), + JsonInput.String(input, "genre"), + Notes: JsonInput.String(input, "notes")), ct); + + return updated is null + ? new ImportToolNotFound($"Project '{projectId}' was not found.") + : updated.ToResponse(); + }); yield return new ImportAgentTool( "create_character", "Add a character dossier, parsed from a characters/*.md file.", CharacterSchema(nameRequired: true).Build(), - async (input, ct) => (await characters.CreateAsync(RequireProjectId(), new CreateCharacterRequest( - JsonInput.RequiredString(input, "name"), - Importance: JsonInput.Enum(input, "importance") ?? CharacterImportance.Supporting, - Occupation: JsonInput.String(input, "occupation"), - Appearance: JsonInput.String(input, "appearance"), - Backstory: JsonInput.String(input, "backstory"), - Want: JsonInput.String(input, "want"), - Notes: JsonInput.String(input, "notes")), ct)).ToResponse()); + async (input, ct) => + { + var projectId = RequireProjectId(); + var created = await characters.CreateAsync(projectId, new CreateCharacterRequest( + JsonInput.RequiredString(input, "name"), + Importance: JsonInput.Enum(input, "importance") ?? CharacterImportance.Supporting, + Occupation: JsonInput.String(input, "occupation"), + Appearance: JsonInput.String(input, "appearance"), + Backstory: JsonInput.String(input, "backstory"), + Want: JsonInput.String(input, "want"), + Notes: JsonInput.String(input, "notes")), ct); + + return created is null + ? new ImportToolNotFound($"Project '{projectId}' was not found.") + : created.ToResponse(); + }); yield return new ImportAgentTool( "update_character", @@ -252,15 +276,18 @@ public class ImportAgentToolset( async (input, ct) => { var characterId = JsonInput.RequiredGuid(input, "character_id"); - return (await characters.UpdateAsync(characterId, new UpdateCharacterRequest( + var updated = await characters.UpdateAsync(characterId, new UpdateCharacterRequest( JsonInput.String(input, "name"), Importance: JsonInput.Enum(input, "importance"), Occupation: JsonInput.String(input, "occupation"), Appearance: JsonInput.String(input, "appearance"), Backstory: JsonInput.String(input, "backstory"), Want: JsonInput.String(input, "want"), - Notes: JsonInput.String(input, "notes")), ct) - ?? throw new NotFoundException("Character", characterId)).ToResponse(); + Notes: JsonInput.String(input, "notes")), ct); + + return updated is null + ? new ImportToolNotFound($"Character '{characterId}' was not found.") + : updated.ToResponse(); }); yield return new ImportAgentTool( @@ -274,13 +301,21 @@ public class ImportAgentToolset( .Str("notes", "The chapter file's ## Notes section, if present.") .StringArray("tags", "The Part value and the raw Thread text, e.g. ['Part I', 'thread:Logen'].") .Build(), - async (input, ct) => (await chapters.CreateAsync(RequireProjectId(), new CreateChapterRequest( - JsonInput.RequiredString(input, "title"), - JsonInput.Int(input, "number"), - JsonInput.String(input, "summary"), - JsonInput.Guid(input, "pov_character_id"), - Notes: JsonInput.String(input, "notes"), - Tags: JsonInput.Strings(input, "tags")), ct)).ToResponse()); + async (input, ct) => + { + var projectId = RequireProjectId(); + var created = await chapters.CreateAsync(projectId, new CreateChapterRequest( + JsonInput.RequiredString(input, "title"), + JsonInput.Int(input, "number"), + JsonInput.String(input, "summary"), + JsonInput.Guid(input, "pov_character_id"), + Notes: JsonInput.String(input, "notes"), + Tags: JsonInput.Strings(input, "tags")), ct); + + return created is null + ? new ImportToolNotFound($"Project '{projectId}' was not found.") + : created.ToResponse(); + }); yield return new ImportAgentTool( "update_chapter", @@ -294,11 +329,14 @@ public class ImportAgentToolset( async (input, ct) => { var chapterId = JsonInput.RequiredGuid(input, "chapter_id"); - return (await chapters.UpdateAsync(chapterId, new UpdateChapterRequest( + var updated = await chapters.UpdateAsync(chapterId, new UpdateChapterRequest( Summary: JsonInput.String(input, "summary"), PovCharacterId: JsonInput.Guid(input, "pov_character_id"), - Notes: JsonInput.String(input, "notes")), ct) - ?? throw new NotFoundException("Chapter", chapterId)).ToResponse(); + Notes: JsonInput.String(input, "notes")), ct); + + return updated is null + ? new ImportToolNotFound($"Chapter '{chapterId}' was not found.") + : updated.ToResponse(); }); yield return new ImportAgentTool( @@ -311,13 +349,21 @@ public class ImportAgentToolset( .Str("what_happened", "The What column.") .Str("whats_next", "The Why column.") .Build(), - async (input, ct) => (await beats.CreateAsync( - JsonInput.RequiredGuid(input, "chapter_id"), - new CreateBeatRequest( - JsonInput.RequiredString(input, "title"), - CharacterId: JsonInput.Guid(input, "character_id"), - WhatHappened: JsonInput.String(input, "what_happened"), - WhatsNext: JsonInput.String(input, "whats_next")), ct)).ToResponse()); + async (input, ct) => + { + var chapterId = JsonInput.RequiredGuid(input, "chapter_id"); + var created = await beats.CreateAsync( + chapterId, + new CreateBeatRequest( + JsonInput.RequiredString(input, "title"), + CharacterId: JsonInput.Guid(input, "character_id"), + WhatHappened: JsonInput.String(input, "what_happened"), + WhatsNext: JsonInput.String(input, "whats_next")), ct); + + return created is null + ? new ImportToolNotFound($"Chapter '{chapterId}' was not found.") + : created.ToResponse(); + }); yield return new ImportAgentTool( "add_arc_stage", @@ -328,12 +374,20 @@ public class ImportAgentToolset( .Str("description", "The bullet's text.") .Str("chapter_id", "The chapter this stage is pinned to, if the (Ch. N) marker resolves to an imported chapter.") .Build(), - async (input, ct) => (await arcs.CreateAsync( - JsonInput.RequiredGuid(input, "character_id"), - new CreateArcStageRequest( - JsonInput.RequiredString(input, "title"), - Description: JsonInput.String(input, "description"), - ChapterId: JsonInput.Guid(input, "chapter_id")), ct)).ToResponse()); + async (input, ct) => + { + var characterId = JsonInput.RequiredGuid(input, "character_id"); + var created = await arcs.CreateAsync( + characterId, + new CreateArcStageRequest( + JsonInput.RequiredString(input, "title"), + Description: JsonInput.String(input, "description"), + ChapterId: JsonInput.Guid(input, "chapter_id")), ct); + + return created is null + ? new ImportToolNotFound($"Character '{characterId}' was not found.") + : created.ToResponse(); + }); } private static JsonSchemaBuilder CharacterSchema(bool nameRequired) => diff --git a/src/Novelly.Api/Program.cs b/src/Novelly.Api/Program.cs index 60f69dc..2843800 100644 --- a/src/Novelly.Api/Program.cs +++ b/src/Novelly.Api/Program.cs @@ -61,7 +61,6 @@ app.UseExceptionHandler(handler => handler.Run(async context => var (status, title) = exception switch { - NotFoundException => (StatusCodes.Status404NotFound, "Not found"), AgentNotConfiguredException => (StatusCodes.Status503ServiceUnavailable, "Agent unavailable"), ArgumentException or InvalidOperationException => (StatusCodes.Status400BadRequest, "Invalid request"), _ => (StatusCodes.Status500InternalServerError, "Unexpected error") diff --git a/src/Novelly.Api/Questions/OpenQuestionEndpoints.cs b/src/Novelly.Api/Questions/OpenQuestionEndpoints.cs index a3755b6..1cf57fd 100644 --- a/src/Novelly.Api/Questions/OpenQuestionEndpoints.cs +++ b/src/Novelly.Api/Questions/OpenQuestionEndpoints.cs @@ -24,7 +24,13 @@ public static class OpenQuestionEndpoints projectScoped.MapPost("/", async ( Guid projectId, CreateOpenQuestionRequest request, OpenQuestionService service, CancellationToken ct) => { - var created = (await service.CreateAsync(projectId, request, ct)).ToResponse(); + var question = await service.CreateAsync(projectId, request, ct); + if (question is null) + { + return Results.NotFound(); + } + + var created = question.ToResponse(); return Results.Created($"/api/questions/{created.Id}", created); }) .WithSummary("Raise an open question, optionally against a chapter outline and/or a character."); diff --git a/src/Novelly.Api/Questions/OpenQuestionService.cs b/src/Novelly.Api/Questions/OpenQuestionService.cs index 0b54980..168048a 100644 --- a/src/Novelly.Api/Questions/OpenQuestionService.cs +++ b/src/Novelly.Api/Questions/OpenQuestionService.cs @@ -4,7 +4,6 @@ using Novelly.Api.Characters; using Novelly.Api.Common; using Novelly.Api.Common.Validation; using Novelly.Api.Data; -using Novelly.Api.Projects; namespace Novelly.Api.Questions; @@ -73,7 +72,8 @@ public class OpenQuestionService( return await FindAsync(id, ct); } - public async Task CreateAsync( + /// Null when no project has this id — a lookup miss is expected, not exceptional. + public async Task CreateAsync( Guid projectId, CreateOpenQuestionRequest request, CancellationToken ct = default) { Guard.Default(projectId, nameof(projectId)); @@ -84,8 +84,8 @@ public class OpenQuestionService( if (!await db.Projects.AnyAsync(p => p.Id == projectId, ct)) { - logger.LogWarning("Rejected open question creation: project {ProjectId} not found", projectId); - throw new NotFoundException(nameof(Project), projectId); + logger.LogInformation("Rejected open question creation: project {ProjectId} not found", projectId); + return null; } await ValidateAssociationsAsync(projectId, request.ChapterId, request.CharacterId, ct); diff --git a/src/Novelly.Api/Scenes/SceneEndpoints.cs b/src/Novelly.Api/Scenes/SceneEndpoints.cs index 42619fb..ba8a940 100644 --- a/src/Novelly.Api/Scenes/SceneEndpoints.cs +++ b/src/Novelly.Api/Scenes/SceneEndpoints.cs @@ -18,7 +18,13 @@ public static class SceneEndpoints chapterScoped.MapPost("/", async ( Guid chapterId, CreateSceneRequest request, SceneService service, CancellationToken ct) => { - var created = (await service.CreateAsync(chapterId, request, ct)).ToResponse(); + var scene = await service.CreateAsync(chapterId, request, ct); + if (scene is null) + { + return Results.NotFound(); + } + + var created = scene.ToResponse(); return Results.Created($"/api/scenes/{created.Id}", created); }) .WithSummary("Add a scene to a chapter."); diff --git a/src/Novelly.Api/Scenes/SceneService.cs b/src/Novelly.Api/Scenes/SceneService.cs index 9a0f1cf..09016d2 100644 --- a/src/Novelly.Api/Scenes/SceneService.cs +++ b/src/Novelly.Api/Scenes/SceneService.cs @@ -1,5 +1,4 @@ using Microsoft.EntityFrameworkCore; -using Novelly.Api.Chapters; using Novelly.Api.Common; using Novelly.Api.Common.Validation; using Novelly.Api.Data; @@ -33,7 +32,8 @@ public class SceneService( return await FindAsync(id, ct); } - public async Task CreateAsync(Guid chapterId, CreateSceneRequest request, CancellationToken ct = default) + /// Null when no chapter has this id — a lookup miss is expected, not exceptional. + public async Task CreateAsync(Guid chapterId, CreateSceneRequest request, CancellationToken ct = default) { Guard.Default(chapterId, nameof(chapterId)); Guard.Null(request, nameof(request)); @@ -43,8 +43,8 @@ public class SceneService( if (!await db.Chapters.AnyAsync(c => c.Id == chapterId, ct)) { - logger.LogWarning("Rejected scene creation: chapter {ChapterId} not found", chapterId); - throw new NotFoundException(nameof(Chapter), chapterId); + logger.LogInformation("Rejected scene creation: chapter {ChapterId} not found", chapterId); + return null; } var scene = new Scene diff --git a/src/Novelly.Api/Tags/TagEndpoints.cs b/src/Novelly.Api/Tags/TagEndpoints.cs index d703515..ca6c07d 100644 --- a/src/Novelly.Api/Tags/TagEndpoints.cs +++ b/src/Novelly.Api/Tags/TagEndpoints.cs @@ -18,7 +18,13 @@ public static class TagEndpoints projectScoped.MapPost("/", async ( Guid projectId, CreateTagRequest request, TagService service, CancellationToken ct) => { - var created = (await service.CreateAsync(projectId, request, ct)).ToResponse(); + var tag = await service.CreateAsync(projectId, request, ct); + if (tag is null) + { + return Results.NotFound(); + } + + var created = tag.ToResponse(); return Results.Created($"/api/tags/{created.Id}", created); }) .WithSummary("Create a tag. Tags are also created on demand when applied by name."); diff --git a/src/Novelly.Api/Tags/TagService.cs b/src/Novelly.Api/Tags/TagService.cs index ce32076..6713ea2 100644 --- a/src/Novelly.Api/Tags/TagService.cs +++ b/src/Novelly.Api/Tags/TagService.cs @@ -2,7 +2,6 @@ using Microsoft.EntityFrameworkCore; using Novelly.Api.Common; using Novelly.Api.Common.Validation; using Novelly.Api.Data; -using Novelly.Api.Projects; namespace Novelly.Api.Tags; @@ -47,7 +46,8 @@ public class TagService( return tag; } - public async Task CreateAsync(Guid projectId, CreateTagRequest request, CancellationToken ct = default) + /// Null when no project has this id — a lookup miss is expected, not exceptional. + public async Task CreateAsync(Guid projectId, CreateTagRequest request, CancellationToken ct = default) { Guard.Default(projectId, nameof(projectId)); Guard.Null(request, nameof(request)); @@ -57,8 +57,8 @@ public class TagService( if (!await db.Projects.AnyAsync(p => p.Id == projectId, ct)) { - logger.LogWarning("Rejected tag creation: project {ProjectId} not found", projectId); - throw new NotFoundException(nameof(Project), projectId); + logger.LogInformation("Rejected tag creation: project {ProjectId} not found", projectId); + return null; } var name = TagMapping.Normalise(request.Name); diff --git a/tests/Novelly.Api.Tests/BeatServiceTests.cs b/tests/Novelly.Api.Tests/BeatServiceTests.cs index 134604f..6ee847a 100644 --- a/tests/Novelly.Api.Tests/BeatServiceTests.cs +++ b/tests/Novelly.Api.Tests/BeatServiceTests.cs @@ -67,13 +67,11 @@ public class BeatServiceTests : ServiceTestFixture } [Test] - public void Reordering_with_an_unknown_beat_is_refused() + public async Task Reordering_with_an_unknown_beat_returns_null_rather_than_throwing() { - Beats.CreateAsync(_chapterId, new CreateBeatRequest("First")).Wait(); + await Beats.CreateAsync(_chapterId, new CreateBeatRequest("First")); - Assert.That( - async () => await Beats.ReorderAsync(_chapterId, new ReorderBeatsRequest([Guid.NewGuid()])), - Throws.TypeOf()); + Assert.That(await Beats.ReorderAsync(_chapterId, new ReorderBeatsRequest([Guid.NewGuid()])), Is.Null); } [Test] diff --git a/tests/Novelly.Api.Tests/CharacterArcTests.cs b/tests/Novelly.Api.Tests/CharacterArcTests.cs index 8f9b816..6a46dde 100644 --- a/tests/Novelly.Api.Tests/CharacterArcTests.cs +++ b/tests/Novelly.Api.Tests/CharacterArcTests.cs @@ -183,11 +183,10 @@ public class CharacterArcTests : ServiceTestFixture } [Test] - public void Reordering_with_an_unknown_stage_is_refused() => + public async Task Reordering_with_an_unknown_stage_returns_null_rather_than_throwing() => Assert.That( - async () => await Arcs.ReorderAsync( - _characterId, new ReorderArcStagesRequest([Guid.NewGuid()])), - Throws.TypeOf()); + await Arcs.ReorderAsync(_characterId, new ReorderArcStagesRequest([Guid.NewGuid()])), + Is.Null); [Test] public async Task The_character_page_sees_every_beat_they_appear_in_across_the_book() diff --git a/tests/Novelly.Api.Tests/ExceptionHandlingTests.cs b/tests/Novelly.Api.Tests/ExceptionHandlingTests.cs index 9d58df5..e926471 100644 --- a/tests/Novelly.Api.Tests/ExceptionHandlingTests.cs +++ b/tests/Novelly.Api.Tests/ExceptionHandlingTests.cs @@ -46,12 +46,6 @@ public class ExceptionHandlingTests : ServiceTestFixture Throws.TypeOf()); [Test] - public async Task An_embedded_reference_to_a_missing_parent_still_throws() - { - // Creating a chapter under a nonexistent project isn't a "look this up" miss — it's - // an invalid precondition for the create, so it stays exceptional. - Assert.That( - async () => await Chapters.CreateAsync(Guid.NewGuid(), new CreateChapterRequest("Landfall")), - Throws.TypeOf()); - } + public async Task Creating_a_chapter_under_a_missing_project_returns_null_rather_than_throwing() => + Assert.That(await Chapters.CreateAsync(Guid.NewGuid(), new CreateChapterRequest("Landfall")), Is.Null); }