[wojciech-dron]

No description provided.

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Error Handling The retry wrapper swallows all exceptions and returns a new default response instance, which can hide failures and make downstream logic treat failed calls as successful (especially if callers don’t validate the returned object). Consider returning an ErrorOr/Result type, rethrowing after logging, or at least allowing callers to opt-in to “return default on failure”. public static async Task<TResponse> InvokeWithRetryAsync<TResponse>(this IMessageBus messageBus, object request, ILogger? logger = null, int retryAttempts = 1, CancellationToken cancellationToken = default) where TResponse : new() { var retryPolicy = Policy.Handle<Exception>() .WaitAndRetryAsync(retryAttempts, retryAttempt => TimeSpan.FromSeconds(Math.Pow(2, retryAttempt))); try { return await retryPolicy.ExecuteAsync(async () => await messageBus.InvokeAsync<TResponse>(request, cancellationToken)); } catch (Exception e) { if (logger is not null) logger.LogError(e, "Error occurred while sending request: {Request}", request); else Console.WriteLine($"Error occurred while sending request: {request}"); return new TResponse(); } Timing Bug Elapsed time is derived from TimeSpan.Milliseconds, which is only the milliseconds component (0-999) and not total duration. This will under-report requests taking longer than a second; use TotalMilliseconds (and an appropriate numeric type) to log accurate timings. public void After(ILogger<WolverineLoggingMiddleware> logger, Envelope envelope) { var elapsedMs = Stopwatch.GetElapsedTime(beginTime).Milliseconds; var requestType = envelope.MessageType; LogRequest(logger, envelope.Id, requestType, elapsedMs); } Runtime Failure The handler no longer catches exceptions around agent chat invocation, so transient provider/network failures can now abort the handler entirely rather than producing a controlled retry/error result. Also, keyed DI lookup uses an upper-cased provider name; ensure service registrations use the same key normalization to avoid missing-service failures at runtime. public async ValueTask<ErrorOr<Response>> Handle(Request request, CancellationToken cancellationToken) { var message = new ChatMessageContent(AuthorRole.User, $"offerContent: {request.OfferContent}"); string? finalContent = null; var retryCount = 0; var chatHistory = new List<ChatItem>(); do { var chat = PrepareAgentsChat(request); chat.AddChatMessage(message); var asyncResponse = chat.InvokeAsync(cancellationToken); await foreach (var response in asyncResponse) { chatHistory.Add(new ChatItem(response.AuthorName, response.Content)); finalContent = response.Content; }

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Fix elapsed time measurement Stopwatch.GetElapsedTime(...).Milliseconds returns only the millisecond component (0-999), not the total duration, which will under-report timings. Use TotalMilliseconds and cast/round to keep accurate performance logs. JobScraper.Web/Modules/Wolverine/WolverineLoggingMiddleware.cs [23-25] -var elapsedMs = Stopwatch.GetElapsedTime(beginTime).Milliseconds; +var elapsedMs = (int)Stopwatch.GetElapsedTime(beginTime).TotalMilliseconds; var requestType = envelope.MessageType; LogRequest(logger, envelope.Id, requestType, elapsedMs); Suggestion importance[1-10]: 7 __ Why: Using Stopwatch.GetElapsedTime(...).Milliseconds only logs the milliseconds component (0-999) rather than total elapsed time, so timings will be incorrect. Switching to TotalMilliseconds makes the performance logging accurate with low risk.	Medium
	Don't swallow invocation failures Returning new TResponse() on failure silently hides errors and can make downstream logic behave as if a request succeeded. Log and rethrow so callers can handle the failure properly, and make retries honor the provided cancellationToken. JobScraper.Web/Modules/Extensions/WolverineExtensions.cs [8-31] public static async Task<TResponse> InvokeWithRetryAsync<TResponse>(this IMessageBus messageBus, object request, ILogger? logger = null, int retryAttempts = 1, CancellationToken cancellationToken = default) - where TResponse : new() { var retryPolicy = Policy.Handle<Exception>() .WaitAndRetryAsync(retryAttempts, retryAttempt => TimeSpan.FromSeconds(Math.Pow(2, retryAttempt))); + try { - return await retryPolicy.ExecuteAsync(async () => - await messageBus.InvokeAsync<TResponse>(request, cancellationToken)); + return await retryPolicy.ExecuteAsync( + ct => messageBus.InvokeAsync<TResponse>(request, ct), + cancellationToken); } catch (Exception e) { if (logger is not null) logger.LogError(e, "Error occurred while sending request: {Request}", request); else Console.WriteLine($"Error occurred while sending request: {request}"); - return new TResponse(); + throw; } } Suggestion importance[1-10]: 6 __ Why: Returning new TResponse() on exceptions can mask failures and lead callers to treat an errored invocation as a success; rethrowing makes failures explicit and easier to handle correctly. Updating Polly execution to honor the provided cancellationToken is also a real improvement.	Low
	Guard provider key normalization request.ProviderName can be null/empty (e.g., user input), which would throw when calling ToUpperInvariant(). Normalize safely by falling back to a default provider name before transforming the key. JobScraper.Web/Features/AiSummary/SummarizeOfferContent.cs [74] -var kernel = serviceProvider.GetRequiredKeyedService<Kernel>(request.ProviderName.ToUpperInvariant()); +var providerKey = string.IsNullOrWhiteSpace(request.ProviderName) + ? AiProvidersConfig.MainProvider + : request.ProviderName; +var kernel = serviceProvider.GetRequiredKeyedService<Kernel>(providerKey.ToUpperInvariant()); + Suggestion importance[1-10]: 3 __ Why: While request.ProviderName is declared as non-nullable and has a default, it can still be null at runtime in some edge cases, and normalizing/falling back could prevent a crash. However, this is mostly defensive and may hide invalid configuration/input rather than surfacing it.	Low