Safe and clean architecture: add modular agent bounded contexts

[halcwb]

This pull request refactors several service modules and updates how ports are composed and used in the application environment. The main focus is to improve separation of concerns by moving business logic into dedicated service modules (with Service suffix) and updating the dependency injection pattern for ports, especially for order and nutrition plans.

Key changes include:

Service module refactoring:

• Renamed modules like Formulary, Parenteralia, Order, OrderPlan, and NutritionPlan to FormularyService, ParenteraliaService, OrderService, OrderPlanService, and NutritionPlanService respectively, and updated all usages accordingly. This centralizes business logic and clarifies responsibilities. [1] [2] [3] [4] [5]

Dependency injection and port composition:

• Refactored the construction of OrderPlanPort and NutritionPlanPort to accept an OrderContextPort instead of passing logger and provider directly, enabling better testability and decoupling from infrastructure. Updated the application environment (makeAppEnv) to create and pass the shared OrderContextPort instance.

Order and Nutrition plan logic updates:

• Updated functions in OrderPlanService and NutritionPlanService to use the injected OrderContextPort for evaluating contexts asynchronously, improving consistency and enabling better error handling. [1] [2] [3]
• Changed calculation functions to use the new OrderService module for order total calculations, ensuring all business logic is routed through the appropriate service abstraction. [1] [2] [3]

Adapters update:

• Updated the Adapters module to use the new service modules and updated function signatures to match the new dependency injection pattern. [1] [2]

These changes collectively improve maintainability, testability, and clarity of the codebase by enforcing a clear separation between business logic and infrastructure concerns.

[greptile-apps]

Greptile Summary

This PR completes the bounded-context refactor by splitting the single monolithic ServerApi.fs and its single MailboxProcessor agent into per-component service modules and dedicated agents (FormularyAgent, OrderCtxAgent, OrderPlanAgent, NutritionAgent). Service modules are renamed with a Service suffix, and order/nutrition plan operations are converted from synchronous direct calls to async port-injected calls via OrderContextPort, enabling proper decoupling and independent serialization per domain.

Key changes:

• ServerApi.fs (1404 lines) deleted; logic decomposed into ServerApi.Services.fs, ServerApi.Adapters.fs, and ServerApi.AgentAdapters.fs
• Modules renamed: Formulary → FormularyService, Parenteralia → ParenteraliaService, Order → OrderService, OrderPlan → OrderPlanService, NutritionPlan → NutritionPlanService
• OrderPlanService.updateOrderPlan and all NutritionPlanService context operations converted to Async using injected OrderContextPort
• Both Adapters.makeAppEnv and AgentAdapters.makeAppEnv create a shared orderCtxPort instance passed to downstream plan ports
• Notable: processOrderPlanCommand and processNutritionCommand in AgentAdapters use Async.RunSynchronously inside Agent.createReply callbacks to bridge the sync callback requirement with the new async service functions — this works correctly but blocks a thread pool thread; see the inline comment for a suggested improvement

Confidence Score: 4/5

• Safe to merge; the refactor is architecturally sound and all async conversions are handled correctly in both adapter paths.
• The per-component agent split and module renames are consistent and correct. Async conversions in OrderPlanService and NutritionPlanService use the port correctly. The one non-blocking concern is Async.RunSynchronously inside the Agent.createReply synchronous callback in AgentAdapters — it works correctly today (no deadlock, bounded thread usage) but is a code-quality issue worth addressing in a follow-up.
• src/Informedica.GenPRES.Server/ServerApi.AgentAdapters.fs — processOrderPlanCommand and processNutritionCommand use Async.RunSynchronously inside a MailboxProcessor callback.

Important Files Changed

Filename	Overview
src/Informedica.GenPRES.Server/ServerApi.AgentAdapters.fs	Refactored from a single monolithic agent (one DU for all commands) to four per-component agents (Formulary, OrderContext, OrderPlan, NutritionPlan), enabling independent serialization per domain. Main concern: Async.RunSynchronously inside the synchronous Agent.createReply callback for OrderPlan and Nutrition agents blocks a thread pool thread during cross-agent calls.
src/Informedica.GenPRES.Server/ServerApi.Adapters.fs	Updated makeOrderPlanPort and makeNutritionPlanPort to accept OrderContextPort instead of raw logger/provider. Uses proper let! async binding for updateOrderPlan, and correctly returns async values directly for nutrition plan operations that now return Async<...>. Changes are correct and clean.
src/Informedica.GenPRES.Server/ServerApi.Services.fs	Module renames (Formulary → FormularyService, Parenteralia → ParenteraliaService, Order → OrderService, OrderPlan → OrderPlanService, NutritionPlan → NutritionPlanService) and async conversion of plan operations to accept OrderContextPort. All callers of the old sync OrderContextService.evaluate are correctly updated to use async port injection. Logic unchanged.
src/Informedica.GenPRES.Server/ServerApi.fs	File deleted. Its 1404-line monolithic implementation (Mappers, full server handler logic) has been decomposed into the new modular Services, Adapters, and AgentAdapters files introduced in prior PRs.

Sequence Diagram

sequenceDiagram
    participant Client
    participant FormularyAgent
    participant OrderCtxAgent
    participant OrderPlanAgent
    participant NutritionAgent

    Note over FormularyAgent,NutritionAgent: Per-component MailboxProcessors

    Client->>+OrderPlanAgent: postAsync UpdateOrderPlan
    Note over OrderPlanAgent: Async.RunSynchronously
    OrderPlanAgent->>+OrderCtxAgent: postAsync Evaluate(cmd, ctx)
    OrderCtxAgent->>OrderCtxAgent: OrderContextService.evaluate
    OrderCtxAgent-->>-OrderPlanAgent: OrderContext result
    OrderPlanAgent->>OrderPlanAgent: calculateTotals
    OrderPlanAgent-->>-Client: OrderPlan result

    Client->>+NutritionAgent: postAsync AddNutritionContext
    Note over NutritionAgent: Async.RunSynchronously
    NutritionAgent->>+OrderCtxAgent: postAsync Evaluate(UpdateOrderContext, ctx)
    OrderCtxAgent-->>-NutritionAgent: OrderContext result
    NutritionAgent-->>-Client: NutritionPlan result

Loading

_{Reviews (1): Last reviewed commit: "refactor(server): switch to bounded doma..." | Re-trigger Greptile}

[halcwb]

@greptile if everything is now correct approve.

[Copilot]

Pull request overview

This PR refactors the GenPRES server architecture toward clearer bounded contexts by moving domain logic into *Service modules and composing ports around a shared OrderContextPort, with agent-backed adapters providing per-component serialization.

Changes:

• Renamed/refactored server “service” modules (e.g., OrderPlan → OrderPlanService, NutritionPlan → NutritionPlanService) and updated call sites.
• Reworked port composition so OrderPlan/NutritionPlan depend on an injected OrderContextPort (improves decoupling/testability).
• Split the single server agent into per-component agents and added Agent.createReplyAsync to support async request/reply processors.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
src/Informedica.GenPRES.Server/ServerApi.fs	Removed legacy monolithic ServerApi implementation (moved into more modular structure).
src/Informedica.GenPRES.Server/ServerApi.Services.fs	Renamed modules to *Service and refactored OrderPlan/NutritionPlan logic to use OrderContextPort async evaluation.
src/Informedica.GenPRES.Server/ServerApi.AgentAdapters.fs	Introduced per-bounded-context MailboxProcessor agents and updated AppEnv composition to use shared OrderContextPort.
src/Informedica.GenPRES.Server/ServerApi.Adapters.fs	Updated non-agent adapters to the new service module names and OrderContextPort-based composition.
src/Informedica.Agents.Lib/Agent.fs	Added createReplyAsync for async request/reply agent processors.

[Copilot]

This setComponent helper also uses Async.RunSynchronously even though it’s already inside an async { ... } block. This blocks a thread-pool thread unnecessarily; prefer match logAgent with | Some a -> do! ... | None -> () and keep the whole function non-blocking.

Suggested change

	match logAgent with
	| Some a -> a |> Logging.setComponentName (Some name) |> Async.RunSynchronously
	| None -> ()
	match cmd with
	| OrderPlanCommand.Update (tp, cmdOpt) ->
	setComponent "TreatmentPlan"
	async {
	match logAgent with
	| Some a -> do! a |> Logging.setComponentName (Some name)
	| None -> return ()
	}
	match cmd with
	| OrderPlanCommand.Update (tp, cmdOpt) ->
	do! setComponent "TreatmentPlan"

[Copilot]

This error log interpolates the exception value as {ex} without an explicit format specifier, which the repository’s F# string interpolation guideline discourages. Consider formatting explicitly (e.g., message vs full exception) so logs are consistent and predictable.

Suggested change

	writeErrorMessage $"[FormularyAgent] error in {cmd |> formularyCommandToString}: {ex}"
	writeErrorMessage $"[FormularyAgent] error in %s{cmd |> formularyCommandToString}: %s{ex.Message}"

[Copilot]

Same string interpolation issue here: {ex} is formatted without an explicit specifier. Please use a specific format (%s for ex.Message or %O/%A for the full exception) to match the project guideline and keep logs consistent.

Suggested change

	writeErrorMessage $"[OrderCtxAgent] error in {cmd |> orderCtxCommandToString}: {ex}"
	writeErrorMessage $"[OrderCtxAgent] error in {cmd |> orderCtxCommandToString}: %O{ex}"

[Copilot]

Same interpolation issue: {ex} is used without an explicit format specifier in this log message. Please use an explicit format so exception logging is consistent and aligned with the repository’s interpolation guideline.

Suggested change

	writeErrorMessage $"[NutritionAgent] error in {cmd |> nutritionCommandToString}: {ex}"
	writeErrorMessage $"[NutritionAgent] error in %s{cmd |> nutritionCommandToString}: %O{ex}"

[Copilot]

This module-level comment claims “thread-safe access to the provider within each component”, but provider is shared across multiple agents and can now be accessed concurrently across components. Unless Resources.IResourceProvider is guaranteed thread-safe, this comment is misleading—either clarify the thread-safety requirement (e.g., must be a locked/cached provider) or reintroduce a shared serialization/lock around provider calls.

Suggested change

	/// - Thread-safe access to the provider within each component
	/// - An async boundary for all calls into the component
	///
	/// Note: The same IResourceProvider instance is shared across all component agents,
	/// so calls into `provider` may occur concurrently across components. The supplied
	/// `provider` implementation must therefore be safe for concurrent use (for example,
	/// internally synchronized or otherwise thread-safe).
	///

[Copilot]

createReplyAsync introduces new agent behavior (request/reply where the processor is async), but there are existing comprehensive Agent tests and none appear to cover this new constructor. Please add tests validating that async processors are awaited, replies are returned, and exceptions behave as expected.

[Copilot]

Same issue here: the error message interpolates {other} without a format specifier. Please use an explicit interpolation format so the union value is rendered predictably (and to match the project’s interpolation guideline).

Suggested change

	| other -> Error [| $"Unexpected response: {other}" |]
	let private extractParenteralia = function
	| FormularyResponse.Parenteralia r -> r
	| other -> Error [| $"Unexpected response: {other}" |]
	| other -> Error [| $"Unexpected response: %A{other}" |]
	let private extractParenteralia = function
	| FormularyResponse.Parenteralia r -> r
	| other -> Error [| $"Unexpected response: %A{other}" |]

[Copilot]

Same interpolation issue: the exception is interpolated as {ex} without an explicit format specifier. Please format explicitly (and consider whether you want ex.Message or the full exception).

Suggested change

	writeErrorMessage $"[OrderPlanAgent] error in {cmd |> orderPlanCommandToString}: {ex}"
	writeErrorMessage $"[OrderPlanAgent] error in {cmd |> orderPlanCommandToString}: %A{ex}"

[Copilot]

In these extraction helpers, the interpolated string uses {other} without an explicit format specifier. This can accidentally call ToString() and lose useful structure; the codebase’s F# guidelines require specifying a format (e.g., %A/%O/%s) in interpolated strings.

Suggested change

	| other -> Error [| $"Unexpected response: {other}" |]
	let private extractParenteralia = function
	| FormularyResponse.Parenteralia r -> r
	| other -> Error [| $"Unexpected response: {other}" |]
	| other -> Error [| $"Unexpected response: %A{other}" |]
	let private extractParenteralia = function
	| FormularyResponse.Parenteralia r -> r
	| other -> Error [| $"Unexpected response: %A{other}" |]

[Copilot]

Async.RunSynchronously inside agent command processing can block the agent thread and can deadlock if the logging agent ever depends on the calling agent. Since this is already inside an async workflow elsewhere in this file, prefer making this path async (e.g., using createReplyAsync and do! Logging.setComponentName ...) instead of blocking.