Open AI integrated #135
Open AI integrated #135
Conversation
WalkthroughThis update introduces a custom OpenAI service with JWT authentication, new models for chat-based AI analysis, and new API endpoints for company AI analysis. Configuration files and environment variables are updated to support OpenAI integration. The dependency injection setup is modified to register both standard and custom OpenAI service implementations. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant CompaniesController
participant GetCompanyAiAnalysisHandler
participant DatabaseContext
participant OpenAiService
Client->>CompaniesController: GET /companies/{id}/open-ai-analysis
CompaniesController->>GetCompanyAiAnalysisHandler: Handle(companyId, cancellationToken)
GetCompanyAiAnalysisHandler->>DatabaseContext: Fetch company (with reviews, rating history)
DatabaseContext-->>GetCompanyAiAnalysisHandler: Company entity
GetCompanyAiAnalysisHandler->>OpenAiService: AnalyzeCompanyAsync(company, correlationId, cancellationToken)
OpenAiService->>OpenAI API: POST /chat/completions (with company data)
OpenAI API-->>OpenAiService: ChatResponse
OpenAiService-->>GetCompanyAiAnalysisHandler: OpenAiChatResult
GetCompanyAiAnalysisHandler-->>CompaniesController: OpenAiChatResult
CompaniesController-->>Client: 200 OK + AI analysis result
Possibly related PRs
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (4)
src/Infrastructure/Services/OpenAi/Custom/Models/OpenAiBodyReportRole.cs (1)
4-4: Namespace update ok – consider unit coverage for historical data loopWhile only the namespace changed, this record’s constructor has non-trivial branching (lines 19-40). Once the migration dust settles, add tests to verify:
- correct cut-off when
salariesForDate.Count == 0- accurate
averageSalaryToCompareselectionHappy to scaffold the tests if needed.
src/Web.Api/Setup/ServiceRegistration.cs (1)
17-17: LGTM! Consider adding documentation for dual OpenAI service registration.The parallel registration of both
IOpenAiServiceandICustomOpenAiServicesuggests a migration pattern. Consider adding inline comments to clarify the purpose of each service and when to use which implementation.Also applies to: 49-49, 61-61
src/Infrastructure/Services/OpenAi/OpenAiService.cs (2)
38-42: Consider externalizing the system prompt for better maintainability.The system prompt is hardcoded within the method. Consider moving it to configuration or a constants file to make it easier to update without modifying code.
127-132: Consider adding timeout and retry logic for HTTP requests.The HTTP client doesn't have timeout configuration, and there's no retry logic for transient failures. This could lead to hanging requests or unnecessary failures.
Would you like me to provide an implementation with Polly for retry logic and proper timeout configuration?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (28)
.github/workflows/deploy.yml(1 hunks)src/.env.example(1 hunks)src/Domain/Entities/Companies/Company.cs(1 hunks)src/Infrastructure/Services/OpenAi/Custom/CustomOpenAiService.cs(1 hunks)src/Infrastructure/Services/OpenAi/Custom/ICustomOpenAiService.cs(1 hunks)src/Infrastructure/Services/OpenAi/Custom/Models/OpenAiBodyReport.cs(1 hunks)src/Infrastructure/Services/OpenAi/Custom/Models/OpenAiBodyReportMetadata.cs(1 hunks)src/Infrastructure/Services/OpenAi/Custom/Models/OpenAiBodyReportRole.cs(1 hunks)src/Infrastructure/Services/OpenAi/Custom/Models/OpenAiBodyReportRoleHistoricalDataItem.cs(1 hunks)src/Infrastructure/Services/OpenAi/Custom/Models/OpenAiBodyReportRoleSalaryData.cs(1 hunks)src/Infrastructure/Services/OpenAi/IOpenAiService.cs(1 hunks)src/Infrastructure/Services/OpenAi/Models/ChatMessage.cs(1 hunks)src/Infrastructure/Services/OpenAi/Models/ChatRequest.cs(1 hunks)src/Infrastructure/Services/OpenAi/Models/ChatResponse.cs(1 hunks)src/Infrastructure/Services/OpenAi/Models/Choice.cs(1 hunks)src/Infrastructure/Services/OpenAi/Models/CompanyAnalyzeRequest.cs(1 hunks)src/Infrastructure/Services/OpenAi/Models/OpenAiChatResult.cs(1 hunks)src/Infrastructure/Services/OpenAi/OpenAiService.cs(2 hunks)src/Web.Api/Features/BackgroundJobs/AiAnalysisSubscriptionJob.cs(2 hunks)src/Web.Api/Features/Companies/CompaniesController.cs(2 hunks)src/Web.Api/Features/Companies/GetCompanyAiAnalysis/GetCompanyAiAnalysisHandler.cs(1 hunks)src/Web.Api/Features/Subscribtions/GetOpenAiReport/GetOpenAiReportHandler.cs(1 hunks)src/Web.Api/Features/Subscribtions/GetOpenAiReportAnalysis/GetOpenAiReportAnalysisHandler.cs(2 hunks)src/Web.Api/Features/Subscribtions/GetOpenAiReportAnalysis/GetOpenAiReportAnalysisResponse.cs(1 hunks)src/Web.Api/Features/Subscribtions/TelegramSubscriptionsController.cs(1 hunks)src/Web.Api/Setup/ServiceRegistration.cs(3 hunks)src/Web.Api/appsettings.Production.json(1 hunks)src/Web.Api/appsettings.json(2 hunks)
🧰 Additional context used
🧠 Learnings (4)
src/Web.Api/Features/Subscribtions/GetOpenAiReport/GetOpenAiReportHandler.cs (1)
Learnt from: maximgorbatyuk
PR: Techinterview-space/web-api#107
File: src/Domain/Entities/Users/UserEmail.cs:21-30
Timestamp: 2025-05-31T18:01:06.749Z
Learning: In the Domain.Entities.Users namespace, the team prefers not to set navigation properties in entity constructors and considers the current validation approach sufficient without additional data annotation attributes.
src/Infrastructure/Services/OpenAi/Models/Choice.cs (1)
Learnt from: maximgorbatyuk
PR: Techinterview-space/web-api#122
File: src/Domain/Entities/Github/GithubProfileBotChat.cs:28-28
Timestamp: 2025-06-19T09:01:04.940Z
Learning: In the GithubProfileBotChat entity (src/Domain/Entities/Github/GithubProfileBotChat.cs), the MessagesCount is intentionally initialized to 1 in the constructor because the entity creation itself implies that at least one message has been sent to trigger the chat creation.
src/Infrastructure/Services/OpenAi/Models/ChatMessage.cs (1)
Learnt from: maximgorbatyuk
PR: Techinterview-space/web-api#122
File: src/Domain/Entities/Github/GithubProfileBotChat.cs:28-28
Timestamp: 2025-06-19T09:01:04.940Z
Learning: In the GithubProfileBotChat entity (src/Domain/Entities/Github/GithubProfileBotChat.cs), the MessagesCount is intentionally initialized to 1 in the constructor because the entity creation itself implies that at least one message has been sent to trigger the chat creation.
src/Infrastructure/Services/OpenAi/Models/ChatRequest.cs (1)
Learnt from: maximgorbatyuk
PR: Techinterview-space/web-api#122
File: src/Domain/Entities/Github/GithubProfileBotChat.cs:28-28
Timestamp: 2025-06-19T09:01:04.940Z
Learning: In the GithubProfileBotChat entity (src/Domain/Entities/Github/GithubProfileBotChat.cs), the MessagesCount is intentionally initialized to 1 in the constructor because the entity creation itself implies that at least one message has been sent to trigger the chat creation.
🧬 Code Graph Analysis (7)
src/Domain/Entities/Companies/Company.cs (1)
src/Domain/Entities/Companies/CompanyReview.cs (1)
IsRelevant(122-126)
src/Infrastructure/Services/OpenAi/Models/Choice.cs (1)
src/Infrastructure/Services/OpenAi/Models/ChatMessage.cs (2)
ChatMessage(7-13)ChatMessage(15-17)
src/Infrastructure/Services/OpenAi/Custom/ICustomOpenAiService.cs (2)
src/Infrastructure/Services/OpenAi/Custom/CustomOpenAiService.cs (1)
GetBearer(27-31)src/Infrastructure/Services/OpenAi/Custom/Models/OpenAiBodyReport.cs (1)
OpenAiBodyReport(10-32)
src/Infrastructure/Services/OpenAi/Models/ChatRequest.cs (1)
src/Infrastructure/Services/OpenAi/Models/ChatMessage.cs (2)
ChatMessage(7-13)ChatMessage(15-17)
src/Web.Api/Features/Companies/GetCompanyAiAnalysis/GetCompanyAiAnalysisHandler.cs (3)
src/Infrastructure/Services/OpenAi/Models/OpenAiChatResult.cs (3)
OpenAiChatResult(5-8)OpenAiChatResult(10-13)OpenAiChatResult(15-21)src/Infrastructure/Services/OpenAi/IOpenAiService.cs (2)
Task(8-11)Task(13-16)src/Infrastructure/Services/OpenAi/OpenAiService.cs (3)
Task(27-52)Task(54-64)Task(66-175)
src/Infrastructure/Services/OpenAi/IOpenAiService.cs (2)
src/Infrastructure/Services/OpenAi/Models/OpenAiChatResult.cs (3)
OpenAiChatResult(5-8)OpenAiChatResult(10-13)OpenAiChatResult(15-21)src/Domain/Entities/Companies/Company.cs (3)
Company(9-259)Company(38-58)Company(256-258)
src/Infrastructure/Services/OpenAi/Models/CompanyAnalyzeRequest.cs (1)
src/Domain/Entities/Companies/Company.cs (5)
Company(9-259)Company(38-58)Company(256-258)List(60-70)CompanyReview(144-152)
🔇 Additional comments (16)
src/.env.example (1)
7-7: LGTM! Environment variable follows existing conventions.The addition of the OpenAI API key environment variable is consistent with existing patterns and properly formatted for .NET configuration binding.
src/Web.Api/Features/Subscribtions/GetOpenAiReportAnalysis/GetOpenAiReportAnalysisResponse.cs (1)
1-1: Verified: OpenAiBodyReport exists under Infrastructure.Services.OpenAi.Custom.ModelsThe
OpenAiBodyReportrecord is defined in
- src/Infrastructure/Services/OpenAi/Custom/Models/OpenAiBodyReport.cs
Namespace change is correct and can be approved.
src/Infrastructure/Services/OpenAi/Custom/Models/OpenAiBodyReport.cs (1)
4-4: Namespace realignment LGTMThe record now sits under the
Customnamespace; no functional impact. ✅
Remember to update any JSON (de)serialisation tests that rely on fully-qualified type names.src/Infrastructure/Services/OpenAi/Custom/Models/OpenAiBodyReportMetadata.cs (1)
3-3: Consistent namespace migration confirmedNothing else changed; good job keeping the DTO immutable.
src/Infrastructure/Services/OpenAi/Custom/Models/OpenAiBodyReportRoleSalaryData.cs (1)
4-4: LGTM - Clean namespace reorganization.The namespace change aligns well with the introduction of the custom OpenAI service architecture, providing clear separation between standard and custom OpenAI models.
src/Infrastructure/Services/OpenAi/Custom/Models/OpenAiBodyReportRoleHistoricalDataItem.cs (1)
4-4: LGTM - Consistent namespace restructuring.The namespace change maintains consistency with the custom OpenAI service model organization.
.github/workflows/deploy.yml (1)
33-33: LGTM - Secure secret injection follows established pattern.The OpenAI API key substitution follows the same secure pattern as other secrets in the deployment workflow.
src/Domain/Entities/Companies/Company.cs (1)
72-80: LGTM - Well-designed method following established patterns.The
HasRelevantReviews()method is efficiently implemented usingAny()and maintains consistency with existing error handling patterns in the class. This is a good addition for supporting the AI analysis workflow.src/Infrastructure/Services/OpenAi/Models/Choice.cs (1)
5-9: LGTM - Well-designed record for OpenAI API integration.The
Choicerecord is properly structured with appropriate JSON serialization attributes and follows good practices for data transfer objects.src/Web.Api/appsettings.json (1)
54-58: Good security practice with placeholder API key.The OpenAI configuration section is well-structured, and using a placeholder API key ("sk-xxxx") in the configuration file is appropriate for security. The actual API key should be injected through environment variables or secure configuration management.
src/Web.Api/Features/Subscribtions/GetOpenAiReportAnalysis/GetOpenAiReportAnalysisHandler.cs (1)
9-10: Clean migration to custom OpenAI service.The transition from
IOpenAiServicetoICustomOpenAiServiceis implemented consistently across using statements, field declaration, and constructor parameter. The service usage in the Handle method correctly calls the expected interface methods.Also applies to: 20-20, 25-25
src/Infrastructure/Services/OpenAi/Custom/ICustomOpenAiService.cs (1)
5-12: Well-designed interface following best practices.The interface is clean and focused, with appropriate method signatures. The
GetAnalysisAsyncmethod correctly follows async patterns with optionalCancellationToken, and theGetBearer()method has a clear purpose for authentication.src/Web.Api/Features/Companies/CompaniesController.cs (1)
103-112: New endpoint follows established patterns correctly.The AI analysis endpoint is implemented consistently with the existing controller methods, including proper admin-only authorization, standard dependency injection pattern, and appropriate route structure.
src/Web.Api/Features/BackgroundJobs/AiAnalysisSubscriptionJob.cs (1)
12-13: Migration to custom OpenAI service looks consistent.The changes correctly update the dependency from
IOpenAiServicetoICustomOpenAiServicealong with the corresponding namespace imports. The job logic remains unchanged, reducing migration risk.Also applies to: 25-25, 31-31
src/Web.Api/Features/Companies/GetCompanyAiAnalysis/GetCompanyAiAnalysisHandler.cs (1)
48-53: Clean delegation to OpenAI service with proper correlation tracking.The handler correctly passes the correlation ID for request tracing and uses the company entity with all necessary related data loaded.
src/Infrastructure/Services/OpenAi/IOpenAiService.cs (1)
8-16: Excellent refactoring to strongly typed results.The interface improvements move from string-based returns to structured
OpenAiChatResultobjects, providing better type safety and error handling capabilities. The optional parameters with sensible defaults enhance usability.
| public record ChatResponse | ||
| { | ||
| [JsonPropertyName("choices")] | ||
| public List<Choice> Choices { get; set; } | ||
| } No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve immutability and null safety for the data model.
The record has several design concerns:
- The
Choicesproperty should be immutable to prevent external modification - Missing null reference handling could cause runtime exceptions
- Missing XML documentation for this public API model
Apply this diff to improve the design:
+/// <summary>
+/// Represents a chat response from OpenAI API containing multiple choices.
+/// </summary>
public record ChatResponse
{
[JsonPropertyName("choices")]
- public List<Choice> Choices { get; set; }
+ public IReadOnlyList<Choice> Choices { get; init; } = Array.Empty<Choice>();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public record ChatResponse | |
| { | |
| [JsonPropertyName("choices")] | |
| public List<Choice> Choices { get; set; } | |
| } | |
| /// <summary> | |
| /// Represents a chat response from OpenAI API containing multiple choices. | |
| /// </summary> | |
| public record ChatResponse | |
| { | |
| [JsonPropertyName("choices")] | |
| public IReadOnlyList<Choice> Choices { get; init; } = Array.Empty<Choice>(); | |
| } |
🤖 Prompt for AI Agents
In src/Infrastructure/Services/OpenAi/Models/ChatResponse.cs around lines 5 to
9, the Choices property should be made immutable by changing it to an init-only
property or a read-only collection to prevent external modification. Initialize
the Choices property to an empty list or use nullable annotations and add null
checks to avoid runtime null reference exceptions. Additionally, add XML
documentation comments to the ChatResponse record and its properties to improve
code clarity and maintainability.
| public ChatMessage() | ||
| { | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unnecessary parameterless constructor.
The parameterless constructor is redundant since records automatically provide one when properties use init accessors.
- public ChatMessage()
- {
- }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public ChatMessage() | |
| { | |
| } |
🤖 Prompt for AI Agents
In src/Infrastructure/Services/OpenAi/Models/ChatMessage.cs at lines 15 to 17,
remove the explicit parameterless constructor since the record type
automatically provides one when properties use init accessors, making this
constructor redundant.
| public record ChatMessage | ||
| { | ||
| public ChatMessage( | ||
| string role, | ||
| string content) | ||
| { | ||
| Role = role; | ||
| Content = content; | ||
| } | ||
|
|
||
| public ChatMessage() | ||
| { | ||
| } | ||
|
|
||
| [JsonPropertyName("role")] | ||
| public string Role { get; set; } // "user", "assistant", or "system" | ||
|
|
||
| [JsonPropertyName("content")] | ||
| public string Content { get; set; } | ||
| } No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve immutability, validation, and documentation.
The record has several design issues:
- Properties should be immutable to prevent external modification
- Role property should validate against expected values mentioned in the comment
- Missing null reference handling
- Missing XML documentation for this public API model
Apply this diff to improve the design:
+/// <summary>
+/// Represents a chat message with role and content for OpenAI API communication.
+/// </summary>
public record ChatMessage
{
+ private static readonly HashSet<string> ValidRoles = new() { "user", "assistant", "system" };
+
public ChatMessage(
string role,
string content)
{
+ if (string.IsNullOrWhiteSpace(role))
+ throw new ArgumentException("Role cannot be null or empty.", nameof(role));
+ if (!ValidRoles.Contains(role))
+ throw new ArgumentException($"Invalid role '{role}'. Expected: {string.Join(", ", ValidRoles)}", nameof(role));
+ if (string.IsNullOrWhiteSpace(content))
+ throw new ArgumentException("Content cannot be null or empty.", nameof(content));
+
Role = role;
Content = content;
}
- public ChatMessage()
- {
- }
[JsonPropertyName("role")]
- public string Role { get; set; } // "user", "assistant", or "system"
+ public string Role { get; init; } = string.Empty; // "user", "assistant", or "system"
[JsonPropertyName("content")]
- public string Content { get; set; }
+ public string Content { get; init; } = string.Empty;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public record ChatMessage | |
| { | |
| public ChatMessage( | |
| string role, | |
| string content) | |
| { | |
| Role = role; | |
| Content = content; | |
| } | |
| public ChatMessage() | |
| { | |
| } | |
| [JsonPropertyName("role")] | |
| public string Role { get; set; } // "user", "assistant", or "system" | |
| [JsonPropertyName("content")] | |
| public string Content { get; set; } | |
| } | |
| /// <summary> | |
| /// Represents a chat message with role and content for OpenAI API communication. | |
| /// </summary> | |
| public record ChatMessage | |
| { | |
| private static readonly HashSet<string> ValidRoles = new() { "user", "assistant", "system" }; | |
| public ChatMessage( | |
| string role, | |
| string content) | |
| { | |
| if (string.IsNullOrWhiteSpace(role)) | |
| throw new ArgumentException("Role cannot be null or empty.", nameof(role)); | |
| if (!ValidRoles.Contains(role)) | |
| throw new ArgumentException($"Invalid role '{role}'. Expected: {string.Join(", ", ValidRoles)}", nameof(role)); | |
| if (string.IsNullOrWhiteSpace(content)) | |
| throw new ArgumentException("Content cannot be null or empty.", nameof(content)); | |
| Role = role; | |
| Content = content; | |
| } | |
| [JsonPropertyName("role")] | |
| public string Role { get; init; } = string.Empty; // "user", "assistant", or "system" | |
| [JsonPropertyName("content")] | |
| public string Content { get; init; } = string.Empty; | |
| } |
🤖 Prompt for AI Agents
In src/Infrastructure/Services/OpenAi/Models/ChatMessage.cs lines 5 to 24, make
the properties immutable by using init-only setters or readonly fields to
prevent external modification. Add validation in the constructor to ensure the
Role value is one of the expected strings ("user", "assistant", or "system") and
throw an exception if invalid. Include null checks for both Role and Content
parameters to prevent null reference issues. Finally, add XML documentation
comments to the class and its members to clearly describe their purpose and
usage.
| using Infrastructure.Database; | ||
| using Infrastructure.Salaries; | ||
| using Infrastructure.Services.OpenAi.Models; | ||
| using Infrastructure.Services.OpenAi.Custom.Models; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Namespace change aligns with OpenAI service refactoring.
The namespace update from Infrastructure.Services.OpenAi.Models to Infrastructure.Services.OpenAi.Custom.Models is consistent with the broader refactoring mentioned in the AI summary.
Verify that all files using the old namespace have been updated consistently:
🏁 Script executed:
#!/bin/bash
# Description: Check for any remaining references to the old namespace
# Expected: No results if all files have been updated
rg "Infrastructure\.Services\.OpenAi\.Models" --type csLength of output: 1027
Incomplete OpenAI service namespace refactoring
Several files still reference the old namespace Infrastructure.Services.OpenAi.Models. Please update these to Infrastructure.Services.OpenAi.Custom.Models to complete the refactoring:
- src/Infrastructure/Services/OpenAi/IOpenAiService.cs (using directive)
- src/Infrastructure/Services/OpenAi/OpenAiService.cs (using directive)
- src/Infrastructure/Services/OpenAi/Models/ChatMessage.cs (namespace declaration)
- src/Infrastructure/Services/OpenAi/Models/ChatResponse.cs (namespace declaration)
- src/Infrastructure/Services/OpenAi/Models/OpenAiChatResult.cs (namespace declaration)
- src/Infrastructure/Services/OpenAi/Models/CompanyAnalyzeRequest.cs (namespace declaration)
- src/Infrastructure/Services/OpenAi/Models/Choice.cs (namespace declaration)
- src/Infrastructure/Services/OpenAi/Models/ChatRequest.cs (namespace declaration)
- src/Web.Api/Features/Companies/GetCompanyAiAnalysis/GetCompanyAiAnalysisHandler.cs (using directive)
Ensure all namespace statements and using directives are consistently updated.
🤖 Prompt for AI Agents
In the files listed including
src/Infrastructure/Services/OpenAi/IOpenAiService.cs, OpenAiService.cs, all
model files under src/Infrastructure/Services/OpenAi/Models/, and
src/Web.Api/Features/Companies/GetCompanyAiAnalysis/GetCompanyAiAnalysisHandler.cs,
update all using directives and namespace declarations from
Infrastructure.Services.OpenAi.Models to
Infrastructure.Services.OpenAi.Custom.Models to complete the refactoring and
ensure consistency across the codebase.
| using Domain.ValueObjects.Pagination; | ||
| using Infrastructure.Services.Mediator; | ||
| using Infrastructure.Services.OpenAi.Models; | ||
| using Infrastructure.Services.OpenAi.Custom.Models; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Namespace swap looks correct – double-check DI & naming consistency
Switching to Infrastructure.Services.OpenAi.Custom.Models is in line with the new ICustomOpenAiService.
Please make sure:
- Service registration (
ServiceRegistration.cs) adds the custom service before consumers are resolved. - Any lingering
Infrastructure.Services.OpenAi.Modelsusings in the same project are removed to prevent ambiguous-type compilation errors. - (Unrelated spelling nit) the folder/route still uses “Subscribtions”. If you ever plan a breaking rename, keep routing/versioning in mind.
Run:
🏁 Script executed:
#!/bin/bash
# Verify no old namespace references remain in Web.Api project
rg --context 1 $'Infrastructure.Services.OpenAi.Models' src/Web.ApiLength of output: 435
Lingering old namespace reference detected – please remove and verify DI ordering
- In src/Web.Api/Features/Companies/GetCompanyAiAnalysis/GetCompanyAiAnalysisHandler.cs, remove the lingering
using Infrastructure.Services.OpenAi.Models;
- In ServiceRegistration.cs, ensure you register the custom implementation of ICustomOpenAiService before any consumers are resolved (i.e., before any default OpenAi service registrations).
- (Nit) The “Subscribtions” folder/route is misspelled. If you plan to rename it to “Subscriptions,” account for breaking‐change impacts on routes and versioning.
🤖 Prompt for AI Agents
In src/Web.Api/Features/Subscribtions/TelegramSubscriptionsController.cs at line
7, remove the unused or outdated namespace reference to
Infrastructure.Services.OpenAi.Custom.Models. Additionally, check
src/Web.Api/Features/Companies/GetCompanyAiAnalysis/GetCompanyAiAnalysisHandler.cs
and remove the using statement for Infrastructure.Services.OpenAi.Models. In
ServiceRegistration.cs, ensure the custom ICustomOpenAiService implementation is
registered before any default OpenAi services to maintain correct dependency
injection order. Also, consider renaming the "Subscribtions" folder and route to
"Subscriptions" carefully, accounting for any breaking changes in routes and
versioning.
| TotalRating = review.TotalRating; | ||
| } |
There was a problem hiding this comment.
Remove duplicate TotalRating assignment.
TotalRating is assigned twice in the constructor.
TotalRating = review.TotalRating;
LikesCount = review.LikesCount;
DislikesCount = review.DislikesCount;
- TotalRating = review.TotalRating;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| TotalRating = review.TotalRating; | |
| } | |
| // in CompanyAnalyzeRequest constructor, replace these lines: | |
| TotalRating = review.TotalRating; | |
| LikesCount = review.LikesCount; | |
| DislikesCount = review.DislikesCount; | |
| // (remove the duplicate `TotalRating = review.TotalRating;` here) |
🤖 Prompt for AI Agents
In src/Infrastructure/Services/OpenAi/Models/CompanyAnalyzeRequest.cs at lines
46 to 47, there is a duplicate assignment of the TotalRating property in the
constructor. Remove the redundant assignment so that TotalRating is only
assigned once to avoid unnecessary repetition.
| RatingHistoryRecords = company.RatingHistory? | ||
| .Select(history => new CompanyRatingHistoryItem(history)) | ||
| .ToList(); | ||
|
|
||
| Reviews = company.Reviews | ||
| .Select(x => new CompanyReviewData(x)) | ||
| .ToList(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add consistent null checking for defensive programming.
The code uses null-conditional operator for RatingHistory but not for Reviews. Consider adding null checks or using consistent patterns to prevent potential NullReferenceException.
RatingHistoryRecords = company.RatingHistory?
.Select(history => new CompanyRatingHistoryItem(history))
.ToList();
- Reviews = company.Reviews
+ Reviews = company.Reviews?
.Select(x => new CompanyReviewData(x))
- .ToList();
+ .ToList() ?? new List<CompanyReviewData>();Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/Infrastructure/Services/OpenAi/Models/CompanyAnalyzeRequest.cs between
lines 12 and 18, the code uses a null-conditional operator for RatingHistory but
lacks null checking for Reviews, which can cause NullReferenceException. Add a
null-conditional operator or a null check before selecting and converting
Reviews to a list, ensuring consistent defensive programming and preventing
runtime errors.
| responseContent = await response.Content.ReadAsStringAsync(cancellationToken); | ||
| if (response.IsSuccessStatusCode) | ||
| { | ||
| return responseContent; | ||
| } | ||
|
|
||
| _logger.LogError( | ||
| "Failed request to OpenAI {Url}. Status {Status}, Response {Response}", | ||
| apiUrl, | ||
| response.StatusCode, | ||
| responseContent); | ||
|
|
||
| return string.Empty; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using a Result pattern instead of returning empty string on failure.
Returning an empty string on failure makes it difficult for callers to distinguish between an empty successful response and a failure. Consider using a Result pattern or throwing exceptions.
🤖 Prompt for AI Agents
In src/Infrastructure/Services/OpenAi/Custom/CustomOpenAiService.cs around lines
58 to 71, the method currently returns an empty string on failure, which makes
it hard to differentiate between a failed request and a successful empty
response. Refactor the method to return a Result type that encapsulates success
or failure states along with the response content or error information, or
alternatively throw exceptions on failure to clearly signal errors to the
caller.
| public string GetBearer() | ||
| { | ||
| var secret = _configuration["OpenAiApiSecret"]; | ||
| return new TechinterviewJwtTokenGenerator(secret).ToString(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Cache or optimize JWT token generation.
Creating a new TechinterviewJwtTokenGenerator instance on every call is inefficient. Consider caching the generator or the token with appropriate expiration.
+ private TechinterviewJwtTokenGenerator _jwtGenerator;
+
public string GetBearer()
{
var secret = _configuration["OpenAiApiSecret"];
- return new TechinterviewJwtTokenGenerator(secret).ToString();
+ _jwtGenerator ??= new TechinterviewJwtTokenGenerator(secret);
+ return _jwtGenerator.ToString();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public string GetBearer() | |
| { | |
| var secret = _configuration["OpenAiApiSecret"]; | |
| return new TechinterviewJwtTokenGenerator(secret).ToString(); | |
| } | |
| private TechinterviewJwtTokenGenerator _jwtGenerator; | |
| public string GetBearer() | |
| { | |
| var secret = _configuration["OpenAiApiSecret"]; | |
| _jwtGenerator ??= new TechinterviewJwtTokenGenerator(secret); | |
| return _jwtGenerator.ToString(); | |
| } |
🤖 Prompt for AI Agents
In src/Infrastructure/Services/OpenAi/Custom/CustomOpenAiService.cs around lines
27 to 31, the GetBearer method creates a new TechinterviewJwtTokenGenerator
instance on every call, which is inefficient. Refactor the code to cache either
the TechinterviewJwtTokenGenerator instance or the generated JWT token,
implementing appropriate expiration logic to refresh the token when needed,
thereby avoiding unnecessary object creation and improving performance.
| using var client = _httpClientFactory.CreateClient(); | ||
|
|
||
| client.BaseAddress = new Uri(apiUrl); | ||
| var request = new HttpRequestMessage(HttpMethod.Post, apiUrl); | ||
| request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", GetBearer()); | ||
| request.Content = new StringContent( | ||
| JsonSerializer.Serialize(report), | ||
| Encoding.UTF8, | ||
| "application/json"); |
There was a problem hiding this comment.
Fix HTTP client usage pattern.
Setting BaseAddress after creating the client and then using the full URL in the request is redundant and potentially incorrect.
using var client = _httpClientFactory.CreateClient();
- client.BaseAddress = new Uri(apiUrl);
- var request = new HttpRequestMessage(HttpMethod.Post, apiUrl);
+ var request = new HttpRequestMessage(HttpMethod.Post, apiUrl);
request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", GetBearer());📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| using var client = _httpClientFactory.CreateClient(); | |
| client.BaseAddress = new Uri(apiUrl); | |
| var request = new HttpRequestMessage(HttpMethod.Post, apiUrl); | |
| request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", GetBearer()); | |
| request.Content = new StringContent( | |
| JsonSerializer.Serialize(report), | |
| Encoding.UTF8, | |
| "application/json"); | |
| using var client = _httpClientFactory.CreateClient(); | |
| var request = new HttpRequestMessage(HttpMethod.Post, apiUrl); | |
| request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", GetBearer()); | |
| request.Content = new StringContent( | |
| JsonSerializer.Serialize(report), | |
| Encoding.UTF8, | |
| "application/json"); |
🤖 Prompt for AI Agents
In src/Infrastructure/Services/OpenAi/Custom/CustomOpenAiService.cs around lines
46 to 54, the code sets the HttpClient's BaseAddress but then uses the full URL
in the HttpRequestMessage, which is redundant and can cause errors. To fix this,
either remove setting BaseAddress and keep using the full URL in the request, or
set BaseAddress and use a relative URI in the HttpRequestMessage constructor.
Adjust the code to follow one consistent pattern for HttpClient usage.
Summary by CodeRabbit
New Features
Configuration
Refactor
Bug Fixes
Chores