Repository navigation
fix(docs): use relative Eclair URLs for Cloudflare preview - #58
Conversation
Changed absolute Eclair URLs to relative paths in documentation to ensure preview URLs work correctly with Cloudflare deployment. Added ignoreDeadLinks config to VitePress to prevent build errors for /eclair/ links that point to a separate app. Closes #57 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
|
Warning Rate limit exceeded@NTCoding has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 7 minutes and 46 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdded VitePress config to ignore Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant B as Browser DOM
participant L as Layout Handler
participant W as Window
rect rgba(232,245,255,0.9)
note right of L: initEclairLinkHandler registered on mount\n(cleaned up on unmount)
end
U->>B: click element
B->>L: document click listener receives event
L->>L: find nearest anchor, read href & modifiers
alt href starts with /eclair/ and no modifier
L->>L: preventDefault()
L->>W: set window.location.href = href
note right of W: navigate to relative /eclair/ (preview-aware)
else other link or modifier pressed
L-->>B: allow default navigation
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
VitePress Vue Router intercepts /eclair/ link clicks and tries to handle them client-side, showing 404. Add global click handler that forces full page navigation for all /eclair/ paths, ensuring the request reaches the server where eclair assets are served. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Deploying living-architecture with
|
| Latest commit: |
cad9947
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://0e32e4f2.living-architecture.pages.dev |
| Branch Preview URL: | https://issue-57-eclair-cloudflare-p.living-architecture.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI Agents
In @apps/docs/.vitepress/theme/Layout.vue:
- Around line 16-28: The initEclairLinkHandler currently adds a click listener
to document and never removes it; change initEclairLinkHandler to capture the
listener callback (the function passed to document.addEventListener), attach it,
and return a cleanup function that calls document.removeEventListener with that
same callback; then update the Layout component to call initEclairLinkHandler()
on mount and invoke the returned cleanup on unmount so the global handler is
removed when the component is destroyed.
- Around line 17-27: The click listener's event parameter is untyped; explicitly
annotate it as MouseEvent in the document.addEventListener callback (e.g.,
(event: MouseEvent) => ...) to improve type safety, then keep the existing
target handling (const target = event.target as HTMLElement) and logic around
closest('a') and href checks; update the anonymous callback signature where
document.addEventListener('click', ...) is declared to include the MouseEvent
type.
- Around line 30-33: onMounted currently calls initEclairLinkHandler() but never
removes its event listener on teardown; capture the cleanup function returned by
initEclairLinkHandler (or update initEclairLinkHandler to return a removal
function) and call that cleanup inside an onUnmounted hook. Specifically, in
Layout.vue replace the bare initEclairLinkHandler() invocation with something
like const cleanup = initEclairLinkHandler() and add onUnmounted(() => cleanup
&& cleanup()), ensuring initEclairLinkHandler returns a function that removes
the listener if it doesn't already.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
apps/docs/.vitepress/theme/Layout.vue
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: main
Address CodeRabbit feedback: - Add onUnmounted hook to remove click event listener - Add explicit MouseEvent type for better type safety - Return cleanup function from initEclairLinkHandler 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In @apps/docs/.vitepress/theme/Layout.vue:
- Line 20: Change the cast of event.target from HTMLElement to the more
appropriate Element type: replace the usage where you currently do "const target
= event.target as HTMLElement" so it is cast to Element instead, since closest()
and related DOM methods are defined on Element; update any subsequent usages
that rely on HTMLElement-specific properties only if needed.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
apps/docs/.vitepress/theme/Layout.vue
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: main
- GitHub Check: Cloudflare Pages
🔇 Additional comments (3)
apps/docs/.vitepress/theme/Layout.vue (3)
3-3: Excellent work addressing the cleanup concerns from previous reviews.The addition of
onUnmountedimport and thecleanupEclairHandlervariable properly addresses the memory leak concern raised in previous reviews.Also applies to: 10-11
18-33: LGTM! Cleanup pattern and MouseEvent typing properly implemented.The function now correctly returns a cleanup function and includes explicit
MouseEventtyping, fully addressing previous review concerns. The logic for intercepting/eclair/links and forcing full-page navigation is appropriate for this use case.
35-45: Perfect implementation of lifecycle cleanup.The
onMountedandonUnmountedhooks properly manage the event listener lifecycle, storing and invoking the cleanup function to prevent memory leaks. The strict null check adds an extra layer of safety.
Replace `as HTMLElement` type assertion with runtime `instanceof Element` check for type safety per project conventions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI Agents
In @apps/docs/.vitepress/theme/Layout.vue:
- Around line 19-30: The click handler currently prevents default for all
clicks; update the handler function to early-return instead of preventing when
the user intent indicates a non-primary navigation: if event.defaultPrevented is
true, or event.button is not 0 (i.e., not a left-click), or any modifier key is
pressed (event.metaKey || event.ctrlKey || event.shiftKey || event.altKey), then
do nothing and allow the browser/other handlers to proceed; otherwise continue
to find link via target.closest('a'), validate href startsWith('/eclair/'), call
event.preventDefault() and set window.location.href as before. Ensure these
checks are added at the top of the existing handler.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
apps/docs/.vitepress/theme/Layout.vue
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: main
🔇 Additional comments (3)
apps/docs/.vitepress/theme/Layout.vue (3)
3-3: LGTM! Proper lifecycle import added.The
onUnmountedimport is correctly added to support the cleanup implementation requested in previous reviews.
20-21: Excellent improvement with runtime type checking!Using
instanceof Elementis superior to the type assertion suggested in past reviews. This runtime check safely handles edge cases whereevent.targetmight be something other than an Element (e.g., the document or window in certain synthetic events).
37-47: LGTM! Proper lifecycle management implemented.The cleanup implementation correctly addresses the memory leak concerns from previous reviews:
- Event listener is registered on mount
- Cleanup function is properly stored
- Listener is removed on unmount
- State is reset to null
Address CodeRabbit feedback: - Respect ctrl+click, cmd+click, shift+click and middle-click for standard browser UX (open in new tab behavior) - Extract isEclairPath helper that handles /eclair, /eclair/, and /eclair?query edge cases 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In @apps/docs/.vitepress/theme/Layout.vue:
- Around line 22-42: In initEclairLinkHandler, update the internal handler
function to bail out early if event.altKey is true or event.defaultPrevented is
true (in addition to the existing ctrlKey/metaKey/shiftKey/button checks) so we
preserve Alt+click browser/extension behavior and respect previously prevented
defaults; place these checks at the top of handler (before resolving
target/link/href) and only call event.preventDefault() and set
window.location.href when none of those flags are set and the href passes
isEclairPath.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
apps/docs/.vitepress/theme/Layout.vue
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: main
- GitHub Check: Cloudflare Pages
🔇 Additional comments (3)
apps/docs/.vitepress/theme/Layout.vue (3)
3-3: LGTM: Lifecycle imports and cleanup variable properly set up.The addition of
onUnmountedand the module-scopedcleanupEclairHandlervariable correctly establish the cleanup pattern to prevent memory leaks.Also applies to: 10-11
18-20: LGTM: isEclairPath helper correctly handles edge cases.The helper function properly matches
/eclair,/eclair/, and/eclair?patterns while avoiding false positives like/eclair-something/. Good abstraction.
44-54: LGTM: Lifecycle hooks properly manage event listener lifecycle.The
onMountedhook correctly captures the cleanup function, andonUnmountedsafely removes the event listener when the component is destroyed. This properly addresses the memory leak concern from earlier reviews.
- Add get-pr-feedback.sh: unified script using native isResolved field - Simplify submit-pr.sh: remove HTML parsing, delegate to new script - Simplify check-coderabbit-review.sh: remove 120+ lines of HTML parsing - Update complete-task.md: use new GraphQL-based workflow - Add "Process before fix" principle to CLAUDE.md The GraphQL approach eliminates brittle HTML parsing and uses GitHub's native review thread resolution status. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Remove wait_for_coderabbit_review() - CodeRabbit is a required check, gh pr checks --watch already waits for it - Remove CodeRabbit-specific logic - script is now check-agnostic - Delete check-coderabbit-review.sh - redundant, nothing uses it submit-pr.sh: 196 → 138 lines 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI Agents
In @.claude/commands/complete-task.md:
- Around line 88-96: The documented success check looking for the literal string
"Unresolved Items: 0" is incorrect because get-pr-feedback.sh emits "✓ No
unresolved feedback" when there are no unresolved items; update the
documentation in the complete-task instructions to check for "✓ No unresolved
feedback" (or alternatively modify get-pr-feedback.sh to print "Unresolved
Items: 0" on success) so the documented condition matches the actual output;
refer to get-pr-feedback.sh and the success-check bullet in the complete-task
instructions and make the change consistently wherever that success string is
referenced.
In @scripts/get-pr-feedback.sh:
- Line 82: Remove the unused AUTHOR variable extraction: delete the line that
assigns AUTHOR (AUTHOR=$(echo "$thread" | jq -r '.comments.nodes[0].author.login
// "unknown"')) and any references to AUTHOR elsewhere; ensure no other code
depends on AUTHOR (if it does, replace with direct usage of the jq expression or
the correct variable).
- Line 79: Remove the unused THREAD_ID variable extraction: delete the line that
sets THREAD_ID from the parsed JSON (the assignment using THREAD_ID=$(echo
"$thread" | jq -r '.id // ""')). Keep the existing use of the thread variable
and any other jq parsing, no other logic changes needed.
In @scripts/submit-pr.sh:
- Around line 79-84: The current show_pr_feedback function silences all errors
from get-pr-feedback.sh via 2>/dev/null which hides useful diagnostic output;
update show_pr_feedback to capture stderr from ./scripts/get-pr-feedback.sh
(e.g., redirect stderr to a debug/log file or capture it into a variable) and
write that stderr to a persistent debug file when the call fails, while still
falling back to echo "(could not fetch feedback)"; refer to the show_pr_feedback
function and the ./scripts/get-pr-feedback.sh invocation when implementing this
change.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
.claude/commands/complete-task.mdCLAUDE.mdscripts/check-coderabbit-review.shscripts/get-pr-feedback.shscripts/submit-pr.sh
🧰 Additional context used
🧠 Learnings (12)
📚 Learning: 2026-01-05T13:01:42.216Z
Learnt from: CR
Repo: NTCoding/living-architecture PR: 0
File: docs/workflow/code-review.md:0-0
Timestamp: 2026-01-05T13:01:42.216Z
Learning: After addressing code review issues, complete the task by following docs/workflow/task-workflow.md → Completing Tasks.
Applied to files:
.claude/commands/complete-task.md
📚 Learning: 2026-01-04T18:35:42.314Z
Learnt from: CR
Repo: NTCoding/living-architecture PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-04T18:35:42.314Z
Learning: Follow the task workflow steps exactly in `docs/workflow/task-workflow.md` before making any changes or committing code
Applied to files:
.claude/commands/complete-task.mdCLAUDE.md
📚 Learning: 2026-01-05T13:01:29.359Z
Learnt from: CR
Repo: NTCoding/living-architecture PR: 0
File: docs/conventions/codebase-structure.md:0-0
Timestamp: 2026-01-05T13:01:29.359Z
Learning: Update CLAUDE.md 'Current packages' section when adding new packages
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-04T18:35:42.314Z
Learnt from: CR
Repo: NTCoding/living-architecture PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-04T18:35:42.314Z
Learning: Applies to eslint.config.mjs|tsconfig.base.json|nx.json|vite.config.*|vitest.config.mts : Do not modify root configuration files (eslint.config.mjs, tsconfig.base.json, nx.json, vite.config, vitest.config.mts)
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-04T18:35:42.314Z
Learnt from: CR
Repo: NTCoding/living-architecture PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-04T18:35:42.314Z
Learning: When writing, editing, refactoring, or reviewing code, always follow `docs/conventions/software-design.md`
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-04T18:35:42.314Z
Learnt from: CR
Repo: NTCoding/living-architecture PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-04T18:35:42.314Z
Learning: Applies to **/*.{test,spec}.{ts,tsx,js,jsx} : Follow `docs/conventions/testing.md` when writing tests
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-05T13:01:42.216Z
Learnt from: CR
Repo: NTCoding/living-architecture PR: 0
File: docs/workflow/code-review.md:0-0
Timestamp: 2026-01-05T13:01:42.216Z
Learning: Applies to docs/workflow/**/*.{ts,tsx,js,jsx} : Avoid dangerous fallback values like `value ?? 'default'` or `value || 'fallback'` without clear reason. If a value is required, make it required. Don't hide missing data.
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-05T13:01:42.216Z
Learnt from: CR
Repo: NTCoding/living-architecture PR: 0
File: docs/workflow/code-review.md:0-0
Timestamp: 2026-01-05T13:01:42.216Z
Learning: First read lint rules to ensure feedback doesn't contradict them at /eslint.config.mjs. Never suggest using banned keywords like 'as' or 'let' when they are prohibited by linting rules.
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-03T17:37:22.708Z
Learnt from: CR
Repo: NTCoding/living-architecture PR: 0
File: docs/conventions/software-design.md:0-0
Timestamp: 2026-01-03T17:37:22.708Z
Learning: Applies to docs/conventions/**/*.{ts,tsx} : Strive for maximum type-safety. No `any`. No `as`. Type escape hatches defeat TypeScript's purpose. There's always a type-safe solution (except `as const` which is allowed).
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-03T17:37:22.708Z
Learnt from: CR
Repo: NTCoding/living-architecture PR: 0
File: docs/conventions/software-design.md:0-0
Timestamp: 2026-01-03T17:37:22.708Z
Learning: Applies to docs/conventions/**/*.{ts,tsx} : Fail-fast over silent fallbacks. Never use fallback chains (`value ?? backup ?? 'unknown'`). If data should exist, validate and throw a clear error.
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-03T17:37:22.708Z
Learnt from: CR
Repo: NTCoding/living-architecture PR: 0
File: docs/conventions/software-design.md:0-0
Timestamp: 2026-01-03T17:37:22.708Z
Learning: Applies to docs/conventions/**/*.{ts,tsx} : Prefer `const` over `let`. Use spread (`...`) over mutation. Prefer `map`/`filter`/`reduce` over `forEach` with mutation.
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-03T17:37:22.708Z
Learnt from: CR
Repo: NTCoding/living-architecture PR: 0
File: docs/conventions/software-design.md:0-0
Timestamp: 2026-01-03T17:37:22.708Z
Learning: Applies to docs/conventions/**/*.{ts,tsx} : Prefer immutability. Default to immutable data. Mutation is a source of bugs—use spread operator and return new values instead of mutating.
Applied to files:
CLAUDE.md
🪛 Shellcheck (0.11.0)
scripts/get-pr-feedback.sh
[warning] 79-79: THREAD_ID appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 82-82: AUTHOR appears unused. Verify use (or export if used externally).
(SC2034)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: main
- GitHub Check: Cloudflare Pages
🔇 Additional comments (13)
CLAUDE.md (1)
147-147: LGTM! Clear "Process before fix" principle.This guideline reinforces fail-fast philosophy and aligns well with the task workflow learnings.
scripts/get-pr-feedback.sh (5)
1-14: LGTM! Clear documentation.Header provides comprehensive usage and exit code documentation.
15-34: LGTM! Proper error handling.PR number resolution with appropriate fallback and error messages to stderr.
56-75: LGTM! Clean response parsing and early exit.Proper jq usage to extract review decision and unresolved threads with an appropriate early exit when no feedback exists.
109-111: Verify AI prompt extraction handles multi-line prompts.The current approach uses
grep -A 20which limits context to 20 lines after the marker, then pipes through multiple filters andhead -12. This might truncate longer AI prompts.Test with a PR that has a long AI prompt to ensure the extraction captures the full content. If truncation is acceptable, consider adding a comment to document this limitation.
126-132: LGTM! Clear actionable guidance.Footer provides clear next steps with proper command examples.
scripts/submit-pr.sh (3)
170-170: LGTM! Proper feedback display on changes requested.Correctly shows feedback when CodeRabbit requests changes.
184-184: LGTM! Feedback shown on success path.Displays feedback even when all checks pass, which is helpful for reviewing nitpicks.
189-189: LGTM! Feedback shown on CI failure.Shows feedback alongside CI failures for comprehensive context.
scripts/check-coderabbit-review.sh (4)
17-23: LGTM! Simplified PR number resolution.Clean implementation with proper error handling.
26-32: LGTM! Proper review state detection.Correctly filters CodeRabbit reviews and handles pending state.
34-38: LGTM! Clean delegation to centralized feedback script.Proper integration with the new feedback mechanism.
40-45: LGTM! Clear exit code logic.Properly exits based on review decision, matching documented behavior.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI Agents
In @scripts/submit-pr.sh:
- Around line 115-121: The script later parses PR_INFO into PR_NUMBER and PR_URL
using jq (and calls gh earlier), but it doesn't verify those CLI tools exist;
add a check right after the initial set -e in submit-pr.sh to validate
availability of jq and gh, printing a clear error to stderr and exiting non‑zero
if either is missing so the PR_INFO/PR_NUMBER/PR_URL parsing steps cannot fail
with unclear errors.
- Around line 44-49: The show_pr_feedback function should verify that the
./scripts/get-pr-feedback.sh script exists and is executable before invoking it:
check the file with a test like -x and, if missing or not executable, echo a
clear message such as "(could not fetch feedback: get-pr-feedback.sh not found
or not executable)"; otherwise call it and preserve the existing fallback for
runtime failures. Ensure to reference the show_pr_feedback function and the
./scripts/get-pr-feedback.sh script when making the change.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
scripts/check-coderabbit-review.shscripts/submit-pr.sh
💤 Files with no reviewable changes (1)
- scripts/check-coderabbit-review.sh
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: NTCoding/living-architecture PR: 0
File: apps/eclair/CLAUDE.md:0-0
Timestamp: 2026-01-02T11:58:23.322Z
Learning: Applies to apps/eclair/**/*.{tsx,jsx,ts,js,scss,css,sass,less} : All UI and UX design must conform to Éclair-specific design system in `/apps/eclair/docs/brand/` (themes, components, graph visualization, accessibility)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: main
- GitHub Check: Cloudflare Pages
🔇 Additional comments (2)
scripts/submit-pr.sh (2)
107-113: LGTM! Informative comment.The comment clarifies why the script waits for all checks, including CodeRabbit as a required check.
1-137: Inconsistency: PR objectives don't match file changes.The PR objectives describe fixing Eclair documentation URLs for Cloudflare preview deployments (changing absolute URLs to relative paths), but this file (
scripts/submit-pr.sh) is a shell script for PR submission automation with no relation to documentation URLs or Eclair.Please verify that the correct files and PR description are being reviewed together.
- Add event.defaultPrevented and event.altKey checks to eclair link handler - Fix complete-task.md success condition string to match actual script output - Remove unused THREAD_ID and AUTHOR variables from get-pr-feedback.sh - Add gh/jq availability check at script start in submit-pr.sh - Validate get-pr-feedback.sh existence before invocation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
|
Re: scripts/get-pr-feedback.sh:47 - GraphQL variables nitpick Not fixing: The sed substitution approach is intentional here. Using |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI Agents
In @apps/docs/.vitepress/theme/Layout.vue:
- Around line 18-20: The helper isEclairPath currently checks exact, subpath,
and query forms but omits hash fragments; update isEclairPath to also treat URLs
with hash fragments for Eclair as matches by checking for href starting with
'/eclair#' or by stripping the fragment (split on '#') and evaluating the base
href (use the same equality/startsWith checks on the fragment-stripped href) so
routes like '/eclair#demo' are recognized.
In @scripts/get-pr-feedback.sh:
- Around line 84-99: The grep pattern for critical severity is truncated ("_⚠️
Potential issue_ | _🔴") making it inconsistent with other checks; update the
condition that tests BODY (the first if that sets SEVERITY="critical" and
ICON="●") to match the full marker (e.g., "_⚠️ Potential issue_ | _🔴
Critical_") so it parallels the other patterns and reliably detects the critical
tag when parsing BODY.
- Around line 1-14: Add explicit availability checks for the CLI tools used by
get-pr-feedback.sh: after the shebang and set -e near the top of the script,
verify that both gh and jq are present (use a command existence test for "gh"
and "jq"), and if either is missing print a clear error message indicating which
tool is required and exit with status 1; ensure the error text is actionable for
standalone usage of get-pr-feedback.sh.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
.claude/commands/complete-task.mdapps/docs/.vitepress/theme/Layout.vuescripts/get-pr-feedback.shscripts/submit-pr.sh
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-01-05T13:01:42.216Z
Learnt from: CR
Repo: NTCoding/living-architecture PR: 0
File: docs/workflow/code-review.md:0-0
Timestamp: 2026-01-05T13:01:42.216Z
Learning: After addressing code review issues, complete the task by following docs/workflow/task-workflow.md → Completing Tasks.
Applied to files:
.claude/commands/complete-task.md
📚 Learning: 2026-01-04T18:35:42.314Z
Learnt from: CR
Repo: NTCoding/living-architecture PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-04T18:35:42.314Z
Learning: Follow the task workflow steps exactly in `docs/workflow/task-workflow.md` before making any changes or committing code
Applied to files:
.claude/commands/complete-task.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: main
- GitHub Check: Cloudflare Pages
🔇 Additional comments (11)
.claude/commands/complete-task.md (1)
88-96: Documentation now correctly matches script output.The success condition check at lines 90 and 96 now correctly references
"✓ No unresolved feedback", which matches the actual output fromget-pr-feedback.sh(line 73). The workflow is coherent and actionable.scripts/get-pr-feedback.sh (2)
77-122: Thread processing logic is sound.The loop correctly processes unresolved threads, with appropriate fallbacks for missing titles (line 103-105) and empty AI prompts. The piped while-read pattern is suitable here since no loop variables need to persist afterward.
67-75: Output format aligns with documentation.The success message
"✓ No unresolved feedback"at line 73 correctly matches whatcomplete-task.mdinstructs users to check for. Exit code 0 is appropriate for both empty results and successful processing.scripts/submit-pr.sh (3)
9-15: Tool validation implemented correctly.The loop checking for
ghandjqavailability with clear error messages addresses the previous review feedback. This prevents unclear failures later in the script.
52-61: Script existence validation implemented as suggested.The
-xcheck and descriptive error message correctly address the previous review feedback. The dual-layer error handling (existence check + runtime fallback) provides clear diagnostics.
134-148: Unified feedback display in both paths.Showing PR feedback in both success and failure paths (lines 137 and 143) ensures users always see pending review items regardless of CI status. This improves workflow visibility.
apps/docs/.vitepress/theme/Layout.vue (5)
3-3: LGTM! Import addition addresses lifecycle cleanup.The
onUnmountedimport is correctly added to support proper cleanup of the global event listener, resolving the memory leak concern from previous reviews.
10-11: LGTM! Cleanup tracker properly scoped.The
cleanupEclairHandlervariable correctly tracks the cleanup function for proper lifecycle management. Module scope is appropriate for VitePress's singleton Layout component.
22-43: Excellent implementation! All previous feedback incorporated.The
initEclairLinkHandlerfunction demonstrates best practices:
- Type safety: Explicit
MouseEventtype andinstanceof Elementcheck- Browser UX: Properly respects modifier keys (Ctrl/Cmd/Shift/Alt), middle-click, and
defaultPrevented- Pattern matching: Uses the robust
isEclairPathhelper to avoid false matches- Lifecycle: Returns cleanup function for proper teardown
- Performance: Early returns minimize overhead for non-Eclair clicks
The use of
window.location.hrefcauses a full page reload, which is intentional since Eclair is deployed as a separate app alongside the docs (per PR objectives).
45-48: LGTM! Proper initialization on mount.The
onMountedhook correctly initializes both the existing zoom functionality and the new Eclair link handler, properly capturing the cleanup function for later teardown.
50-55: LGTM! Proper cleanup prevents memory leaks.The
onUnmountedhook correctly removes the global click listener when the Layout component unmounts, fully addressing the memory leak concern from previous reviews. The defensive null check and cleanup are well-implemented.
- Add hash fragment support to isEclairPath for routes like /eclair#demo - Add gh/jq availability check to get-pr-feedback.sh - Complete critical severity pattern with full marker text 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In @apps/docs/.vitepress/theme/Layout.vue:
- Around line 18-21: The isEclairPath function contains a redundant condition;
remove the final check href.startsWith('/eclair#') from isEclairPath and keep
the existing basePath logic (const basePath = href.split('#')[0]) along with the
comparisons basePath === '/eclair', basePath.startsWith('/eclair/'), and
basePath.startsWith('/eclair?') so behavior remains identical but simpler.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
apps/docs/.vitepress/theme/Layout.vuescripts/get-pr-feedback.sh
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-02T11:58:23.322Z
Learnt from: CR
Repo: NTCoding/living-architecture PR: 0
File: apps/eclair/CLAUDE.md:0-0
Timestamp: 2026-01-02T11:58:23.322Z
Learning: Applies to apps/eclair/**/*.{tsx,jsx,ts,js,scss,css,sass,less} : All UI and UX design must conform to Éclair-specific design system in `/apps/eclair/docs/brand/` (themes, components, graph visualization, accessibility)
Applied to files:
apps/docs/.vitepress/theme/Layout.vue
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: main
- GitHub Check: Cloudflare Pages
🔇 Additional comments (6)
apps/docs/.vitepress/theme/Layout.vue (5)
3-3: LGTM! Lifecycle import correctly added.The addition of
onUnmountedto the imports is appropriate for the cleanup logic implemented below.
10-11: Module-level state is acceptable for VitePress Layout.The module-level
cleanupEclairHandlervariable is appropriate here since the VitePress Layout component is effectively a singleton. While module-level state can be problematic with multiple component instances, this pattern is safe in this context.
23-44: LGTM! Click handler correctly implements full-page navigation to Eclair.The implementation properly:
- Respects browser navigation patterns (modifier keys, middle-click, already-prevented events)
- Validates the click target and href before intercepting
- Returns a cleanup function to prevent memory leaks
- Uses
window.location.hreffor full-page navigation, which is appropriate given that docs and Eclair are separate applications served together on the same domain
46-49: LGTM! Mount hook correctly initializes handlers.The
onMountedhook properly initializes both zoom functionality and the Eclair link handler, storing the cleanup function for unmount.
51-56: LGTM! Unmount hook properly cleans up the event listener.The
onUnmountedhook correctly removes the click event listener to prevent memory leaks, following best practices with null checking and cleanup.scripts/get-pr-feedback.sh (1)
1-139: LGTM! All previous feedback has been addressed.The script is well-structured and correctly implements PR feedback retrieval:
- Tool availability checks now verify
ghandjqpresence (lines 15-21) ✓- Critical severity pattern includes the full marker text (line 92) ✓
- Unused variables (
THREAD_ID,AUTHOR) have been removed ✓- GraphQL query handles errors appropriately and correctly parses unresolved threads
- The output format is clear and actionable, with specific commands for users to follow
The sed-based placeholder substitution approach (lines 45-55) works correctly—the user's explanation in the PR objectives that this approach maintains readability and avoids complex JSON escaping in bash is reasonable.
- Add thread ID to GraphQL query and display - Show author (coderabbitai vs human reviewers) - Create reply-to-thread.sh for replying directly to review threads - Update get-pr-feedback.sh to show clean Reply command per thread - Replies go to the specific thread, not generic PR comments 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI Agents
In @scripts/reply-to-thread.sh:
- Around line 1-12: Add explicit availability checks for required CLI tools in
reply-to-thread.sh: verify that both gh and jq are installed (e.g., using
command -v or which) at the top of the script before any commands run, and if
either is missing print a clear error message identifying the missing tool(s)
and exit non‑zero. Keep this check consistent with how get-pr-feedback.sh
performs its tool validation so users get a uniform error message and early fail
when running reply-to-thread.sh.
- Around line 29-30: The current ESCAPED_MESSAGE assignment only escapes double
quotes and will produce malformed JSON for backslashes and control characters;
replace the sed-based escaping by encoding MESSAGE as a proper JSON string
(e.g., set ESCAPED_MESSAGE by running MESSAGE through a JSON encoder such as jq
-Rs '.' or an equivalent json.dumps call) and then use ESCAPED_MESSAGE directly
in the JSON payload without adding extra surrounding quotes; update the code
that builds the JSON body to consume ESCAPED_MESSAGE as an already-quoted JSON
string and ensure variables referenced are MESSAGE and ESCAPED_MESSAGE (or
fallback to a safe json-encoding helper if jq may be missing).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
scripts/get-pr-feedback.shscripts/reply-to-thread.sh
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: NTCoding
Repo: NTCoding/living-architecture PR: 58
File: apps/docs/.vitepress/theme/Layout.vue:18-21
Timestamp: 2026-01-06T18:04:45.308Z
Learning: In `apps/docs/.vitepress/theme/Layout.vue`, NTCoding prefers keeping explicit path-checking conditions in `isEclairPath()` even when technically redundant, as they improve clarity and readability over minimal optimization.
📚 Learning: 2026-01-06T18:02:51.422Z
Learnt from: NTCoding
Repo: NTCoding/living-architecture PR: 58
File: scripts/get-pr-feedback.sh:45-55
Timestamp: 2026-01-06T18:02:51.422Z
Learning: In shell scripts that call GraphQL APIs (e.g., scripts/get-pr-feedback.sh and similar), prefer using sed-based interpolation for GraphQL variables instead of gh api graphql's -f/-F flags. Rationale: sed-based interpolation is typically simpler and more robust in Bash contexts, avoids JSON encoding pitfalls, and keeps the script portable. Implement by building the GraphQL query as a string and substituting variables with sed or parameter expansion, ensuring proper escaping. If you must use -f/-F, provide clear justification in a comment.
Applied to files:
scripts/get-pr-feedback.shscripts/reply-to-thread.sh
🪛 Shellcheck (0.11.0)
scripts/reply-to-thread.sh
[style] 30-30: See if you can use ${variable//search/replace} instead.
(SC2001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: main
🔇 Additional comments (9)
scripts/reply-to-thread.sh (1)
35-47: LGTM!Error handling and success output are well-implemented. The
// emptyfallback handles missing URL gracefully.scripts/get-pr-feedback.sh (8)
13-21: LGTM!Tool availability checks are properly implemented, addressing standalone usage concerns.
23-42: LGTM!PR number resolution with fallback to current branch is well-implemented with proper error handling.
44-55: LGTM!The sed-based GraphQL variable interpolation aligns with the established preference for this codebase. Based on learnings, this approach is preferred over
gh api graphql's native-f/-Fflags.
57-83: LGTM!Response parsing, error handling, and early exit for zero unresolved items are well-structured.
85-91: LGTM!Thread processing correctly extracts all necessary fields. THREAD_ID and AUTHOR are now properly used in the output (lines 120, 123, 135).
93-109: LGTM!Severity detection covers the expected CodeRabbit formats with appropriate icons and a sensible default fallback.
111-132: LGTM!Title and AI prompt extraction handle the expected CodeRabbit comment format appropriately. The multi-filter approach for prompt extraction may miss some edge cases, but this is acceptable for a display utility.
134-147: LGTM!The reply command integration and actionable guidance provide a clear workflow for addressing feedback.
Replies now include "🤖 Via Claude Code" signature so it's clear the response was automated, not from the user directly. Uses jq for proper JSON escaping of multiline messages. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Clarify that automated replies are not from the repository owner.
basePath already strips the hash, so href.startsWith('/eclair#') is
covered by basePath === '/eclair'.
|
1 participant
Summary
ignoreDeadLinksconfig to VitePress for/eclair/pathsFixes #57
Problem
Documentation was hardcoding
https://living-architecture.dev/eclair/URLs. When Cloudflare creates preview deployments (e.g.,https://abc123.living-architecture.pages.dev/), the Eclair links pointed to production instead of the preview.Solution
Changed all Eclair URLs from absolute to relative (
/eclair/...). Since both docs and eclair are deployed as a single Cloudflare Pages project, relative URLs work correctly in both production and preview environments.Trade-off
Demo links won't work during
nx serve docsalone (since eclair runs on a different port). Developers can run eclair separately if needed.Test plan
nx build docs🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.