docs: move pending tasks to GitHub Issues #8-#25#41
Conversation
Replace per-task entries in docs/todo.md with a pointer and an issue index. The 18 issues (10 implementation, 2 observability, 6 testing) now live on GitHub as the source of truth; Linear mirrors separately. The Canvas2D renderer milestone (#8) is the active milestone. https://claude.ai/code/session_01SbiXfM7JiKYJrG29pg9t6E
📝 WalkthroughWalkthroughThis PR establishes a holographic/glass design system for the renderer by introducing a centralized theme module with comprehensive color palettes and animation constants, creating a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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 |
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (4 files)
Reviewed by grok-code-fast-1:optimized:free · 210,767 tokens |
Lift-and-shift from agent-flow: - theme.ts: holographic COLORS palette, ROLE_COLORS, TIMING/ANIM/FX/BEAM animation constants, getStateColor and withAlpha helpers. - components/GlassCard.tsx: backdrop-filter blur card with mount/unmount fade+scale animation keyed to TIMING.glassAnimMs (200ms). - styles.css: rewrite CSS vars to void/cyan holographic palette; add .glass-card rules (blur(20px), cyan border, top highlight ::before); restyle topbar/panels/pills/graph-node in cyan on deep-space bg. No App.tsx changes yet — existing SVG graph continues to render in the new palette. Canvas2D layer replacement lands in Step 3. https://claude.ai/code/session_01SbiXfM7JiKYJrG29pg9t6E
|
@coderabbitai review please |
|
✅ Actions performedReview triggered.
|
|
@jules review please |
|
@openai-code-agent review please |
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/todo.md (1)
1-40:⚠️ Potential issue | 🟡 MinorInconsistency between PR description and actual commits.
The PR description states "No code changes in this PR," but the commit summary shows that commit 2 includes code changes:
feat(renderer): port holographic palette and glass card (Steps 1-2)- Adds
theme.ts,GlassCard.tsx, andstyles.csswith palette, components, and styling rulesThe AI summary also confirms code changes are present. This discrepancy should be resolved—either the PR description should be updated to reflect the actual scope, or the renderer changes should be moved to a separate PR if the intent was truly to only migrate task tracking in this PR.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/todo.md` around lines 1 - 40, The PR description claims "No code changes in this PR" but commits include renderer work (commit message feat(renderer): port holographic palette and glass card (Steps 1-2)) and added files theme.ts, GlassCard.tsx, and styles.css; fix by either (A) update the PR description and summary to accurately list these renderer changes and adjust the PR title/description to match the commits (include the commit summary and filenames theme.ts, GlassCard.tsx, styles.css), or (B) if the renderer work should be in a separate PR, split the commits: interactively rebase to move the renderer commits into a new branch/PR or revert those changes from this branch, then update commit messages and the PR description accordingly so the PR and commit history are consistent; ensure the final PR description and commit titles reference the correct scope.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@shadow-agent/src/renderer/components/GlassCard.tsx`:
- Around line 15-24: The requestAnimationFrame scheduled in the useEffect when
visible becomes true isn't canceled, so if visible flips true→false before the
rAF runs it re-enables animating; fix by capturing the rAF id (returned by
requestAnimationFrame) and clearing it in the effect cleanup (and when switching
to the exit branch) before calling setAnimating(false)/starting the unmount
timer; update the useEffect that references visible, setMounted, setAnimating
and TIMING.glassAnimMs to store the raf handle, cancel it with
cancelAnimationFrame in every cleanup/transition path, and ensure you still
clear the unmount timeout there as well.
- Around line 33-38: The inline style in the GlassCard component hardcodes
"0.2s" for transitions which duplicates TIMING.glassAnimMs and can desync;
update the transition to derive its duration from TIMING.glassAnimMs (convert ms
to seconds) instead of the literal "0.2s", or alternatively move the transition
rule into the .glass-card CSS class so there is a single source of truth; adjust
the style block that sets opacity/transform/transition and references animating
so it uses the computed duration from TIMING.glassAnimMs (or remove transition
from the inline style if you consolidate it in styles.css).
In `@shadow-agent/src/renderer/styles.css`:
- Around line 74-81: The body styles removed the citadel dot-grid and all 13
tiered `@keyframes`; restore or reference the spring-damped dot-grid background,
pulse API (burst/ripple) and the full set of 13 cascade-timed `@keyframes` from
the third_party/citadel reference rather than replacing them with static radial
gradients—reintroduce the dot-grid styles (spring-damped variables), add the 13
`@keyframes` with tier timing, ensure GlassCard uses those CSS
keyframes/transitions instead of inline JS, and if you cannot fully port the
Canvas2D layer now, add a clear TODO/comment and create a follow-up issue
linking third_party/citadel so the pattern set (dot-grid + pulse API +
keyframes) is tracked for Step 3.
- Around line 53-58: CSS and TS token sets are out of sync: theme.ts exposes
holoBorder06, holoBorder08, holoBorder10, holoBorder12 while styles.css defines
--holo-border-08, --holo-border-12, --holo-border-15; reconcile them by picking
one canonical set (prefer matching theme.ts) and mirroring it across both files
— add CSS variables --holo-border-06 and --holo-border-10 (or remove/replace
theme tokens if you prefer the CSS set) and ensure the RGBA values exactly match
the corresponding exports in theme.ts (holoBorder06, holoBorder08, holoBorder10,
holoBorder12) so JS↔CSS consumers resolve the same tokens.
- Around line 3-11: Update the CSS to satisfy stylelint and ensure the custom
fonts are actually loaded: remove the quotes around the Geist family name in the
font-family declaration (change 'Geist' to Geist) and normalize the
text-rendering value to the lowercase keyword optimizelegibility; then either
add proper font loading for "Geist" and "Geist Mono" (via `@font-face/`@import or
adding the <link> to index.html / package dependency) or remove those font names
from the font-family stacks so the UI doesn't silently fall back to
ui-sans-serif. Use the font-family identifier (font-family: ... Geist ...) and
the text-rendering identifier (text-rendering: optimizelegibility) to locate the
changes.
In `@shadow-agent/src/renderer/theme.ts`:
- Around line 218-220: withAlpha currently assumes rgbaBase is a partial string
like "rgba(r,g,b," and will silently produce invalid CSS for normal color
inputs; update withAlpha to either (A) enforce/validate the expected shape by
introducing a nominal/branded type (RgbaBase) and throw a clear error if
rgbaBase does not match the "rgba(...," pattern, or (B) broaden the API to
accept numeric components (r:number, g:number, b:number, a:number) or normal
color strings by parsing/converting them to a valid rgba string before appending
alpha. Reference the withAlpha function and add either the RgbaBase brand type
or the new numeric signature, plus input validation and an explicit error
message when validation fails.
- Around line 22-26: The COLORS object contains two keys in snake_case
(tool_calling, waiting_permission) that should be renamed to camelCase to match
the rest of the theme; update the COLORS map to use toolCalling and
waitingPermission and then search for and update all usages (imports, property
accesses, and any mapping to CSS variables like --state-tool / --state-waiting)
to reference the new property names (e.g., replace references to
COLORS.tool_calling and COLORS.waiting_permission with COLORS.toolCalling and
COLORS.waitingPermission) to keep naming consistent and avoid breaking
consumers.
---
Outside diff comments:
In `@docs/todo.md`:
- Around line 1-40: The PR description claims "No code changes in this PR" but
commits include renderer work (commit message feat(renderer): port holographic
palette and glass card (Steps 1-2)) and added files theme.ts, GlassCard.tsx, and
styles.css; fix by either (A) update the PR description and summary to
accurately list these renderer changes and adjust the PR title/description to
match the commits (include the commit summary and filenames theme.ts,
GlassCard.tsx, styles.css), or (B) if the renderer work should be in a separate
PR, split the commits: interactively rebase to move the renderer commits into a
new branch/PR or revert those changes from this branch, then update commit
messages and the PR description accordingly so the PR and commit history are
consistent; ensure the final PR description and commit titles reference the
correct scope.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ce5b778d-846b-4d2f-9141-90b9bb552041
📒 Files selected for processing (4)
docs/todo.mdshadow-agent/src/renderer/components/GlassCard.tsxshadow-agent/src/renderer/styles.cssshadow-agent/src/renderer/theme.ts
| useEffect(() => { | ||
| if (visible) { | ||
| setMounted(true); | ||
| requestAnimationFrame(() => setAnimating(true)); | ||
| return; | ||
| } | ||
| setAnimating(false); | ||
| const timer = setTimeout(() => setMounted(false), TIMING.glassAnimMs); | ||
| return () => clearTimeout(timer); | ||
| }, [visible]); |
There was a problem hiding this comment.
Race: pending requestAnimationFrame is never cancelled.
If visible flips true → false before the rAF callback fires, the queued callback still runs and sets animating back to true after the exit branch already set it false. The card then stays mounted/visible until the next visible change (the unmount timeout still fires, but the card is fully opaque during that window). Capture and cancel the handle in cleanup.
Proposed fix
useEffect(() => {
if (visible) {
setMounted(true);
- requestAnimationFrame(() => setAnimating(true));
- return;
+ const raf = requestAnimationFrame(() => setAnimating(true));
+ return () => cancelAnimationFrame(raf);
}
setAnimating(false);
const timer = setTimeout(() => setMounted(false), TIMING.glassAnimMs);
return () => clearTimeout(timer);
}, [visible]);📝 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 (visible) { | |
| setMounted(true); | |
| requestAnimationFrame(() => setAnimating(true)); | |
| return; | |
| } | |
| setAnimating(false); | |
| const timer = setTimeout(() => setMounted(false), TIMING.glassAnimMs); | |
| return () => clearTimeout(timer); | |
| }, [visible]); | |
| useEffect(() => { | |
| if (visible) { | |
| setMounted(true); | |
| const raf = requestAnimationFrame(() => setAnimating(true)); | |
| return () => cancelAnimationFrame(raf); | |
| } | |
| setAnimating(false); | |
| const timer = setTimeout(() => setMounted(false), TIMING.glassAnimMs); | |
| return () => clearTimeout(timer); | |
| }, [visible]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@shadow-agent/src/renderer/components/GlassCard.tsx` around lines 15 - 24, The
requestAnimationFrame scheduled in the useEffect when visible becomes true isn't
canceled, so if visible flips true→false before the rAF runs it re-enables
animating; fix by capturing the rAF id (returned by requestAnimationFrame) and
clearing it in the effect cleanup (and when switching to the exit branch) before
calling setAnimating(false)/starting the unmount timer; update the useEffect
that references visible, setMounted, setAnimating and TIMING.glassAnimMs to
store the raf handle, cancel it with cancelAnimationFrame in every
cleanup/transition path, and ensure you still clear the unmount timeout there as
well.
| style={{ | ||
| ...style, | ||
| opacity: animating ? 1 : 0, | ||
| transform: animating ? 'scale(1)' : 'scale(0.95)', | ||
| transition: 'opacity 0.2s ease-out, transform 0.2s ease-out' | ||
| }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Hardcoded 0.2s duplicates TIMING.glassAnimMs and will silently desync.
The unmount delay is driven by TIMING.glassAnimMs (200ms), but the actual CSS transition durations are two separate string literals (0.2s). Bumping glassAnimMs will break the visual fade because the CSS keeps running for the old duration. Derive the transition from the constant (or move it into .glass-card in styles.css so there's a single source).
Proposed fix
- style={{
- ...style,
- opacity: animating ? 1 : 0,
- transform: animating ? 'scale(1)' : 'scale(0.95)',
- transition: 'opacity 0.2s ease-out, transform 0.2s ease-out'
- }}
+ style={{
+ ...style,
+ opacity: animating ? 1 : 0,
+ transform: animating ? 'scale(1)' : 'scale(0.95)',
+ transition: `opacity ${TIMING.glassAnimMs}ms ease-out, transform ${TIMING.glassAnimMs}ms ease-out`
+ }}📝 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.
| style={{ | |
| ...style, | |
| opacity: animating ? 1 : 0, | |
| transform: animating ? 'scale(1)' : 'scale(0.95)', | |
| transition: 'opacity 0.2s ease-out, transform 0.2s ease-out' | |
| }} | |
| style={{ | |
| ...style, | |
| opacity: animating ? 1 : 0, | |
| transform: animating ? 'scale(1)' : 'scale(0.95)', | |
| transition: `opacity ${TIMING.glassAnimMs}ms ease-out, transform ${TIMING.glassAnimMs}ms ease-out` | |
| }} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@shadow-agent/src/renderer/components/GlassCard.tsx` around lines 33 - 38, The
inline style in the GlassCard component hardcodes "0.2s" for transitions which
duplicates TIMING.glassAnimMs and can desync; update the transition to derive
its duration from TIMING.glassAnimMs (convert ms to seconds) instead of the
literal "0.2s", or alternatively move the transition rule into the .glass-card
CSS class so there is a single source of truth; adjust the style block that sets
opacity/transform/transition and references animating so it uses the computed
duration from TIMING.glassAnimMs (or remove transition from the inline style if
you consolidate it in styles.css).
| font-family: 'Geist', ui-sans-serif, system-ui, -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, sans-serif; | ||
| line-height: 1.5; | ||
| font-weight: 400; | ||
| color: #e4edf7; | ||
| background: | ||
| radial-gradient(circle at top left, rgba(67, 111, 143, 0.28), transparent 32%), | ||
| radial-gradient(circle at top right, rgba(99, 71, 140, 0.28), transparent 34%), | ||
| linear-gradient(180deg, #07111d 0%, #0b1626 55%, #09111d 100%); | ||
| color: #aaeeff; | ||
| background: #050510; | ||
| font-synthesis: none; | ||
| text-rendering: optimizeLegibility; | ||
| -webkit-font-smoothing: antialiased; | ||
| -moz-osx-font-smoothing: grayscale; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify Geist / Geist Mono are loaded somewhere in the renderer.
fd -e html -e css -e ts -e tsx . shadow-agent | xargs rg -nP '(`@font-face`|@import|fontsource|Geist)' -C2Repository: Coldaine/AgentVisualCrazy
Length of output: 827
🏁 Script executed:
# Search entire repo for `@font-face` and `@import` statements
rg -nP '(`@font-face`|@import)' --type css
# Find index.html files
fd -name 'index.html'
# Check package.json for font-related dependencies
fd -name 'package.json' | head -5 | xargs catRepository: Coldaine/AgentVisualCrazy
Length of output: 531
🏁 Script executed:
# Look at the reference implementation mentioned in coding guidelines
fd -path '*third_party/citadel*' -type f | head -20Repository: Coldaine/AgentVisualCrazy
Length of output: 238
🏁 Script executed:
# Check for font files or font-related npm packages
rg -i 'fontsource|@geist-ui|geist' package.json package-lock.jsonRepository: Coldaine/AgentVisualCrazy
Length of output: 180
🏁 Script executed:
# List root directory
ls -la | head -20Repository: Coldaine/AgentVisualCrazy
Length of output: 1234
🏁 Script executed:
# Find all package.json files
fd 'package.json'Repository: Coldaine/AgentVisualCrazy
Length of output: 95
🏁 Script executed:
# Search for `@font-face` and `@import` across entire repo with correct rg syntax
rg '@font-face|@import' -A 2Repository: Coldaine/AgentVisualCrazy
Length of output: 51
🏁 Script executed:
# Find index.html files
fd 'index.html'Repository: Coldaine/AgentVisualCrazy
Length of output: 93
🏁 Script executed:
cat shadow-agent/package.jsonRepository: Coldaine/AgentVisualCrazy
Length of output: 895
🏁 Script executed:
cat shadow-agent/index.htmlRepository: Coldaine/AgentVisualCrazy
Length of output: 377
🏁 Script executed:
# Look for third_party/citadel reference implementation
fd -path '*third_party*' -type d | head -10Repository: Coldaine/AgentVisualCrazy
Length of output: 238
🏁 Script executed:
cat .stylelintrc.jsonRepository: Coldaine/AgentVisualCrazy
Length of output: 1429
🏁 Script executed:
# List third_party directory without type flag
fd 'citadel' --max-depth 3Repository: Coldaine/AgentVisualCrazy
Length of output: 110
🏁 Script executed:
# List entire third_party directory
ls -la third_party/ 2>/dev/null || echo "No third_party at root"Repository: Coldaine/AgentVisualCrazy
Length of output: 239
🏁 Script executed:
# Check the citadel reference
cat docs/research/visual-patterns-citadel.md | head -50Repository: Coldaine/AgentVisualCrazy
Length of output: 1644
🏁 Script executed:
# Search for any stylelint configuration or build output that might show actual violations
rg 'stylelint|font-family-name-quotes|value-keyword-case|optimizeL' --type-list allRepository: Coldaine/AgentVisualCrazy
Length of output: 459
🏁 Script executed:
# Check what the default stylelint-config-standard-scss rules are
# by looking at what's imported
cat shadow-agent/src/renderer/styles.css | head -20Repository: Coldaine/AgentVisualCrazy
Length of output: 597
🏁 Script executed:
# Verify exact stylelint rule names from standard config
rg 'font-family' --type mdRepository: Coldaine/AgentVisualCrazy
Length of output: 164
🏁 Script executed:
# Search for any documentation or comments about the Geist font choice
rg -i 'geist|font' --type md | head -20Repository: Coldaine/AgentVisualCrazy
Length of output: 737
🏁 Script executed:
# Check if there are any other CSS files that might import fonts
fd -name '*.css' -o -name '*.scss' | xargs rg '@import|@font-face' 2>/dev/null || echo "No font imports found"Repository: Coldaine/AgentVisualCrazy
Length of output: 340
Fix two stylelint violations and add font loading.
Lines 3 and 9 have stylelint violations enforced by your config:
- Line 3:
'Geist'should be unquoted perfont-family-name-quotesrule - Line 9:
optimizeLegibilityshould be lowercaseoptimizelegibilitypervalue-keyword-caserule
Additionally, Geist (and Geist Mono on line 162) are referenced but not loaded anywhere—no @font-face, no @import, no font package in package.json, and no font link in index.html. The UI will silently fall back to ui-sans-serif. Either load the font or remove it from the stack.
Proposed stylelint fix
- font-family: 'Geist', ui-sans-serif, system-ui, -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, sans-serif;
+ font-family: Geist, ui-sans-serif, system-ui, -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, sans-serif;
line-height: 1.5;
font-weight: 400;
color: `#aaeeff`;
background: `#050510`;
font-synthesis: none;
- text-rendering: optimizeLegibility;
+ text-rendering: optimizelegibility;📝 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.
| font-family: 'Geist', ui-sans-serif, system-ui, -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, sans-serif; | |
| line-height: 1.5; | |
| font-weight: 400; | |
| color: #e4edf7; | |
| background: | |
| radial-gradient(circle at top left, rgba(67, 111, 143, 0.28), transparent 32%), | |
| radial-gradient(circle at top right, rgba(99, 71, 140, 0.28), transparent 34%), | |
| linear-gradient(180deg, #07111d 0%, #0b1626 55%, #09111d 100%); | |
| color: #aaeeff; | |
| background: #050510; | |
| font-synthesis: none; | |
| text-rendering: optimizeLegibility; | |
| -webkit-font-smoothing: antialiased; | |
| -moz-osx-font-smoothing: grayscale; | |
| font-family: Geist, ui-sans-serif, system-ui, -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, sans-serif; | |
| line-height: 1.5; | |
| font-weight: 400; | |
| color: `#aaeeff`; | |
| background: `#050510`; | |
| font-synthesis: none; | |
| text-rendering: optimizelegibility; | |
| -webkit-font-smoothing: antialiased; | |
| -moz-osx-font-smoothing: grayscale; |
🧰 Tools
🪛 Stylelint (17.9.0)
[error] 3-3: Expected no quotes around "Geist" (font-family-name-quotes)
(font-family-name-quotes)
[error] 9-9: Expected "optimizeLegibility" to be "optimizelegibility" (value-keyword-case)
(value-keyword-case)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@shadow-agent/src/renderer/styles.css` around lines 3 - 11, Update the CSS to
satisfy stylelint and ensure the custom fonts are actually loaded: remove the
quotes around the Geist family name in the font-family declaration (change
'Geist' to Geist) and normalize the text-rendering value to the lowercase
keyword optimizelegibility; then either add proper font loading for "Geist" and
"Geist Mono" (via `@font-face/`@import or adding the <link> to index.html /
package dependency) or remove those font names from the font-family stacks so
the UI doesn't silently fall back to ui-sans-serif. Use the font-family
identifier (font-family: ... Geist ...) and the text-rendering identifier
(text-rendering: optimizelegibility) to locate the changes.
| --holo-bg-03: rgba(100, 200, 255, 0.03); | ||
| --holo-bg-05: rgba(100, 200, 255, 0.05); | ||
| --holo-bg-10: rgba(100, 200, 255, 0.1); | ||
| --holo-border-08: rgba(100, 200, 255, 0.08); | ||
| --holo-border-12: rgba(100, 200, 255, 0.12); | ||
| --holo-border-15: rgba(100, 200, 255, 0.15); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Holo border tokens diverge between theme.ts and styles.css.
theme.ts exports holoBorder06, holoBorder08, holoBorder10, holoBorder12 (no 15), while this file defines --holo-border-08, --holo-border-12, --holo-border-15 (no 06/10). Same color system, two sets of tokens — consumers crossing the JS↔CSS boundary will hit missing values. Pick one set and mirror it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@shadow-agent/src/renderer/styles.css` around lines 53 - 58, CSS and TS token
sets are out of sync: theme.ts exposes holoBorder06, holoBorder08, holoBorder10,
holoBorder12 while styles.css defines --holo-border-08, --holo-border-12,
--holo-border-15; reconcile them by picking one canonical set (prefer matching
theme.ts) and mirroring it across both files — add CSS variables
--holo-border-06 and --holo-border-10 (or remove/replace theme tokens if you
prefer the CSS set) and ensure the RGBA values exactly match the corresponding
exports in theme.ts (holoBorder06, holoBorder08, holoBorder10, holoBorder12) so
JS↔CSS consumers resolve the same tokens.
| body { | ||
| min-height: 100vh; | ||
| overflow: hidden; | ||
| background: | ||
| radial-gradient(circle at 20% 15%, rgba(100, 200, 255, 0.06), transparent 40%), | ||
| radial-gradient(circle at 80% 85%, rgba(204, 136, 255, 0.05), transparent 45%), | ||
| var(--void); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Citadel reference patterns (dot-grid + 13 @keyframes) not yet present.
Per the renderer CSS guideline, this file should incorporate spring-damped dot-grid backgrounds, the pulse API (burst/ripple), and the 13 @keyframes with tier cascade timing from third_party/citadel. The current overhaul replaces the body with static radial gradients and adds no @keyframes at all (the GlassCard transition is inlined in JS). The PR commit notes Steps 1–2 with Canvas2D layer landing in Step 3 — please ensure the citadel patterns are tracked for a follow-up step so they aren't dropped.
As per coding guidelines: "Reference implementation patterns from third_party/citadel for spring-damped dot-grid backgrounds, pulse API (burst/ripple), and 13 CSS @keyframes with tier cascade timing."
Want me to open a follow-up issue to track porting the citadel dot-grid + keyframe set?
Also applies to: 124-201
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@shadow-agent/src/renderer/styles.css` around lines 74 - 81, The body styles
removed the citadel dot-grid and all 13 tiered `@keyframes`; restore or reference
the spring-damped dot-grid background, pulse API (burst/ripple) and the full set
of 13 cascade-timed `@keyframes` from the third_party/citadel reference rather
than replacing them with static radial gradients—reintroduce the dot-grid styles
(spring-damped variables), add the 13 `@keyframes` with tier timing, ensure
GlassCard uses those CSS keyframes/transitions instead of inline JS, and if you
cannot fully port the Canvas2D layer now, add a clear TODO/comment and create a
follow-up issue linking third_party/citadel so the pattern set (dot-grid + pulse
API + keyframes) is tracked for Step 3.
| tool_calling: '#ffbb44', | ||
| complete: '#66ffaa', | ||
| error: '#ff5566', | ||
| paused: '#888899', | ||
| waiting_permission: '#ffaa33', |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Naming inconsistency: tool_calling / waiting_permission break camelCase convention.
Every other key in COLORS is camelCase (e.g., holoBase, nodeInterior, liveResumeBg). These two snake_case keys stand out and don't align with the CSS counterparts (--state-tool, --state-waiting) either. Consider renaming to toolCalling / waitingPermission before consumers start importing them.
Proposed rename
- tool_calling: '#ffbb44',
+ toolCalling: '#ffbb44',
complete: '#66ffaa',
error: '#ff5566',
paused: '#888899',
- waiting_permission: '#ffaa33',
+ waitingPermission: '#ffaa33',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@shadow-agent/src/renderer/theme.ts` around lines 22 - 26, The COLORS object
contains two keys in snake_case (tool_calling, waiting_permission) that should
be renamed to camelCase to match the rest of the theme; update the COLORS map to
use toolCalling and waitingPermission and then search for and update all usages
(imports, property accesses, and any mapping to CSS variables like --state-tool
/ --state-waiting) to reference the new property names (e.g., replace references
to COLORS.tool_calling and COLORS.waiting_permission with COLORS.toolCalling and
COLORS.waitingPermission) to keep naming consistent and avoid breaking
consumers.
| export function withAlpha(rgbaBase: string, alpha: number): string { | ||
| return `${rgbaBase} ${alpha})`; | ||
| } |
There was a problem hiding this comment.
Fragile withAlpha contract — silently produces invalid CSS for non-Base inputs.
withAlpha only works when rgbaBase is one of the partial …Base strings (e.g., 'rgba(80, 160, 220,'). Passing any normal color ('#66ccff', 'rgba(...)') produces invalid CSS like "#66ccff 0.5)" with no error. At minimum, document the expected input shape; ideally branch a typed helper like nominal type RgbaBase = string & { __brand: 'rgba-base' } or accept (r,g,b,a) numerics.
Minimal doc fix
+/**
+ * Appends an alpha channel to a partial rgba prefix produced via the `*Base`
+ * COLORS entries (e.g., `'rgba(80, 160, 220,'`). Passing any other shape
+ * yields invalid CSS — do not use with hex or complete rgba strings.
+ */
export function withAlpha(rgbaBase: string, alpha: number): string {
return `${rgbaBase} ${alpha})`;
}📝 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 function withAlpha(rgbaBase: string, alpha: number): string { | |
| return `${rgbaBase} ${alpha})`; | |
| } | |
| /** | |
| * Appends an alpha channel to a partial rgba prefix produced via the `*Base` | |
| * COLORS entries (e.g., `'rgba(80, 160, 220,'`). Passing any other shape | |
| * yields invalid CSS — do not use with hex or complete rgba strings. | |
| */ | |
| export function withAlpha(rgbaBase: string, alpha: number): string { | |
| return `${rgbaBase} ${alpha})`; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@shadow-agent/src/renderer/theme.ts` around lines 218 - 220, withAlpha
currently assumes rgbaBase is a partial string like "rgba(r,g,b," and will
silently produce invalid CSS for normal color inputs; update withAlpha to either
(A) enforce/validate the expected shape by introducing a nominal/branded type
(RgbaBase) and throw a clear error if rgbaBase does not match the "rgba(...,"
pattern, or (B) broaden the API to accept numeric components (r:number,
g:number, b:number, a:number) or normal color strings by parsing/converting them
to a valid rgba string before appending alpha. Reference the withAlpha function
and add either the RgbaBase brand type or the new numeric signature, plus input
validation and an explicit error message when validation fails.
|
Attempted auto-merge failed: PR branch conflicts with main. Please rebase on main to resolve before merging. |
Summary
docs/todo.mdwith a pointer to Issues #8–#25 and a one-line index per issue.Test plan
docs/todo.mdrenders correctly on GitHub with working issue linksimplementation,observability,testing+ domain tags)https://claude.ai/code/session_01SbiXfM7JiKYJrG29pg9t6E
Generated by Claude Code
Pull Request Summary
Overview
This PR accomplishes two distinct objectives:
docs/todo.mdto GitHub Issues (Port agent-flow Canvas2D renderer #8–Maintain a shared replay fixture corpus #25)Changes Overview
docs/todo.mdshadow-agent/src/renderer/theme.tsshadow-agent/src/renderer/styles.cssshadow-agent/src/renderer/components/GlassCard.tsxDocumentation Changes
docs/todo.mdShifts task tracking from an in-file dated checklist to external GitHub Issues:
Code Changes
New: Theme Module (
shadow-agent/src/renderer/theme.ts)Centralizes holographic styling and animation parameters:
Exports:
COLORS— Immutable palette covering backgrounds, agent states, UI chrome, glass effects, transcript/message feed, code/diff colorsROLE_COLORS— Maps agent roles (assistant,thinking,user) tobg,bgSelected,text,labelvaluesTIMING— Animation timing constantsANIM— Easing and interaction physicsFX— Visual effects durations/countsBEAM— Beam geometry parametersMIN_VISIBLE_OPACITY— Visibility threshold constantgetStateColor(state)— Utility to mapAgentNode['state']to colorwithAlpha(rgbaBase, alpha)— Utility for alpha-adjusted color stringsNew: Glass Card Component (
shadow-agent/src/renderer/components/GlassCard.tsx)React component for animated card overlays with conditional rendering:
Props:
children: ReactNodeclassName?: stringstyle?: CSSPropertiesvisible: boolean— Controls mount/unmount and animation stateBehavior:
visible=trueand triggers fade-in/scale animationvisible=falseopacity,transform) with fixed CSS transitionglass-cardclassUpdated: CSS Theme (
shadow-agent/src/renderer/styles.css)Comprehensive redesign from prior Segoe/UI palette to hologram/glass aesthetic:
Key changes:
:rootpalette: New hologram, agent-state, glass-card, panel chrome, and translucent tint variables--void--state-*strokes and drop-shadow effects for active/completed states.glass-cardcomponent styling and.scrollbar-hideutilityIssue Coverage
All 18 created issues are labeled with domain tags (implementation, observability, testing) and issue #8 (Canvas2D renderer milestone) is marked as active.
Note
The PR description states "No code changes," but commit history reveals feature work on renderer theming and components (Step 1–2). Step 3 (Canvas2D layer replacement) will land separately.