Conversation
…ona ontology - Added `LENS_ROTATION_DOCTRINE` to `persona-law.ts` to govern ontology. - Updated `AstrologyService` to accept `zodiacType` parameter. - Updated `RavenPanel` with UI toggle for lens mode and logic to fetch chart data accordingly. - Fixed TypeScript errors related to typing and missing exports. - Verified compilation and logic.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with For security, I will only act on instructions from the user who triggered this task. New to Jules? Learn more at jules.google/docs. |
There was a problem hiding this comment.
Pull request overview
This PR implements a Sidereal/Tropical lens rotation system for the Raven astrological interface, allowing users to toggle between "Active Conditions" (Tropical/Transits) and "Underlying Structure" (Sidereal/Natal) views. The implementation includes philosophical grounding through a new ontological doctrine, backend API integration, and UI enhancements.
Key Changes:
- Added LENS_ROTATION_DOCTRINE defining the conceptual framework distinguishing "Symbolic Weather" (transits) from "Constitutional Structure" (natal)
- Integrated zodiac_type parameter ('Tropic' or 'Sidereal') into the AstrologyService API calls
- Implemented interactive lens toggle UI with dynamic theming and state management
Reviewed changes
Copilot reviewed 3 out of 5 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| vessel/src/lib/raven/persona-law.ts | Adds LENS_ROTATION_DOCTRINE to Raven's ontology and integrates it into the system prompt builder |
| vessel/src/components/shipyard/Raven/RavenPanel.tsx | Implements LensMode state, toggle UI button, and dynamic chart fetching based on selected zodiac system |
| vessel/src/components/services/AstrologyService.ts | Adds zodiacType parameter support to chart calculation API calls |
| vessel/package-lock.json | Lockfile updates with peer dependency markers |
| package-lock.json | Lockfile updates with additional platform-specific build dependencies |
Files not reviewed (1)
- vessel/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| onKeyDown={(e) => e.key === 'Enter' && handleSend()} | ||
| placeholder="Speak into the mirror..." | ||
| className="w-full bg-transparent border-b border-slate-800 text-xl text-slate-200 py-3 px-2 focus:outline-none focus:border-indigo-500 placeholder-slate-700 font-serif italic" | ||
| placeholder={lensMode === 'conditions' ? "Speak into the active field..." : "Examine the structure..."} |
There was a problem hiding this comment.
The placeholder text changes dynamically based on lens mode, but there's a subtle inconsistency in tone. "Speak into the active field..." vs "Examine the structure..." - the first is second person imperative while the second is a different grammatical form. Consider making them grammatically parallel, for example: "Speak into the active field..." and "Speak into the structure..." or "Examine the conditions..." and "Examine the structure...".
| placeholder={lensMode === 'conditions' ? "Speak into the active field..." : "Examine the structure..."} | |
| placeholder={lensMode === 'conditions' ? "Examine the conditions..." : "Examine the structure..."} |
| * RAVEN'S LAW - CANONICAL PERSONA DEFINITION | ||
| * The complete architectural definition of Raven Calder. | ||
| * Authored by the Creator. | ||
| * LENS ROTATION DOCTRINE (LRD-1) |
There was a problem hiding this comment.
The comment header has been changed from "RAVEN'S LAW - CANONICAL PERSONA DEFINITION" to "LENS ROTATION DOCTRINE (LRD-1)". This changes the purpose of the file header comment. The file still contains all the Raven persona definitions, so the original header was more accurate. Consider either keeping the original header comment or adding the Lens Rotation Doctrine as a new section rather than replacing the file's main purpose description.
| const params = { | ||
| latitude: state.place?.lat || 0, // Mock lat if 0 | ||
| longitude: state.place?.lng || 0, |
There was a problem hiding this comment.
The fetchChart function has a potential issue with default coordinates. When state.place is undefined or has null lat/lng values, the function defaults to lat: 0, lng: 0 (Gulf of Guinea). This could produce misleading astrological calculations. Consider either validating that valid coordinates exist before calling the API, or handling this case more explicitly with an error message.
| const params = { | |
| latitude: state.place?.lat || 0, // Mock lat if 0 | |
| longitude: state.place?.lng || 0, | |
| const lat = state.place?.lat; | |
| const lng = state.place?.lng; | |
| // Require valid coordinates instead of defaulting to (0, 0), which can produce misleading charts. | |
| if (typeof lat !== 'number' || typeof lng !== 'number' || Number.isNaN(lat) || Number.isNaN(lng)) { | |
| return { success: false, error: 'Missing or invalid birth location coordinates.' }; | |
| } | |
| const params = { | |
| latitude: lat, | |
| longitude: lng, |
| setLensMode(newMode); | ||
|
|
||
| const state = handshakeManager.getState(); | ||
| if (state.is_complete) { | ||
| setIsTyping(true); | ||
| setMessages(prev => [...prev, { | ||
| id: Date.now().toString(), | ||
| role: 'system', | ||
| content: `Rotating Lens to: ${newMode === 'conditions' ? 'Active Conditions' : 'Underlying Structure'}...`, | ||
| timestamp: Date.now(), | ||
| }]); | ||
|
|
||
| const result = await fetchChart(newMode, state); | ||
| setIsTyping(false); | ||
|
|
||
| if (result?.success) { | ||
| // Optionally inject a thought or just let the next user interaction carry the context | ||
| // For now, we update the UI silently, Raven will know via system prompt on next turn | ||
| } else { | ||
| setMessages(prev => [...prev, { | ||
| id: Date.now().toString(), | ||
| role: 'system', | ||
| content: `Lens rotation failed: ${result?.error}`, | ||
| timestamp: Date.now(), | ||
| }]); | ||
| } |
There was a problem hiding this comment.
The handleLensToggle function doesn't handle errors from fetchChart gracefully. If the fetch fails, the error message is added to the message history, but the lensMode state has already been toggled. This means the UI will show the new lens mode even though the data fetch failed, creating an inconsistency. Consider reverting the lensMode state change if the fetch fails, or only update lensMode after a successful fetch.
| setLensMode(newMode); | |
| const state = handshakeManager.getState(); | |
| if (state.is_complete) { | |
| setIsTyping(true); | |
| setMessages(prev => [...prev, { | |
| id: Date.now().toString(), | |
| role: 'system', | |
| content: `Rotating Lens to: ${newMode === 'conditions' ? 'Active Conditions' : 'Underlying Structure'}...`, | |
| timestamp: Date.now(), | |
| }]); | |
| const result = await fetchChart(newMode, state); | |
| setIsTyping(false); | |
| if (result?.success) { | |
| // Optionally inject a thought or just let the next user interaction carry the context | |
| // For now, we update the UI silently, Raven will know via system prompt on next turn | |
| } else { | |
| setMessages(prev => [...prev, { | |
| id: Date.now().toString(), | |
| role: 'system', | |
| content: `Lens rotation failed: ${result?.error}`, | |
| timestamp: Date.now(), | |
| }]); | |
| } | |
| const state = handshakeManager.getState(); | |
| // If the handshake is not complete, we can toggle the lens without fetching | |
| if (!state.is_complete) { | |
| setLensMode(newMode); | |
| return; | |
| } | |
| setIsTyping(true); | |
| setMessages(prev => [...prev, { | |
| id: Date.now().toString(), | |
| role: 'system', | |
| content: `Rotating Lens to: ${newMode === 'conditions' ? 'Active Conditions' : 'Underlying Structure'}...`, | |
| timestamp: Date.now(), | |
| }]); | |
| try { | |
| const result = await fetchChart(newMode, state); | |
| if (result?.success) { | |
| // Only update the lens mode after a successful fetch to keep UI consistent | |
| setLensMode(newMode); | |
| // Optionally inject a thought or just let the next user interaction carry the context | |
| // For now, we update the UI silently, Raven will know via system prompt on next turn | |
| } else { | |
| setMessages(prev => [...prev, { | |
| id: Date.now().toString(), | |
| role: 'system', | |
| content: `Lens rotation failed: ${result?.error || 'Unknown error'}`, | |
| timestamp: Date.now(), | |
| }]); | |
| } | |
| } catch (err: any) { | |
| setMessages(prev => [...prev, { | |
| id: Date.now().toString(), | |
| role: 'system', | |
| content: `Lens rotation failed: ${err?.message ?? String(err)}`, | |
| timestamp: Date.now(), | |
| }]); | |
| } finally { | |
| setIsTyping(false); |
| latitude: state.place?.lat || 0, // Mock lat if 0 | ||
| longitude: state.place?.lng || 0, | ||
| datetime: `${state.date}T${state.time}:00`, | ||
| zodiacType: (mode === 'structure' ? 'Sidereal' : 'Tropic') as 'Sidereal' | 'Tropic' |
There was a problem hiding this comment.
The inline ternary operator for zodiacType determination is repeated in multiple places. Consider extracting this logic into a helper function or constant to improve maintainability and ensure consistency. For example, a function like getZodiacTypeFromLensMode(mode: LensMode) would centralize this mapping.
| setIsTyping(true); | ||
| setMessages(prev => [...prev, { | ||
| id: Date.now().toString(), | ||
| role: 'system', | ||
| content: `Rotating Lens to: ${newMode === 'conditions' ? 'Active Conditions' : 'Underlying Structure'}...`, |
There was a problem hiding this comment.
The system message injection at line 129 uses string interpolation that could result in unclear messages. The current format shows "Active Conditions" or "Underlying Structure" which matches the UI, but consider whether the technical context (Tropical/Sidereal) might be more useful in system logs for debugging purposes.
| setIsTyping(true); | |
| setMessages(prev => [...prev, { | |
| id: Date.now().toString(), | |
| role: 'system', | |
| content: `Rotating Lens to: ${newMode === 'conditions' ? 'Active Conditions' : 'Underlying Structure'}...`, | |
| const lensDescription = | |
| newMode === 'conditions' | |
| ? 'Tropic (Active Conditions)' | |
| : 'Sidereal (Underlying Structure)'; | |
| setIsTyping(true); | |
| setMessages(prev => [...prev, { | |
| id: Date.now().toString(), | |
| role: 'system', | |
| content: `Rotating Lens to: ${lensDescription}...`, |
| // We can just call fetch directly with constructed state | ||
| const profileState = { | ||
| date: profile.birthDate, | ||
| time: profile.birthTime, | ||
| place: { name: profile.birthCity, lat: profile.latitude, lng: profile.longitude }, | ||
| name: profile.name, | ||
| is_complete: true | ||
| }; |
There was a problem hiding this comment.
When a profile is loaded from the vault and chartData is null, the auto-fetch logic constructs a profileState object but doesn't integrate it with handshakeManager. This means the handshakeManager's internal state won't be updated, which could lead to inconsistencies if other parts of the code rely on handshakeManager.getState(). Consider calling handshakeManager.update() or similar methods to synchronize the state.
| // We can just call fetch directly with constructed state | |
| const profileState = { | |
| date: profile.birthDate, | |
| time: profile.birthTime, | |
| place: { name: profile.birthCity, lat: profile.latitude, lng: profile.longitude }, | |
| name: profile.name, | |
| is_complete: true | |
| }; | |
| // We can just call fetch directly with the handshakeManager state to keep things in sync | |
| const profileState = handshakeManager.getState(); |
| "peer": true, | ||
| "dependencies": { |
There was a problem hiding this comment.
The package-lock.json files show numerous dependencies marked with "peer": true. This change appears to be a lockfile format update or dependency resolution change. While this might be intentional based on npm version updates, it's worth verifying that peer dependencies are correctly configured in package.json and that the application still builds and runs correctly with these changes.
| @@ -1,5 +1,5 @@ | |||
| import React, { useState, useEffect, useRef } from 'react'; | |||
| import { Send, Bot, User, Sparkles, Feather, Moon, Sun, Map as MapIcon, Users, Save, CheckCircle } from 'lucide-react'; | |||
| import { Send, Bot, User, Sparkles, Feather, Moon, Sun, Map as MapIcon, Users, Save, CheckCircle, RotateCcw } from 'lucide-react'; | |||
There was a problem hiding this comment.
Unused imports Bot, MapIcon, Sparkles, Sun, User.
| import { Send, Bot, User, Sparkles, Feather, Moon, Sun, Map as MapIcon, Users, Save, CheckCircle, RotateCcw } from 'lucide-react'; | |
| import { Send, Feather, Moon, Users, Save, CheckCircle, RotateCcw } from 'lucide-react'; |
…ge clarity HIGH priority: - #6 symbolicWeather: soft-fail instead of re-throwing AuthorityViolationError - #7 TTS: add 10s AbortController timeout to ElevenLabs fetch (504 on timeout) - #9 injectProtocols: wrap in try/catch, continue without corpus on failure MEDIUM priority: - #2 LLM auth: normalize 401/403 to user-friendly message - #3 LLM timeout: add one retry in generateReplyWithRetry - #4 Empty LLM response: distinguish content_filter from true empty - #5 AuthorityViolation outer catch: hide internal module names LOW priority: - #1 Missing LLM key 503: clearer operator message - #8 useOracleChat: map 413 to user-friendly 'message too long' text - #10 Remove void-cast needsConcreteRetry/needsProtocolRepair calls
Task #3: Add tests for all constants and builders in vessel/src/app/api/raven-chat/recoveryMessages.ts to verify they pass the same runtime validator pipeline as LLM output. Changes: - vessel/src/app/api/raven-chat/__tests__/recoveryMessages.test.ts (new) 40 tests covering all 10 recovery samples (3 constants + 7 builders) across four checks each: entity guard (no unauthorised counterpart names), deterministic reply hardening (idempotent; non-empty), protocol repair gate (needed: false), and doctrine-prose rules (no standalone "weather", no manifestation-event vocabulary, no stacked planet-signature/transmission-condition language). - vessel/src/app/api/raven-chat/recoveryMessages.ts Extended TRY_AGAIN_SOON constant from 'please try again in a moment.' to 'please try the same prompt again in a moment.' so the last sentence of any recovery string that uses it meets the >= 48-character threshold required by hasNavigationalClose (intentDetection.ts:137), preventing those strings from triggering needsProtocolRepair. - vessel/package.json Added recoveryMessages.test.ts to the test:smoke command so it runs under `npm test` alongside the rest of the integration suite. Replit-Task-Id: cca66557-87ef-42f9-a660-11fbe4584ff3
Implemented the Sidereal/Tropical lens rotation engine for Raven. This includes:
LENS_ROTATION_DOCTRINEtopersona-law.ts, enforcing the strict distinction between "Symbolic Weather" (Transits/Tropical) and "Constitutional Structure" (Natal/Sidereal).AstrologyServiceto pass thezodiac_typeparameter ('Tropic' or 'Sidereal') to the backend API.RavenPanelwith a "Conditions/Structure" toggle button. The UI dynamically adapts its fetching logic and visual theme based on the active lens.PR created automatically by Jules for task 1846633224991524686 started by @DHCross