feat(genai): initial logic for chat completion - #105
Conversation
|
Caution Review failedAn error occurred during the review process. Please try again later. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/orchestrator-service/src/main/java/com/team/bank/orchestrator/DashboardController.java (1)
142-152: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winWrap the genai
/chatcall in try/catch to match the existing resilience pattern.The null-response fallback (Lines 150-152) only covers the case where genai returns a null body, but the
webClientcall itself isn't guarded against connection failures/timeouts, unlike the identical pattern already used for genai/banking calls indashboard()(Lines 95-122). If genai-service is down, this will surface an unhandled 500 instead of degrading gracefully.🛡️ Proposed fix
- ChatResponse response = - webClient - .post() - .uri(genaiServiceUrl + "/chat") - .bodyValue(request) - .retrieve() - .bodyToMono(ChatResponse.class) - .block(); - return response == null - ? new ChatResponse("I could not process that request.", null) - : response; + ChatResponse response = null; + try { + response = + webClient + .post() + .uri(genaiServiceUrl + "/chat") + .bodyValue(request) + .retrieve() + .bodyToMono(ChatResponse.class) + .block(); + } catch (RuntimeException e) { + log.warn("genai-service unavailable, returning fallback chat response", e); + } + return response == null + ? new ChatResponse("I could not process that request.", null) + : response;🤖 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 `@server/orchestrator-service/src/main/java/com/team/bank/orchestrator/DashboardController.java` around lines 142 - 152, The genai chat request in DashboardController’s /chat handler is missing the same resilience wrapper used in dashboard(), so connection failures or timeouts can still bubble up as 500s. Wrap the WebClient post/block sequence for the genaiServiceUrl + "/chat" call in a try/catch, and in the catch return the same graceful fallback ChatResponse used for a null body so the endpoint degrades consistently. Use the existing dashboard() error-handling pattern and the ChatResponse-returning logic as the reference points.
🧹 Nitpick comments (4)
server/banking-service/src/main/java/com/team/bank/banking/controller/BankingController.java (1)
140-155: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid redundant DB query for the ACTIVE lookup.
connections(all rows for the account) is already fetched on Line 142. The extrafindByAccountIdAndStatusquery on Line 150 can be replaced with an in-memory filter overconnections, saving a round-trip per request.♻️ Proposed refactor
List<BankingConnection> connections = connectionRepository.findByAccountId(accountId); if (connections.isEmpty()) { return ResponseEntity.ok(new ConnectionStatus("NONE", null, null)); } // Prefer an ACTIVE connection over stale PENDING rows from earlier, abandoned // OAuth attempts; otherwise surface the most recently updated one. BankingConnection connection = - connectionRepository - .findByAccountIdAndStatus(accountId, "ACTIVE") - .orElseGet( - () -> - connections.stream() - .max(java.util.Comparator.comparing(BankingConnection::getUpdatedAt)) - .orElse(connections.get(0))); + connections.stream() + .filter(c -> "ACTIVE".equals(c.getStatus())) + .findFirst() + .orElseGet( + () -> + connections.stream() + .max(java.util.Comparator.comparing(BankingConnection::getUpdatedAt)) + .orElse(connections.get(0)));🤖 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 `@server/banking-service/src/main/java/com/team/bank/banking/controller/BankingController.java` around lines 140 - 155, The status lookup in BankingController.status is doing a redundant database round-trip by calling findByAccountIdAndStatus after connections has already loaded all rows for the account. Update the ACTIVE selection to filter the existing connections collection in memory within status, then fall back to the most recently updated connection as it already does, keeping the same behavior while removing the extra repository query.genai/tests/test_main.py (1)
28-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding coverage for
logos/ollamaprovider routing and fallback.This test only exercises the default
localprovider path. The newly addedlogos_chatand rewrittenollama_chat, plus the fallback-on-exception branch in/chat, have no test coverage in this file. Mockingrequests.postand togglingMODEL_PROVIDERwould validate the new routing logic end-to-end.🤖 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 `@genai/tests/test_main.py` around lines 28 - 34, The current test only covers the default local provider path, so add coverage in test_chat_messages_shape (or nearby tests) for the new /chat routing logic. Mock requests.post and vary MODEL_PROVIDER to exercise logos_chat and the rewritten ollama_chat paths, and add a case that forces an exception so the fallback branch in /chat is validated end-to-end.genai/main.py (2)
237-245: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winLog the swallowed exception before falling back to
local_chat.The broad
except Exceptionis a sound resilience choice, but silently swallowing the error with no logging will make provider outages (badLOGOS_KEY, network failures, malformed responses) invisible in production.♻️ Proposed fix
+import logging + +logger = logging.getLogger(__name__) + try: if provider == "logos": return logos_chat(messages) if provider == "ollama": return ollama_chat(messages) - except Exception: + except Exception: + logger.exception("chat provider '%s' failed, falling back to local", provider) # Never surface an upstream failure to the user — fall back to the # local canned assistant so the chat panel stays usable. return local_chat(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 `@genai/main.py` around lines 237 - 245, The broad exception handler in the provider dispatch path is hiding upstream failures without any trace, so add logging in the try/except around the provider selection logic before falling back to local_chat. In the function that routes on provider values ("logos" and "ollama"), catch the exception as a variable, log the swallowed error with enough context to identify the provider call that failed, then return local_chat(messages) exactly as before.Source: Linters/SAST tools
97-120: 🔒 Security & Privacy | 🔵 TrivialConfirm data-governance stance on sending dashboard context to Logos.
_context_messageserializes the full dashboard snapshot (balances, trend, expenses, connection/bank info) into the prompt sent externally whenMODEL_PROVIDER=logos(vialogos_chat). If this is intentional per product design, consider whether any fields should be minimized/redacted before leaving the boundary, given the sensitivity of financial data.🤖 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 `@genai/main.py` around lines 97 - 120, Review the _context_message and _normalize_messages flow to confirm the intended data-governance policy for externally sending DashboardContext to Logos. If the full dashboard snapshot is not meant to leave the boundary, minimize or redact sensitive fields before model serialization in _context_message and keep _normalize_messages unchanged except for using the safer context payload. If it is intentional, document that decision clearly in the code near _context_message so the external data transfer is explicit.
🤖 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 `@client/src/api.ts`:
- Around line 80-87: The sendChat payload has changed, so update the /api/chat
OpenAPI contract to match the implementation in sendChat. In
server/openapi.yaml, replace the legacy request body shape with the new fields
used by sendChat, namely messages and context, and make sure the response schema
also includes the reasoning field. Keep the documented request/response models
aligned with the API types so generated clients stay in sync.
- Around line 101-104: Update the ChatReply contract in api.ts so reasoning can
be omitted as well as null, matching the GenAI response shape. Adjust the
ChatReply interface to make reasoning optional in addition to nullable, and keep
the rest of the client parsing flow aligned with that type.
In `@client/src/App.tsx`:
- Around line 205-220: The session persistence helpers in App.tsx are storing
chat transcripts in localStorage by default, which keeps sensitive banking data
across logout and restarts. Update loadSessions and saveSessions to avoid
persistent storage unless the user explicitly opts in, or switch the ChatSession
persistence to session-scoped/ephemeral storage and clear per-account data on
logout; use the existing sessionsStorageKey, loadSessions, and saveSessions
symbols to wire the change through the account flow.
- Around line 612-622: The cleanup in the useEffect around onKey is written as a
concise arrow that returns the removeEventListener call, which the build check
flags as a returned value. Update the effect in App.tsx to use a block-bodied
cleanup function that explicitly calls window.removeEventListener("keydown",
onKey) without returning it, keeping the rest of the Escape-key listener logic
unchanged.
- Around line 347-350: The history built in App.tsx is forwarding an empty
assistant turn when a reply is still pending; update the `history` निर्माण in
the `active.messages`/`userTurn` flow to filter out any pending or empty turns
before slicing and mapping. Make the fix in the logic that prepares
`ChatMessage[]` for the backend so only completed role/content pairs are sent,
even if `active.messages` already contains an in-flight assistant entry.
- Line 626: The onResizeStart handler is using React.MouseEvent without a React
binding in scope, so update the imports in App.tsx to bring in the mouse event
type explicitly from react and switch the handler signature to use that imported
type (for example, a ReactMouseEvent alias). Make the change near onResizeStart
and any other handlers in App that use React.MouseEvent so the type reference
resolves cleanly.
In `@client/src/styles/app.css`:
- Line 739: Replace the deprecated word-wrap declaration with overflow-wrap in
app/css styles so Stylelint stops flagging property-no-deprecated errors. Update
both affected declarations in app.css, keeping the same wrapping behavior, and
verify any related rules near the referenced style blocks still behave as
intended.
- Around line 831-841: Update the .sr-only helper to use the modern
visually-hidden pattern by adding clip-path: inset(50%) alongside the existing
clip fallback. Keep the current .sr-only declarations intact, but ensure the
style block includes clip-path so Stylelint no longer flags clip as the only
hiding mechanism.
---
Outside diff comments:
In
`@server/orchestrator-service/src/main/java/com/team/bank/orchestrator/DashboardController.java`:
- Around line 142-152: The genai chat request in DashboardController’s /chat
handler is missing the same resilience wrapper used in dashboard(), so
connection failures or timeouts can still bubble up as 500s. Wrap the WebClient
post/block sequence for the genaiServiceUrl + "/chat" call in a try/catch, and
in the catch return the same graceful fallback ChatResponse used for a null body
so the endpoint degrades consistently. Use the existing dashboard()
error-handling pattern and the ChatResponse-returning logic as the reference
points.
---
Nitpick comments:
In `@genai/main.py`:
- Around line 237-245: The broad exception handler in the provider dispatch path
is hiding upstream failures without any trace, so add logging in the try/except
around the provider selection logic before falling back to local_chat. In the
function that routes on provider values ("logos" and "ollama"), catch the
exception as a variable, log the swallowed error with enough context to identify
the provider call that failed, then return local_chat(messages) exactly as
before.
- Around line 97-120: Review the _context_message and _normalize_messages flow
to confirm the intended data-governance policy for externally sending
DashboardContext to Logos. If the full dashboard snapshot is not meant to leave
the boundary, minimize or redact sensitive fields before model serialization in
_context_message and keep _normalize_messages unchanged except for using the
safer context payload. If it is intentional, document that decision clearly in
the code near _context_message so the external data transfer is explicit.
In `@genai/tests/test_main.py`:
- Around line 28-34: The current test only covers the default local provider
path, so add coverage in test_chat_messages_shape (or nearby tests) for the new
/chat routing logic. Mock requests.post and vary MODEL_PROVIDER to exercise
logos_chat and the rewritten ollama_chat paths, and add a case that forces an
exception so the fallback branch in /chat is validated end-to-end.
In
`@server/banking-service/src/main/java/com/team/bank/banking/controller/BankingController.java`:
- Around line 140-155: The status lookup in BankingController.status is doing a
redundant database round-trip by calling findByAccountIdAndStatus after
connections has already loaded all rows for the account. Update the ACTIVE
selection to filter the existing connections collection in memory within status,
then fall back to the most recently updated connection as it already does,
keeping the same behavior while removing the extra repository query.
🪄 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 Plus
Run ID: 0bd089ae-b86b-46d8-b276-1ed5d99b1914
📒 Files selected for processing (12)
client/src/App.test.tsxclient/src/App.tsxclient/src/api.tsclient/src/styles/app.cssdocker-compose.ymlgenai/main.pygenai/tests/test_main.pyserver/banking-service/src/main/java/com/team/bank/banking/controller/BankingController.javaserver/orchestrator-service/src/main/java/com/team/bank/orchestrator/DashboardController.javaserver/orchestrator-service/src/main/java/com/team/bank/orchestrator/DashboardModels.javaserver/orchestrator-service/src/main/resources/application.ymlserver/orchestrator-service/src/test/java/com/team/bank/orchestrator/ChatRequestTest.java
| export async function sendChat( | ||
| messages: ChatMessage[], | ||
| context?: ChatContext, | ||
| ): Promise<ChatReply> { | ||
| const response = await fetch(`${API_BASE}/api/chat`, { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify({ message }), | ||
| body: JSON.stringify({ messages, context }), |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Update the documented /api/chat contract with this payload change.
sendChat now posts { messages, context }, but the provided server/openapi.yaml snippet still documents only the legacy { message } body. Generated clients/docs will be out of sync with this PR unless the spec also includes messages, context, and the reasoning response field.
🤖 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 `@client/src/api.ts` around lines 80 - 87, The sendChat payload has changed, so
update the /api/chat OpenAPI contract to match the implementation in sendChat.
In server/openapi.yaml, replace the legacy request body shape with the new
fields used by sendChat, namely messages and context, and make sure the response
schema also includes the reasoning field. Keep the documented request/response
models aligned with the API types so generated clients stay in sync.
| export interface ChatReply { | ||
| reply: string; | ||
| reasoning: string | null; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Allow reasoning to be omitted.
The GenAI response model makes reasoning optional/nullable, so the parsed JSON can be missing this property. Match that contract in the client type.
Proposed fix
export interface ChatReply {
reply: string;
- reasoning: string | null;
+ reasoning?: string | null;
}📝 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.
| export interface ChatReply { | |
| reply: string; | |
| reasoning: string | null; | |
| } | |
| export interface ChatReply { | |
| reply: string; | |
| reasoning?: string | null; | |
| } |
🤖 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 `@client/src/api.ts` around lines 101 - 104, Update the ChatReply contract in
api.ts so reasoning can be omitted as well as null, matching the GenAI response
shape. Adjust the ChatReply interface to make reasoning optional in addition to
nullable, and keep the rest of the client parsing flow aligned with that type.
| function loadSessions(accountId: string): ChatSession[] { | ||
| try { | ||
| const raw = localStorage.getItem(sessionsStorageKey(accountId)); | ||
| if (!raw) { | ||
| return []; | ||
| } | ||
| const parsed = JSON.parse(raw) as ChatSession[]; | ||
| return Array.isArray(parsed) ? parsed : []; | ||
| } catch { | ||
| return []; | ||
| } | ||
| } | ||
|
|
||
| function saveSessions(accountId: string, sessions: ChatSession[]): void { | ||
| try { | ||
| localStorage.setItem(sessionsStorageKey(accountId), JSON.stringify(sessions)); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Avoid retaining banking chat transcripts in localStorage by default.
These sessions can contain sensitive financial context and survive logout/browser restarts while remaining readable to any same-origin script. Prefer explicit opt-in retention, session-scoped storage, or clearing per-account chat data on logout.
🧰 Tools
🪛 GitHub Actions: CI / 0_build-test.txt
[warning] 212-212: ESLint (padding-line-between-statements): Expected blank line before this statement.
🪛 GitHub Actions: CI / build-test
[warning] 212-212: ESLint (padding-line-between-statements): Expected blank line before this statement
🪛 GitHub Check: build-test
[warning] 212-212:
Expected blank line before this statement
🤖 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 `@client/src/App.tsx` around lines 205 - 220, The session persistence helpers
in App.tsx are storing chat transcripts in localStorage by default, which keeps
sensitive banking data across logout and restarts. Update loadSessions and
saveSessions to avoid persistent storage unless the user explicitly opts in, or
switch the ChatSession persistence to session-scoped/ephemeral storage and clear
per-account data on logout; use the existing sessionsStorageKey, loadSessions,
and saveSessions symbols to wire the change through the account flow.
| // Only forward role/content to the backend, capped to the recent window. | ||
| const history: ChatMessage[] = [...active.messages, userTurn] | ||
| .slice(-MAX_HISTORY) | ||
| .map(({ role, content }) => ({ role, content })); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Filter pending turns before sending history.
If the user sends another prompt while a reply is pending, active.messages can include an empty assistant turn, which is then forwarded to the model.
Proposed fix
const history: ChatMessage[] = [...active.messages, userTurn]
+ .filter((m) => !m.pending && m.content.trim())
.slice(-MAX_HISTORY)
.map(({ role, content }) => ({ role, content }));📝 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.
| // Only forward role/content to the backend, capped to the recent window. | |
| const history: ChatMessage[] = [...active.messages, userTurn] | |
| .slice(-MAX_HISTORY) | |
| .map(({ role, content }) => ({ role, content })); | |
| // Only forward role/content to the backend, capped to the recent window. | |
| const history: ChatMessage[] = [...active.messages, userTurn] | |
| .filter((m) => !m.pending && m.content.trim()) | |
| .slice(-MAX_HISTORY) | |
| .map(({ role, content }) => ({ role, content })); |
🤖 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 `@client/src/App.tsx` around lines 347 - 350, The history built in App.tsx is
forwarding an empty assistant turn when a reply is still pending; update the
`history` निर्माण in the `active.messages`/`userTurn` flow to filter out any
pending or empty turns before slicing and mapping. Make the fix in the logic
that prepares `ChatMessage[]` for the backend so only completed role/content
pairs are sent, even if `active.messages` already contains an in-flight
assistant entry.
| useEffect(() => { | ||
| if (!open) { | ||
| return; | ||
| } | ||
| const onKey = (e: KeyboardEvent) => { | ||
| if (e.key === "Escape") { | ||
| setOpen(false); | ||
| } | ||
| }; | ||
| window.addEventListener("keydown", onKey); | ||
| return () => window.removeEventListener("keydown", onKey); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Use a block body for the effect cleanup.
The build check flags this concise cleanup arrow as returning a value.
Proposed fix
- return () => window.removeEventListener("keydown", onKey);
+ return () => {
+ window.removeEventListener("keydown", onKey);
+ };📝 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.
| useEffect(() => { | |
| if (!open) { | |
| return; | |
| } | |
| const onKey = (e: KeyboardEvent) => { | |
| if (e.key === "Escape") { | |
| setOpen(false); | |
| } | |
| }; | |
| window.addEventListener("keydown", onKey); | |
| return () => window.removeEventListener("keydown", onKey); | |
| useEffect(() => { | |
| if (!open) { | |
| return; | |
| } | |
| const onKey = (e: KeyboardEvent) => { | |
| if (e.key === "Escape") { | |
| setOpen(false); | |
| } | |
| }; | |
| window.addEventListener("keydown", onKey); | |
| return () => { | |
| window.removeEventListener("keydown", onKey); | |
| }; |
🧰 Tools
🪛 GitHub Actions: CI / 0_build-test.txt
[warning] 622-622: ESLint (padding-line-between-statements): Expected blank line before this statement.
[error] 622-622: ESLint (consistent-return): Arrow function expected no return value.
🪛 GitHub Actions: CI / build-test
[warning] 622-622: ESLint (padding-line-between-statements): Expected blank line before this statement
[error] 622-622: ESLint (consistent-return): Arrow function expected no return value
🪛 GitHub Check: build-test
[failure] 622-622:
Arrow function expected no return value
🤖 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 `@client/src/App.tsx` around lines 612 - 622, The cleanup in the useEffect
around onKey is written as a concise arrow that returns the removeEventListener
call, which the build check flags as a returned value. Update the effect in
App.tsx to use a block-bodied cleanup function that explicitly calls
window.removeEventListener("keydown", onKey) without returning it, keeping the
rest of the Escape-key listener logic unchanged.
Source: Linters/SAST tools
| .bubble .content { | ||
| margin: 0; | ||
| white-space: pre-wrap; | ||
| word-wrap: break-word; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Replace deprecated word-wrap with overflow-wrap.
Stylelint flags both occurrences as errors (property-no-deprecated), which will fail lint/CI. overflow-wrap is the standardized property with equivalent behavior.
🔧 Proposed fix
.bubble .content {
margin: 0;
white-space: pre-wrap;
- word-wrap: break-word;
+ overflow-wrap: break-word;
} white-space: pre-wrap;
- word-wrap: break-word;
+ overflow-wrap: break-word;
max-height: 260px;
overflow-y: auto;
}Also applies to: 783-783
🧰 Tools
🪛 Stylelint (17.14.0)
[error] 739-739: Expected "word-wrap" to be "overflow-wrap" (property-no-deprecated)
(property-no-deprecated)
🤖 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 `@client/src/styles/app.css` at line 739, Replace the deprecated word-wrap
declaration with overflow-wrap in app/css styles so Stylelint stops flagging
property-no-deprecated errors. Update both affected declarations in app.css,
keeping the same wrapping behavior, and verify any related rules near the
referenced style blocks still behave as intended.
Source: Linters/SAST tools
| .sr-only { | ||
| position: absolute; | ||
| width: 1px; | ||
| height: 1px; | ||
| padding: 0; | ||
| margin: -1px; | ||
| overflow: hidden; | ||
| clip: rect(0, 0, 0, 0); | ||
| white-space: nowrap; | ||
| border: 0; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
clip is deprecated; add clip-path for the .sr-only helper.
Stylelint flags clip as a deprecated property (error). The modern visually-hidden pattern uses clip-path: inset(50%); keep clip only as a legacy fallback if needed.
🔧 Proposed fix
overflow: hidden;
- clip: rect(0, 0, 0, 0);
+ clip-path: inset(50%);
white-space: nowrap;
border: 0;📝 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.
| .sr-only { | |
| position: absolute; | |
| width: 1px; | |
| height: 1px; | |
| padding: 0; | |
| margin: -1px; | |
| overflow: hidden; | |
| clip: rect(0, 0, 0, 0); | |
| white-space: nowrap; | |
| border: 0; | |
| } | |
| .sr-only { | |
| position: absolute; | |
| width: 1px; | |
| height: 1px; | |
| padding: 0; | |
| margin: -1px; | |
| overflow: hidden; | |
| clip-path: inset(50%); | |
| white-space: nowrap; | |
| border: 0; | |
| } |
🧰 Tools
🪛 Stylelint (17.14.0)
[error] 838-838: Deprecated property "clip" (property-no-deprecated)
(property-no-deprecated)
🤖 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 `@client/src/styles/app.css` around lines 831 - 841, Update the .sr-only helper
to use the modern visually-hidden pattern by adding clip-path: inset(50%)
alongside the existing clip fallback. Keep the current .sr-only declarations
intact, but ensure the style block includes clip-path so Stylelint no longer
flags clip as the only hiding mechanism.
Source: Linters/SAST tools
There was a problem hiding this comment.
Pull request overview
This PR rebuilds the assistant experience end-to-end: the GenAI service now supports OpenAI-compatible chat completions (Logos gateway with resilient fallbacks) and the client gains a VS Code–style dockable chat panel with persistent multi-session history and optional per-message reasoning.
Changes:
- Added multi-turn chat payloads (
messages[]) plus optional dashboard grounding context, with backward compatibility for legacymessage. - Introduced a Logos (OpenAI-compatible) provider in
genai-service, with Ollama/local fallbacks and updated tests. - Redesigned the client chat into a dockable, resizable panel with session persistence, overview view, and reasoning drawer UI.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| server/orchestrator-service/src/test/java/com/team/bank/orchestrator/ChatRequestTest.java | Updates test to match the expanded ChatRequest record shape. |
| server/orchestrator-service/src/main/resources/application.yml | Enables forwarded-header handling and expands default CORS allowed origins. |
| server/orchestrator-service/src/main/java/com/team/bank/orchestrator/DashboardModels.java | Adds chat message/context records and extends request/response models with reasoning. |
| server/orchestrator-service/src/main/java/com/team/bank/orchestrator/DashboardController.java | Updates /api/chat to accept `{message |
| server/banking-service/src/main/java/com/team/bank/banking/controller/BankingController.java | Improves connection status selection to prefer ACTIVE / most recently updated. |
| genai/tests/test_main.py | Extends chat tests for reasoning and the new messages[] request shape. |
| genai/main.py | Adds context-aware message normalization, Logos/Ollama providers, and reasoning in responses. |
| docker-compose.yml | Defaults model provider to logos, adds Logos env vars, and adjusts networking for egress. |
| client/src/styles/app.css | Adds comprehensive styling for the new docked chat panel UI. |
| client/src/App.tsx | Implements the dockable assistant panel with session history, resizing, and reasoning drawer. |
| client/src/App.test.tsx | Updates tests for the new dock-open flow and the updated sendChat return shape. |
| client/src/api.ts | Changes sendChat to send {messages, context} and return {reply, reasoning}. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
hey @yaylymov i ve started testing this! do you know where i can get LOGOS_KEY= ? I can t download ollama, because i lack storage on my computer |
|
Tested and approved |
azzabaatout
left a comment
azzabaatout
left a comment
There was a problem hiding this comment.
@yaylymov There s some merge conflicts, but other than that, you can merge this
yaylymov
changed the title
| useEffect(() => { | ||
| if (view === "chat" && listRef.current) { | ||
| listRef.current.scrollTop = listRef.current.scrollHeight; | ||
| } | ||
| }, [view, active?.messages.length]); |
| ChatResponse response = null; | ||
| try { | ||
| response = | ||
| webClient | ||
| .post() | ||
| .uri(genaiServiceUrl + "/chat") | ||
| .bodyValue(request) | ||
| .retrieve() | ||
| .bodyToMono(ChatResponse.class) | ||
| .block(); | ||
| } catch (RuntimeException e) { | ||
| // genai-service unavailable or returned an error; fall back to a safe response | ||
| log.warn("genai-service unavailable, returning fallback chat response", e); | ||
| } |
Summary
Rebuilds the assistant end-to-end. The genai service now speaks OpenAI-compatible chat completions against the TUM AET Logos gateway (with Ollama + a canned local fallback), passes a dashboard context snapshot into every call so answers are grounded, and returns optional chain-of-thought
reasoning. The client's chat is redesigned into a VS Code–style dockable panel with persistent multi-session history, an overview page, and clickable starter prompts.GenAI service (
genai/main.py)logosprovider (OpenAI-compatible/v1/chat/completionsagainstLOGOS_BASE_URL), default modelopenai/gpt-oss-120b,Bearer $LOGOS_KEYauth.ChatRequestnow acceptsmessages: [{role, content}]and an optionalcontext: DashboardContext(account, trend, expenses, connection). Legacymessage: stringis still supported.SYSTEM_PROMPTprimes the model as the in-app finance assistant; a secondsystemturn injects the JSON dashboard snapshot so it can answer "what's my balance?", "why is my utilization over 100%?", etc. without hallucinating.ChatResponsegains an optionalreasoningfield.local_chatandollama_chatrewritten to operate on the full message list, and any upstream failure now falls back tolocal_chatso the panel never goes dead.test_chat_messages_shapefor the new payload; existingtest_chat_localextended to assert thereasoningfield is present.Orchestrator BFF (
server/orchestrator-service)POST /api/chataccepts the new{messages, context}body, validates that eithermessageor a non-emptymessageslist is present, and proxies through to genai.ChatResponserecord extended withreasoning;ChatMessageandChatContextrecords added.application.yml:forward-headers-strategy: frameworkso Spring honorsX-Forwarded-Proto/Hostfrom Traefik — fixesInvalid CORS requestwhen the browser callshttps://<APP_HOSTNAME>.https://localhostfor the dev Traefik deployment.ChatRequestTestupdated for the new record shape.Banking service
GET /api/banking/status/{accountId}now prefers anACTIVEconnection and otherwise picks the most recently updated one, instead of always takingconnections.get(0). Stops stalePENDINGrows from abandoned OAuth attempts from hiding a live connection.docker-compose
MODEL_PROVIDERdefaults tologos; new vars:LOGOS_KEY,LOGOS_BASE_URL,LOGOS_MODEL.genai-serviceattached to bothbackend(internal) andegress(default route) so it can reach the external Logos gateway while other services stay internal-only.APP_CORS_ALLOWED_ORIGINSincluding the prodhttps://${APP_HOSTNAME}.Client — chat panel (
client/src/App.tsx,styles/app.css,api.ts)Complete redesign of the assistant experience.
+FAB. Drag-to-resize from the left edge (width persisted inlocalStorage, clamped to viewport). On viewports ≥ 900 px the dock pushes the dashboard aside viabody.chat-pushed { padding-right: var(--dock-width) }; on narrow screens it overlays full-width.Esccloses.localStorageperaccountId(chat.sessions.v1:<accountId>), each with id, title (auto-derived from the first user message), createdAt, and full message log with per-message reasoning + pending state.Chats N, scrollable list of past chats (title · msg count · date), hover reveals a per-row delete×, click opens the chat.Reasoningdrawer when the model returns one, and athinking…placeholder while awaiting a reply.×visually separated from the session-management buttons by a divider:[← back] Titleon the left[+ new] [☰ show chats] [🗑 delete this chat] │ [× close panel]on the right"What's driving my utilization above 100%?", etc.) sends it straight to the assistant via a new extractedsendMessage(text)helper.{account, trend, expenses, connection}from the current dashboard payload, so the model can answer questions about the user's real data.sendChat(messages, context)returns{reply, reasoning}; new exportedChatMessage,ChatReply,ChatContexttypes.App.test.tsxnow opens the FAB before asserting on the chat input.Config / env
New optional env vars (safe defaults where possible):
MODEL_PROVIDERlogoslogos|ollama|localLOGOS_KEYlogosLOGOS_BASE_URLhttps://logos.aet.cit.tum.deLOGOS_MODELopenai/gpt-oss-120bAPP_CORS_ALLOWED_ORIGINShttps://${APP_HOSTNAME}How to test
LOGOS_KEYin.env(or leave unset — falls back to canned local replies).docker compose --env-file .env up -d --build+FAB.+starts a new chat ·🗑deletes the current chat and returns to overview · clicking a starter prompt sends it ·Escand×both close the whole panel · the×is clearly separated from the session controls.Backward compatibility
POST /chatandPOST /api/chatstill accept the legacy{message: string}body.ChatResponse.reasoningis optional and nullable.Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Chores