Feature/bifrost bridge setup - #1
Merged
Merged
Conversation
…; rename WebClientConfiguration to RestClientConfiguration
…pdate ChatMessage timestamp handling
… add OpenRouterResponseDTO and update API endpoints Move API key value to EV
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughIntroduces a complete OpenRouter LLM chat integration service with REST endpoints, session storage, and message history management. Includes domain models, DTOs, HTTP configuration, service layer, and controller exposing ChangesOpenRouter Chat Integration
Sequence DiagramsequenceDiagram
participant Client
participant Controller as BifrostController
participant Service as ChatService
participant Storage as ChatSessionStorage
participant RestClient
participant OpenRouter
Client->>Controller: POST /v1/chat (ChatRequestDTO)
Controller->>Controller: Log personality, message, sessionId
Controller->>Service: sendRequestToLLM(dto)
Service->>Storage: getOrCreateChatSession(sessionId)
Storage-->>Service: ChatSession
Service->>Service: Append user message to session
Service->>Service: Build apiMessages with system prompt + history
Service->>RestClient: POST /chat/completions (OpenRouterRequestDTO)
RestClient->>OpenRouter: POST request with auth headers
OpenRouter-->>RestClient: OpenRouterResponseDTO
RestClient-->>Service: Response body
Service->>Service: Parse & validate response
Service->>Service: Extract first choice content
Service->>Service: Append assistant message to session
Service-->>Controller: LLM response string
Controller-->>Client: Response (200 OK)
Client->>Controller: GET /v1/chat/{sessionId}
Controller->>Service: getSessionHistory(sessionId)
Service->>Storage: getOrCreateChatSession(sessionId)
Storage-->>Service: ChatSession
Service-->>Controller: ChatSession
Controller-->>Client: ChatSession (200 OK)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. Comment |
There was a problem hiding this comment.
Pull request overview
This PR adds the initial backend for a Norse-themed chat application: REST endpoints to send messages and retrieve chat history, in-memory session storage, and integration with the OpenRouter chat-completions API with personality-driven system prompts.
Changes:
- Added REST controller endpoints for chat and session-history retrieval.
- Implemented chat domain/session storage and a service that builds OpenRouter requests and persists conversation history.
- Added OpenRouter RestClient configuration and basic application properties for API key + HTTP timeouts.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/resources/application.properties | Adds OpenRouter API key binding and HTTP client timeout properties. |
| src/main/java/org/example/projectbifrost/BifrostController.java | Introduces REST endpoints for welcome, chat send, and session history retrieval. |
| src/main/java/org/example/projectbifrost/configuration/RestClientConfiguration.java | Configures a RestClient targeting OpenRouter with auth headers. |
| src/main/java/org/example/projectbifrost/domain/ChatMessage.java | Adds message domain model with role/content/timestamp. |
| src/main/java/org/example/projectbifrost/domain/ChatSession.java | Adds session domain model with message history and append helper. |
| src/main/java/org/example/projectbifrost/domain/PersonalityPromptProvider.java | Maps personalities to system prompts for LLM context. |
| src/main/java/org/example/projectbifrost/dto/ChatRequestDTO.java | Adds request DTO carrying personality, message, and sessionId. |
| src/main/java/org/example/projectbifrost/dto/OpenRouterResponseDTO.java | Adds response DTO for parsing OpenRouter choices/messages. |
| src/main/java/org/example/projectbifrost/dto/Personality.java | Adds supported personality enum values. |
| src/main/java/org/example/projectbifrost/service/ChatService.java | Implements session-aware LLM request/response logic and persistence to history. |
| src/main/java/org/example/projectbifrost/storage/ChattSessionStorage.java | Adds thread-oriented in-memory session storage component. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…on; rename ChattSessionStorage to ChatSessionStorage
…pdate field declaration
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (9)
src/main/java/org/example/projectbifrost/domain/ChatMessage.java (2)
17-18: 💤 Low value
@Setteris unnecessary —ChatMessageis write-once.
ChatMessageis only ever created via its constructor and subsequently stored in session history. No code path mutates fields after construction. Removing@Settermakes the immutability intent explicit and prevents accidental mutation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/org/example/projectbifrost/domain/ChatMessage.java` around lines 17 - 18, Remove the Lombok `@Setter` from the ChatMessage class to enforce write-once semantics: delete the `@Setter` annotation (and its import if present), mark instance fields in ChatMessage as final so they can only be set via the existing constructor, and keep `@Getter` for read access; ensure the constructor initializes all final fields and remove any callers/tests that rely on mutating setters.
22-27: ⚡ Quick winReplace
LocalDateTime.now()with a timezone-aware type.
LocalDateTimecarries no timezone information. If the service ever runs in multiple JVMs or regions, timestamps from different nodes become incomparable.Instant.now()(orZonedDateTime.now(ZoneOffset.UTC)) is unambiguous.♻️ Proposed fix
-import java.time.LocalDateTime; +import java.time.Instant; `@Getter` `@Setter` public class ChatMessage { private String role; private String content; - private LocalDateTime timeStamp; + private Instant timeStamp; public ChatMessage(String role, String message) { this.role = role; this.content = message; - this.timeStamp = LocalDateTime.now(); + this.timeStamp = Instant.now(); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/org/example/projectbifrost/domain/ChatMessage.java` around lines 22 - 27, The ChatMessage class uses a LocalDateTime timeStamp set in the constructor; replace it with a timezone-aware type (e.g., change the timeStamp field type to java.time.Instant or java.time.ZonedDateTime) and set it using Instant.now() (or ZonedDateTime.now(ZoneOffset.UTC)) inside the ChatMessage(String role, String message) constructor; also update imports and any getters/setters/serialization code that reference timeStamp to use the new type to ensure timestamps are unambiguous across JVMs/regions.src/main/java/org/example/projectbifrost/dto/OpenRouterRequestDTO.java (1)
6-7: ⚡ Quick winConsider replacing
List<Map<String, String>>with a dedicated message record.
List<Map<String, String>>is stringly-typed — a typo in a key ("rol"instead of"role") silently produces a malformed request to OpenRouter. A nested record enforces the contract at compile time.♻️ Proposed refactor
import java.util.List; -import java.util.Map; +public record OpenRouterRequestDTO(String model, List<Message> messages) { + public record Message(String role, String content) {} +} -public record OpenRouterRequestDTO(String model, - List<Map<String, String>> messages) { -}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/org/example/projectbifrost/dto/OpenRouterRequestDTO.java` around lines 6 - 7, The messages field in OpenRouterRequestDTO is currently a List<Map<String,String>> which is error-prone; replace it with a dedicated nested record (e.g., create record MessageDTO(String role, String content) or a top-level MessageDTO) and change the type to List<MessageDTO> in OpenRouterRequestDTO (constructor and accessor names remain as in the record). Update any code constructing or serializing OpenRouterRequestDTO to build MessageDTO instances instead of maps and adjust JSON/serialization mappings if needed so keys are enforced as "role" and "content" at compile time.src/main/java/org/example/projectbifrost/domain/PersonalityPromptProvider.java (1)
9-9: ⚡ Quick winAddress the TODO: move prompts into the
Personalityenum.The Swedish comment flags this as a deferred refactor. Co-locating each constant's prompt directly in the enum eliminates the need for
PersonalityPromptProviderentirely, reduces indirection, and ensures a new personality value can't be added without also providing its prompt.Would you like me to generate a refactored
Personalityenum that embeds the prompt strings and replacesPersonalityPromptProvider?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/org/example/projectbifrost/domain/PersonalityPromptProvider.java` at line 9, PersonalityPromptProvider is an indirection for storing prompts; move each prompt into the Personality enum and remove PersonalityPromptProvider: add a prompt field and accessor (e.g., private final String prompt; public String getPrompt()) to the Personality enum and initialize it for every enum constant, update callers to use Personality.getPrompt() instead of PersonalityPromptProvider.getPrompt(personality), and delete the PersonalityPromptProvider class and any unused imports.src/main/java/org/example/projectbifrost/BifrostController.java (2)
13-13: 💤 Low value
LoggerFactoryreferenced via fully-qualified name rather than an import.
Loggeris imported butLoggerFactoryis used inline. The idiomatic pattern is to import both and useLoggerFactory.getLogger(...).♻️ Proposed fix
import org.slf4j.Logger; +import org.slf4j.LoggerFactory; ... - private static final Logger logger = org.slf4j.LoggerFactory.getLogger(BifrostController.class); + private static final Logger logger = LoggerFactory.getLogger(BifrostController.class);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/org/example/projectbifrost/BifrostController.java` at line 13, Replace the fully-qualified reference to org.slf4j.LoggerFactory with an import and use it directly: add an import for org.slf4j.LoggerFactory and update the static logger initialization to use LoggerFactory.getLogger(BifrostController.class) (keeping the existing Logger type and BifrostController class name).
25-29: ⚡ Quick winNo input validation on
ChatRequestDTO— missing@Valid.Without
@Valid(plus constraint annotations on the DTO), a request body with anullmessage ornullsessionId will reachChatServiceand either produce aNullPointerExceptionor silently create malformed state. Adding@Validwith@NotBlankon the DTO fields closes this gap cheaply.♻️ Proposed fix
- public String sendChatRequest(`@RequestBody` ChatRequestDTO dto) { + public String sendChatRequest(`@Valid` `@RequestBody` ChatRequestDTO dto) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/org/example/projectbifrost/BifrostController.java` around lines 25 - 29, The controller currently accepts ChatRequestDTO without validation; update sendChatRequest in BifrostController to annotate the request body with `@Valid` (i.e., change the parameter to `@Valid` `@RequestBody` ChatRequestDTO dto) and add bean validation annotations to the DTO (e.g., annotate message() and sessionId() with `@NotBlank` in ChatRequestDTO; if it's a record annotate the record components). Ensure the necessary imports (javax.validation.Valid / jakarta.validation.Valid and javax.validation.constraints.NotBlank or jakarta equivalents) are added and, if your app requires, add `@Validated` to the controller class to enable method-level validation.src/main/java/org/example/projectbifrost/storage/ChatSessionStorage.java (2)
31-34: 💤 Low valueRemove the Swedish Copilot artifact comment.
Line 33 is a raw Copilot suggestion left in production code. Either convert it to a tracked
// TODO:item in English or remove it entirely.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/org/example/projectbifrost/storage/ChatSessionStorage.java` around lines 31 - 34, In ChatSessionStorage.deleteSession(String sessionId), remove the leftover Swedish Copilot comment on line 33; either delete the artifact entirely or replace it with an English tracked TODO (e.g., "// TODO: consider bounding sessionStorage size and evict oldest entries when exceeding MAX_SIZE") so the method contains only valid production comments and the intent is recorded.
21-34: No eviction policy — sessions accumulate indefinitely in memory.
sessionStorageis unbounded. A long-running deployment will accumulate a session entry per uniquesessionIdforever. Consider a time-based eviction strategy (e.g.Caffeinecache withexpireAfterAccess) or a maximum-capacity policy;deleteSessioncurrently has no callers so unused sessions are never reclaimed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/org/example/projectbifrost/storage/ChatSessionStorage.java` around lines 21 - 34, sessionStorage is unbounded and will leak memory; replace the ConcurrentHashMap with a bounded/time-expiring cache (e.g., Caffeine) and update getOrCreateChatSession/deleteSession to use cache APIs: use a Cache<String,ChatSession> with expireAfterAccess and maximumSize configured, implement getOrCreateChatSession via cache.get(sessionId, id -> new ChatSession(id, synchronizedList(new ArrayList<>()))), and implement deleteSession via cache.invalidate(sessionId); add the Caffeine dependency and configuration where this class is constructed.src/main/java/org/example/projectbifrost/domain/ChatSession.java (1)
18-20: ⚡ Quick win
@SetteronchatHistorybreaks the thread-safety contract.
setChatHistory(List<ChatMessage>)lets any caller swap in an unsynchronizedArrayList, silently discarding theCollections.synchronizedListinvariant established byChatSessionStorage. Remove@Setter(or, at minimum, guard it);@Getteralone is sufficient for the current usage.♻️ Proposed fix
`@Getter` -@Setter `@AllArgsConstructor` public class ChatSession {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/org/example/projectbifrost/domain/ChatSession.java` around lines 18 - 20, The `@Setter` on ChatSession allows callers to replace chatHistory with an unsynchronized List, violating the synchronized-list invariant established by ChatSessionStorage; remove the Lombok `@Setter` (keep `@Getter` and `@AllArgsConstructor`) for the chatHistory field in class ChatSession, or if you must keep a setter, make setChatHistory(List<ChatMessage>) defensive by wrapping/copying the incoming List into Collections.synchronizedList(new ArrayList<>(incoming)) (and assign that), ensuring chatHistory always remains a synchronized list; update any references that relied on the generated setter accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/org/example/projectbifrost/BifrostController.java`:
- Line 27: The current logging statement in BifrostController (logger.info(...))
prints dto.message() and dto.sessionId(), which may expose PII; remove the
message body from logs and avoid including identifiable session IDs unless
allowed by policy. Update the logger.info call in BifrostController to log only
non-sensitive metadata (e.g., dto.personality()) and, if session tracking is
required, replace dto.sessionId() with a redacted/hashed value or a
boolean/opaque identifier that complies with data-handling rules; ensure
dto.message() is never logged at INFO level (or at all) and adjust any related
log messages or helper methods accordingly.
In
`@src/main/java/org/example/projectbifrost/configuration/RestClientConfiguration.java`:
- Around line 15-23: The openAIWebClient method should use the auto-configured
RestClient.Builder bean instead of RestClient.builder() so Spring Boot's
timeouts and customizers are preserved; update the RestClientConfiguration by
injecting a RestClient.Builder (e.g., add a parameter to the openAIWebClient
method or constructor) and call
builder.baseUrl(URI.create("https://openrouter.ai/api/v1")).defaultHeader("Authorization",
"Bearer " + apiKey).defaultHeader("Content-Type", "application/json").build()
using that injected Builder, leaving the bean name openAIWebClient and
preserving apiKey usage.
In `@src/main/java/org/example/projectbifrost/dto/ChatRequestDTO.java`:
- Around line 10-12: Update the ChatRequestDTO record to enforce non-empty
strings: add `@NotBlank` to the message parameter and replace `@NotNull` with
`@NotBlank` on the sessionId parameter in the ChatRequestDTO declaration, and
ensure the NotBlank import (e.g., javax.validation.constraints.NotBlank) is
present; this prevents blank messages from being sent and empty sessionId values
from bypassing validation.
In `@src/main/java/org/example/projectbifrost/service/ChatService.java`:
- Around line 73-75: getSessionHistory currently calls
ChatSessionStorage.getOrCreateChatSession which silently creates empty sessions
for unknown IDs; change storage to add a getSession(String sessionId) that
returns Optional<ChatSession>, update ChatService.getSessionHistory to call
chatSessionStorage.getSession(sessionId) and return/propagate the Optional (or
throw a NotFound-like exception), and update the controller handling the GET
/v1/chat/{sessionId} to translate an empty Optional into a 404 response instead
of returning an empty ChatSession; keep getOrCreateChatSession for the
send-message flow only.
- Around line 47-53: The iteration over chatSession.getChatHistory() is unsafe
because the list is a synchronizedList and must be iterated while holding its
monitor; fix by taking a thread-safe snapshot or locking the list: either
synchronize on the chatHistory object returned by chatSession.getChatHistory()
before calling .stream() (e.g., synchronized(chatHistory) { ... stream(...) ...
}) or create a local copy (e.g., new ArrayList<>(chatHistory)) and stream over
that when building messages in ChatService; alternatively change the storage to
use CopyOnWriteArrayList in ChatSessionStorage if append-heavy, read-safe
semantics are desired.
- Around line 57-67: The call to
restClient.post()...body(OpenRouterResponseDTO.class) can return null and
response.choices() can be empty, leading to NPE or NoSuchElementException when
you call response.choices().getFirst().message().content(); update ChatService
to defensively handle these cases: after receiving OpenRouterResponseDTO
response, check response != null, response.choices() != null and not empty (or
use Optional/stream to findFirst safely) before accessing message/content, and
if missing throw a clear RuntimeException (or custom exception) including
context (e.g., "Empty OpenRouter response or no choices") and any available
response metadata so callers don't crash unexpectedly. Ensure the null/empty
checks cover the symbols response, choices(), getFirst()/findFirst(), and
message()/content().
In `@src/main/resources/application.properties`:
- Around line 4-5: The timeout properties in application.properties won't affect
the custom RestClient built in RestClientConfiguration because it uses
RestClient.builder() directly; update RestClientConfiguration to either inject
the auto-configured RestClient.Builder from Spring (so
spring.http.client.connect-timeout and spring.http.client.read-timeout are
applied) or read those properties and apply them when constructing the
RestClient (e.g., configure the underlying HTTP client's connect/read timeouts
on the builder created in RestClientConfiguration instead of calling
RestClient.builder() with defaults). Reference RestClientConfiguration,
RestClient.builder(), and the properties spring.http.client.connect-timeout /
spring.http.client.read-timeout when making the change.
---
Nitpick comments:
In `@src/main/java/org/example/projectbifrost/BifrostController.java`:
- Line 13: Replace the fully-qualified reference to org.slf4j.LoggerFactory with
an import and use it directly: add an import for org.slf4j.LoggerFactory and
update the static logger initialization to use
LoggerFactory.getLogger(BifrostController.class) (keeping the existing Logger
type and BifrostController class name).
- Around line 25-29: The controller currently accepts ChatRequestDTO without
validation; update sendChatRequest in BifrostController to annotate the request
body with `@Valid` (i.e., change the parameter to `@Valid` `@RequestBody`
ChatRequestDTO dto) and add bean validation annotations to the DTO (e.g.,
annotate message() and sessionId() with `@NotBlank` in ChatRequestDTO; if it's a
record annotate the record components). Ensure the necessary imports
(javax.validation.Valid / jakarta.validation.Valid and
javax.validation.constraints.NotBlank or jakarta equivalents) are added and, if
your app requires, add `@Validated` to the controller class to enable method-level
validation.
In `@src/main/java/org/example/projectbifrost/domain/ChatMessage.java`:
- Around line 17-18: Remove the Lombok `@Setter` from the ChatMessage class to
enforce write-once semantics: delete the `@Setter` annotation (and its import if
present), mark instance fields in ChatMessage as final so they can only be set
via the existing constructor, and keep `@Getter` for read access; ensure the
constructor initializes all final fields and remove any callers/tests that rely
on mutating setters.
- Around line 22-27: The ChatMessage class uses a LocalDateTime timeStamp set in
the constructor; replace it with a timezone-aware type (e.g., change the
timeStamp field type to java.time.Instant or java.time.ZonedDateTime) and set it
using Instant.now() (or ZonedDateTime.now(ZoneOffset.UTC)) inside the
ChatMessage(String role, String message) constructor; also update imports and
any getters/setters/serialization code that reference timeStamp to use the new
type to ensure timestamps are unambiguous across JVMs/regions.
In `@src/main/java/org/example/projectbifrost/domain/ChatSession.java`:
- Around line 18-20: The `@Setter` on ChatSession allows callers to replace
chatHistory with an unsynchronized List, violating the synchronized-list
invariant established by ChatSessionStorage; remove the Lombok `@Setter` (keep
`@Getter` and `@AllArgsConstructor`) for the chatHistory field in class ChatSession,
or if you must keep a setter, make setChatHistory(List<ChatMessage>) defensive
by wrapping/copying the incoming List into Collections.synchronizedList(new
ArrayList<>(incoming)) (and assign that), ensuring chatHistory always remains a
synchronized list; update any references that relied on the generated setter
accordingly.
In
`@src/main/java/org/example/projectbifrost/domain/PersonalityPromptProvider.java`:
- Line 9: PersonalityPromptProvider is an indirection for storing prompts; move
each prompt into the Personality enum and remove PersonalityPromptProvider: add
a prompt field and accessor (e.g., private final String prompt; public String
getPrompt()) to the Personality enum and initialize it for every enum constant,
update callers to use Personality.getPrompt() instead of
PersonalityPromptProvider.getPrompt(personality), and delete the
PersonalityPromptProvider class and any unused imports.
In `@src/main/java/org/example/projectbifrost/dto/OpenRouterRequestDTO.java`:
- Around line 6-7: The messages field in OpenRouterRequestDTO is currently a
List<Map<String,String>> which is error-prone; replace it with a dedicated
nested record (e.g., create record MessageDTO(String role, String content) or a
top-level MessageDTO) and change the type to List<MessageDTO> in
OpenRouterRequestDTO (constructor and accessor names remain as in the record).
Update any code constructing or serializing OpenRouterRequestDTO to build
MessageDTO instances instead of maps and adjust JSON/serialization mappings if
needed so keys are enforced as "role" and "content" at compile time.
In `@src/main/java/org/example/projectbifrost/storage/ChatSessionStorage.java`:
- Around line 31-34: In ChatSessionStorage.deleteSession(String sessionId),
remove the leftover Swedish Copilot comment on line 33; either delete the
artifact entirely or replace it with an English tracked TODO (e.g., "// TODO:
consider bounding sessionStorage size and evict oldest entries when exceeding
MAX_SIZE") so the method contains only valid production comments and the intent
is recorded.
- Around line 21-34: sessionStorage is unbounded and will leak memory; replace
the ConcurrentHashMap with a bounded/time-expiring cache (e.g., Caffeine) and
update getOrCreateChatSession/deleteSession to use cache APIs: use a
Cache<String,ChatSession> with expireAfterAccess and maximumSize configured,
implement getOrCreateChatSession via cache.get(sessionId, id -> new
ChatSession(id, synchronizedList(new ArrayList<>()))), and implement
deleteSession via cache.invalidate(sessionId); add the Caffeine dependency and
configuration where this class is constructed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b2ec9e2a-c0cf-4ff8-a7f5-cec37bea2b84
📒 Files selected for processing (12)
src/main/java/org/example/projectbifrost/BifrostController.javasrc/main/java/org/example/projectbifrost/configuration/RestClientConfiguration.javasrc/main/java/org/example/projectbifrost/domain/ChatMessage.javasrc/main/java/org/example/projectbifrost/domain/ChatSession.javasrc/main/java/org/example/projectbifrost/domain/PersonalityPromptProvider.javasrc/main/java/org/example/projectbifrost/dto/ChatRequestDTO.javasrc/main/java/org/example/projectbifrost/dto/OpenRouterRequestDTO.javasrc/main/java/org/example/projectbifrost/dto/OpenRouterResponseDTO.javasrc/main/java/org/example/projectbifrost/dto/Personality.javasrc/main/java/org/example/projectbifrost/service/ChatService.javasrc/main/java/org/example/projectbifrost/storage/ChatSessionStorage.javasrc/main/resources/application.properties
|
|
||
| @PostMapping("/v1/chat") | ||
| public String sendChatRequest(@RequestBody ChatRequestDTO dto) { | ||
| logger.info("Received chat request: Personality={}, Message={}, SessionId={}", dto.personality(), dto.message(), dto.sessionId()); |
There was a problem hiding this comment.
Logging user message content at INFO level is a PII/compliance risk.
dto.message() is arbitrary user-supplied chat text and must not appear in application logs — it can contain PII, credentials, or other sensitive content. dto.sessionId() can also serve as a user identifier. At minimum, omit the message body from the log line; if session tracking in logs is required, ensure it is covered by your data-handling policy.
🛡️ Proposed fix
- logger.info("Received chat request: Personality={}, Message={}, SessionId={}", dto.personality(), dto.message(), dto.sessionId());
+ logger.info("Received chat request: Personality={}, SessionId={}", dto.personality(), dto.sessionId());📝 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.
Suggested change
| logger.info("Received chat request: Personality={}, Message={}, SessionId={}", dto.personality(), dto.message(), dto.sessionId()); | |
| logger.info("Received chat request: Personality={}, SessionId={}", dto.personality(), dto.sessionId()); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/org/example/projectbifrost/BifrostController.java` at line 27,
The current logging statement in BifrostController (logger.info(...)) prints
dto.message() and dto.sessionId(), which may expose PII; remove the message body
from logs and avoid including identifiable session IDs unless allowed by policy.
Update the logger.info call in BifrostController to log only non-sensitive
metadata (e.g., dto.personality()) and, if session tracking is required, replace
dto.sessionId() with a redacted/hashed value or a boolean/opaque identifier that
complies with data-handling rules; ensure dto.message() is never logged at INFO
level (or at all) and adjust any related log messages or helper methods
accordingly.
…message and session ID validation
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/org/example/projectbifrost/service/ChatService.java`:
- Around line 39-40: The code currently mutates session state
(chatSession.addMessage(new ChatMessage("user", dto.message())) in ChatService)
before calling the external OpenRouter API, which can leave orphaned/duplicated
turns on failure; change the flow to build the payload (apiMessages) including
the pending user input first, perform the OpenRouter /chat/completions call and
await success, then only on success append the user message and the assistant
response to the ChatSession via chatSessionStorage.getOrCreateChatSession(...)
and chatSession.addMessage(...); ensure any retries or exceptions do not mutate
session state until after the external call returns successfully.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 46b3eeac-f610-4529-8999-6d92e655984a
📒 Files selected for processing (5)
src/main/java/org/example/projectbifrost/dto/ChatRequestDTO.javasrc/main/java/org/example/projectbifrost/dto/OpenRouterRequestDTO.javasrc/main/java/org/example/projectbifrost/dto/Personality.javasrc/main/java/org/example/projectbifrost/service/ChatService.javasrc/main/resources/application.properties
✅ Files skipped from review due to trivial changes (2)
- src/main/java/org/example/projectbifrost/dto/Personality.java
- src/main/resources/application.properties
… handling; ensure message addition occurs after validation
This was referenced May 5, 2026
Merged
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces the core backend functionality for a Norse mythology-themed chat application, implementing REST endpoints, chat session management, integration with the OpenRouter LLM API, and personality-driven prompts. The changes establish the main domain models, service layer, storage, and configuration needed for the chat system.
Key changes include:
API Endpoints and Controller:
BifrostControllerto expose REST endpoints for sending chat messages, retrieving chat histories, and a welcome endpoint. The controller delegates chat logic to the service layer and logs incoming requests.Chat Domain and Session Management:
ChatMessageandChatSessiondomain classes to represent individual chat messages and maintain session history. [1] [2]ChattSessionStoragefor thread-safe, in-memory management of chat sessions, supporting retrieval and creation by session ID.Service Layer and LLM Integration:
ChatServiceto handle chat logic: manages session state, builds context-aware requests for the LLM, processes responses, and stores assistant replies. Integrates with OpenRouter via a configurableRestClient.RestClientConfigurationto configure the HTTP client with API key and base URL for OpenRouter.Personality and Prompt Handling:
Personalityenum andPersonalityPromptProviderto generate system prompts tailored to Norse gods, influencing LLM responses according to selected personalities. [1] [2]DTOs and Configuration:
ChatRequestDTO,OpenRouterResponseDTO). [1] [2]application.propertiesto support API key injection and HTTP client timeouts.Summary by CodeRabbit
Release Notes