fix: pinned chat panel layout, CommonMark parsing, and CSS animation

[WilliamAGH]

Summary

Improves the Learn view chat panel UX with a pinned frame layout, fixes CommonMark-compliant ordered list parsing, and resolves CSS animation issues.

Changes by Category

UI/UX Improvements

• Pinned chat panel frame: Chat panel now stays fixed in viewport with independent scrolling
  • Wider panel (460px on large screens, 400px on medium)
  • Header pinned at top, input pinned at bottom
  • Subtle accent gradient indicator at top edge
  • Left border shadow for visual separation
• Responsive breakpoints: Added intermediate 1025-1280px breakpoint for panel sizing

Bug Fixes

• CSS animation mismatch (ThinkingIndicator): Renamed generate-glow keyframes to sparkle-glow to match .icon-sparkle class reference
• Null safety (ExceptionResponseBuilder): Added null checks for responseBody and headers before calling methods on them
• Citation error handling (LearnView): Added missing .catch() handler on citation fetch promise

Spec Compliance

• CommonMark whitespace requirement (OrderedMarkerScanner): startsWithOrderedMarker() now rejects markers without whitespace after delimiter (e.g., "1.Foo" rejected, "1. Foo" accepted)

Technical Details

• Frontend: Svelte 5, CSS Grid/Flexbox layout with min-height: 0 for proper overflow scrolling
• Backend: Java 21, Spring Boot

Files Changed

File	Changes
LearnView.svelte	+58/-5 - Pinned chat panel layout
ThinkingIndicator.svelte	+2/-2 - Animation keyframe rename
OrderedMarkerScanner.java	+29/-3 - CommonMark whitespace enforcement
ExceptionResponseBuilder.java	+4/-3 - Null safety checks

[coderabbitai]

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

📝 Walkthrough

Summary by CodeRabbit

• Bug Fixes

  • Improved error handling for lesson sources with readable messages and guard checks
  • Fixed ordered list marker validation to follow CommonMark formatting
  • Strengthened null-safety in error response assembly
  • Corrected chat history ordering in guided learning to avoid duplicating the latest user message

• New Features

  • Session validation endpoint to verify stored chat sessions
  • Enhanced chat prompt handling to include assistant context

• Style

  • Refined chat panel visuals, pinned header/input, and scrolling behavior
  • Adjusted column widths and responsive layout
  • Tweaked thinking indicator animation

• Tests

  • Added comprehensive chat memory unit tests

_{✏️ Tip: You can customize this high-level summary in your review settings.}

Walkthrough

Adds frontend layout and scrolling fixes, error handling for lesson citations, a renamed animation, CommonMark-accurate ordered-list detection, stricter null checks in exception builders, multi-turn prompt inclusion, a session-validation endpoint and response record, and tests for ChatMemory behavior.

Changes

Cohort / File(s)	Summary
Frontend: Learn view & chat UI frontend/src/lib/components/LearnView.svelte	Added try/catch for lesson citations fetch with stale-response guard and error messaging; adjusted layout sizes (right column 400→460px), removed right border, added min-height: 0/overflow/flex fixes, pinned full-height chat panel, decorative top line, pinned input styling, smoother scrolling, and 1280px breakpoint refinements.
Frontend: Indicator animation frontend/src/lib/components/ThinkingIndicator.svelte	Renamed CSS animation and @keyframes from generate-glow to sparkle-glow; no logic changes.
Backend: Markdown parsing src/main/java/.../OrderedMarkerScanner.java	Tightened startsWithOrderedMarker to require CommonMark whitespace after . or ) delimiters (rejects 1.Foo, accepts 1. Foo or 1. at EOL).
Backend: Exception detail null-safety src/main/java/.../ExceptionResponseBuilder.java	Added null/emptiness guards before appending responseBody and headers; caches headers into a local variable prior to checks.
Backend: Prompt building (multi-turn) src/main/java/.../ChatService.java	buildPromptFromMessages now includes AssistantMessage text prefixed with Assistant: ; uses pattern matching for message types.
Backend: Session validation endpoint & response src/main/java/.../ChatController.java, src/main/java/.../SessionValidationResponse.java	New GET /api/chat/session/validate endpoint that returns SessionValidationResponse(sessionId, turnCount, exists, message); returns 400 for missing sessionId. Added public record for payload.
Backend: Guided learning history ordering src/main/java/.../GuidedLearningController.java	Reordered chat-memory ops to load history before adding the current user message so the prompt builder receives history excluding the latest user message.
Tests: Chat memory src/test/java/.../ChatMemoryServiceTest.java	New test suite covering storing/retrieving user & assistant messages, interleaving/order, session isolation, clearing, immutable snapshots, and turn-tracking.

Sequence Diagram(s)

sequenceDiagram
participant Client as Client
participant Controller as GuidedLearningController
participant Memory as ChatMemory
participant Builder as PromptBuilder
participant OpenAI as OpenAIStream
Client->>Controller: POST /guided-learning (user message)
Controller->>Memory: loadHistory(sessionId)
Controller->>Builder: buildGuidedPromptWithContext(history, latestUserMessage)
Controller->>Memory: addUserMessage(sessionId, latestUserMessage)
Controller->>OpenAI: start streaming with prompt
OpenAI-->>Controller: stream tokens/results
Controller-->>Client: stream SSE/chunks to client
Controller->>Memory: addAssistantMessage(sessionId, assistantFinal)

Loading

Estimated Code Review Effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

✨ Panels stretch, the chat stands tall and neat,
Markers mind their spaces, tidy and sweet,
Null checks hum softly to keep things sound,
Assistant lines join the thread all around,
Tests guard the turns so memory stays complete.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 35.29% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description check	✅ Passed	The pull request description clearly relates to the changeset, covering UI/UX improvements, bug fixes, and spec compliance changes across multiple files.
Title Check	✅ Passed	Title check skipped as CodeRabbit has written the PR title.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch dev

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Pull request overview

This PR tightens error/edge-case handling across backend and frontend, and refines markdown ordered-marker detection and UI behavior.

Changes:

• Add null-safety guards when building exception details for WebClient/OpenAI errors.
• Refine ordered-list marker detection to enforce CommonMark-style whitespace after delimiters.
• Adjust frontend UI/UX: lesson citations promise rejection handling, chat panel layout/styling, and a keyframes rename for the thinking indicator.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
src/main/java/com/williamcallahan/javachat/web/ExceptionResponseBuilder.java	Prevent potential NPEs when formatting exception response bodies/headers.
src/main/java/com/williamcallahan/javachat/service/markdown/OrderedMarkerScanner.java	Update ordered-marker detection behavior/documentation toward CommonMark compliance.
frontend/src/lib/components/ThinkingIndicator.svelte	Rename sparkle glow keyframes and update the animation reference accordingly.
frontend/src/lib/components/LearnView.svelte	Add citations .catch handling and update chat/lesson layout CSS for pinned scrolling behavior and responsiveness.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In
`@src/main/java/com/williamcallahan/javachat/service/markdown/OrderedMarkerScanner.java`:
- Around line 73-80: In OrderedMarkerScanner, rename the loop index `i` to a
descriptive name (e.g., `cursor`) in the for loop inside the method that
iterates trimmedLine so replace uses of `i` with `cursor` (update the for
header, the char retrieval `trimmedLine.charAt(...)`, and `posAfterDelimiter =
...`) and ensure any other references in the method are updated accordingly to
avoid the banned generic identifier.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@src/main/java/com/williamcallahan/javachat/web/ChatController.java`:
- Around line 285-305: The null sessionId passed into the bad-request
SessionValidationResponse should be replaced with a safe placeholder (e.g. empty
string) to avoid returning null in JSON; update the validateSession method so
the ResponseEntity.badRequest() builds new SessionValidationResponse with "" (or
a chosen placeholder) instead of sessionId while keeping the rest of the fields
the same; ensure this change references the validateSession(...) endpoint and
the SessionValidationResponse constructor so the response object accepts the
non-null placeholder.

🧹 Nitpick comments (1)

src/main/java/com/williamcallahan/javachat/web/SessionValidationResponse.java (1)

12-17: Clean record design! 📝

This is a well-structured DTO with clear, domain-specific field names. The Javadoc nicely explains the purpose of each field.

One small thought: per coding guidelines, domain value types benefit from constructor validation. You could add a compact constructor to guard against null sessionId:

💡 Optional: Add constructor validation

 public record SessionValidationResponse(
String sessionId,
int turnCount,
boolean exists,
String message) {
+ public SessionValidationResponse {+ if (sessionId == null) {+ throw new IllegalArgumentException("sessionId cannot be null");+ }+ }
}

That said, since the controller validates sessionId before constructing this response, this is a nice-to-have rather than essential.