feat: anchor feedback via GitHub Discussions#318
Merged
rdmueller merged 6 commits intoLLM-Coding:mainfrom Mar 18, 2026
Merged
Conversation
- Create 104 GitHub Discussions (one per anchor) in "Anchor Feedback" category - Add fetch-discussion-votes.js to generate feedback.json with upvote/comment counts - Card grid: show upvote count (>1) and comment badges linking to discussions - Anchor modal: feedback section with Vote and Discuss buttons - data-loader: load feedback.json alongside other data (graceful fallback if missing) - Update tests for new feedback data flow Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Caution Review failedPull request was closed or merged during review WalkthroughFügt eine GitHub‑Discussions‑Feedback‑Pipeline: zwei Node‑Skripte (Erstellen/Abrufen von Discussions), eine generierte Changes
Sequence Diagram(s)sequenceDiagram
participant FetchScript as "scripts/fetch-discussion-votes.js"
participant CreateScript as "scripts/create-anchor-discussions.js"
participant GitHub as "GitHub GraphQL API"
participant FS as "website/public/data/feedback.json"
participant DataLoader as "website/src/utils/data-loader.js"
participant Web as "Website (main.js)"
participant UI as "Components (anchor-modal / card-grid)"
FetchScript->>GitHub: Query discussions (paginiert)
GitHub-->>FetchScript: Diskussionen (title, body, upvoteCount, comments, url)
FetchScript->>FS: Schreibe Mapping anchorId -> {upvotes,comments,url}
CreateScript->>GitHub: List existing discussions (GraphQL) -> create Diskussionen (Mutation) [throttled]
Web->>DataLoader: fetchData()/fetchFeedbackData()
DataLoader-->>Web: anchors, categories, roles, feedback
Web->>UI: setFeedbackData(feedback) / render
UI-->>GitHub: Nutzer klickt Discuss/Upvote → öffnet Discussion URL
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
website/src/styles/main.css (1)
180-182: Feedback-Aktionen auf kleinen Screens umbrechbar machen.In Line 181 ist die Leiste rein horizontal; mit längeren Labels kann sie auf sehr kleinen Geräten überlaufen.
💡 Vorschlag
.feedback-actions { - `@apply` flex justify-center gap-3; + `@apply` flex flex-wrap justify-center gap-3; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/src/styles/main.css` around lines 180 - 182, Die Feedback-Aktionen (.feedback-actions) sind derzeit strikt horizontal und können auf sehr kleinen Bildschirmen aus dem Container laufen; ändere die Klasse so dass die Leiste umbrechen kann (z.B. flex-wrap oder responsive utilities wie flex-wrap / sm:flex-nowrap), damit Buttons/Labels bei kleinen Viewports untereinander gehen, behalte justify-center und gap-3 für Abstand und zentrierte Ausrichtung.website/src/utils/data-loader.test.js (1)
283-294: Fallback-Inhalt fürfeedbackexplizit prüfen.Der neue 404-Fall ist gut abgedeckt; eine zusätzliche Assertion auf den tatsächlichen Fallback macht den Test noch robuster.
🧪 Ergänzung
const first = await fetchData() const second = await fetchData() expect(first).toEqual(second) + expect(first.feedback).toEqual({}) expect(global.fetch).toHaveBeenCalledTimes(4)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/src/utils/data-loader.test.js` around lines 283 - 294, Der Test should additionally assert that when the fetch for 'data/feedback.json' returns 404 the returned payload contains the expected fallback value for the feedback field: after calling fetchData() (function fetchData) assert response.feedback equals the known fallback (e.g. [] or {} depending on the implementation) so the test explicitly verifies the fallback behavior for the 'feedback' key rather than only checking fetch call counts and URLs.website/src/utils/data-loader.js (1)
99-100: Fallback fürfeedback.jsonsollte nur „Datei fehlt“ abfangen.
catch(() => ({}))in Line 99 maskiert auch echte Laufzeitprobleme (z. B. 500/Netzwerk), wodurch Fehler schwer erkennbar werden.🎯 Präziseres Error-Handling
- fetchJson('data/feedback.json').catch(() => ({})), + fetchJson('data/feedback.json').catch((error) => { + if (error instanceof Error && /: 404$/.test(error.message)) return {} + throw error + }),As per coding guidelines
website/src/**/*.js: Check for: security issues, unused variables, proper error handling.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/src/utils/data-loader.js` around lines 99 - 100, The current catch(() => ({})) on the fetchJson('data/feedback.json') call swallows all errors; change it to only return an empty object for a missing file (HTTP 404 / ENOENT) and rethrow all other errors. Specifically, update the Promise chain around fetchJson('data/feedback.json') so the catch handler for that call inspects the error (e.g., err.status, err.response?.status, err.code, or err.message includes '404') and returns {} only when it indicates "not found"; otherwise rethrow the error so real network/server issues surface. Ensure this change targets the fetchJson invocation for 'data/feedback.json'.website/src/components/card-grid.js (1)
190-214: Potenzielle XSS-Schwachstelle beifb.urlprüfen.
escapeHtml()schützt vor HTML-Injection, aber nicht vorjavascript:-URLs. Fallsfeedback.jsonmanipuliert oder aus einer nicht vertrauenswürdigen Quelle stammt, könnte ein bösartiger URL wiejavascript:alert(1)ausgeführt werden.Da
feedback.jsondurch ein kontrolliertes Script generiert wird und GitHub-Discussion-URLs enthält, ist das Risiko gering – dennoch wäre eine Validierung als Defense-in-Depth sinnvoll.🛡️ Optionale URL-Validierung
+function isValidHttpUrl(url) { + try { + const parsed = new URL(url) + return parsed.protocol === 'https:' || parsed.protocol === 'http:' + } catch { + return false + } +} + // In renderAnchorCard: - ${ - fb && fb.upvotes > 1 - ? ` - <a href="${escapeHtml(fb.url)}" ...> + ${ + fb && fb.upvotes > 1 && isValidHttpUrl(fb.url) + ? ` + <a href="${escapeHtml(fb.url)}" ...>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/src/components/card-grid.js` around lines 190 - 214, The anchors rendering fb.url should reject non-http(s) schemes to prevent javascript: XSS; before interpolating fb.url in card-grid.js (the template that uses escapeHtml(fb.url) for the feedback-badge links), validate the URL scheme (e.g., implement an isValidHttpUrl function and only render the <a> when it returns true or when the scheme is in a whitelist like http/https), or fallback to a safe placeholder/href="#" for invalid URLs; keep using escapeHtml for HTML-encoding but add this scheme check around the blocks that render fb.url to ensure no javascript: or other dangerous schemes are emitted.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/create-anchor-discussions.js`:
- Around line 84-85: Fehlgeschlagene Aufrufe von createDiscussion werden derzeit
stillschweigend ignoriert; wrappe jeden createDiscussion-Aufruf (in der Schleife
über toCreate, und analog bei den anderen Schleifen) in ein try/catch,
incrementiere created nur im try nach erfolgreichem Abschluss und logge den
Fehler im catch; sammle Fehlervorkommen (z.B. failedCount oder failedIds) und am
Ende des Skripts beende mit non-zero Exit-Code (process.exit(1)) falls any
failures > 0, damit CI fehlschlägt; referenziere dabei die Variablen/IDs im Log
für einfache Nachverfolgung (toCreate, created, createDiscussion).
- Around line 24-26: ESLint is flagging use of setTimeout in the sleep(ms)
function because the ESLint config's nodeGlobals list omits setTimeout; update
the ESLint configuration where nodeGlobals is defined by adding setTimeout:
'readonly' to the nodeGlobals object so setTimeout is recognized as a read-only
global and the sleep(ms) function (and other uses) no longer error.
In `@scripts/fetch-discussion-votes.js`:
- Around line 68-78: The loop that populates feedback is silently overwriting
duplicate anchorIds (variable anchorId) when assigning feedback[anchorId] inside
the for (const d of discussions) loop; change this to detect existing
feedback[anchorId] and either merge the entries (e.g., sum up upvotes/comments
and aggregate URLs) or store duplicates as an array of entries so no data is
lost, updating the mapped counter logic accordingly; touch the feedback
assignment site and ensure any consumers of feedback handle the new structure
(e.g., expect an array or merged counts) and include anchorId, d.url,
d.upvoteCount, and d.comments.totalCount in the merged/aggregated
representation.
In `@website/src/components/anchor-modal.js`:
- Around line 271-273: The code uses fb.url directly as an href (fb.url in
anchor-modal.js after fetchFeedbackData/fb) which is unsafe; validate and
sanitize it before using as a link target by parsing it with the URL constructor
(or a robust URL parser) and enforcing an allowlist of safe protocols (e.g.,
http:, https: — optionally mailto:) and a non-empty hostname when applicable; if
validation fails, do not render the anchor or render a safe fallback (e.g.,
disabled link or plain text) and log/handle the error via the existing
error-handling path.
- Around line 281-282: The modal contains hardcoded display strings (e.g., the
Vote label rendered as <span>Vote${fb.upvotes > 1 ? ` (${fb.upvotes})` :
''}</span> and other strings around lines with fb/upvote usage) in
website/src/components/anchor-modal.js; replace these literals with i18n.t(...)
calls (create appropriate translation keys like "modal.vote" and
"modal.vote_count" or use i18n pluralization/interpolation for the upvote count)
so the Vote label and the other hardcoded strings at the referenced spots use
i18n.t(...) and include fb.upvotes via interpolation rather than string
concatenation. Ensure you update the translation files with the new keys and
keep the conditional plural/count behavior via i18n's pluralization or explicit
keys.
---
Nitpick comments:
In `@website/src/components/card-grid.js`:
- Around line 190-214: The anchors rendering fb.url should reject non-http(s)
schemes to prevent javascript: XSS; before interpolating fb.url in card-grid.js
(the template that uses escapeHtml(fb.url) for the feedback-badge links),
validate the URL scheme (e.g., implement an isValidHttpUrl function and only
render the <a> when it returns true or when the scheme is in a whitelist like
http/https), or fallback to a safe placeholder/href="#" for invalid URLs; keep
using escapeHtml for HTML-encoding but add this scheme check around the blocks
that render fb.url to ensure no javascript: or other dangerous schemes are
emitted.
In `@website/src/styles/main.css`:
- Around line 180-182: Die Feedback-Aktionen (.feedback-actions) sind derzeit
strikt horizontal und können auf sehr kleinen Bildschirmen aus dem Container
laufen; ändere die Klasse so dass die Leiste umbrechen kann (z.B. flex-wrap oder
responsive utilities wie flex-wrap / sm:flex-nowrap), damit Buttons/Labels bei
kleinen Viewports untereinander gehen, behalte justify-center und gap-3 für
Abstand und zentrierte Ausrichtung.
In `@website/src/utils/data-loader.js`:
- Around line 99-100: The current catch(() => ({})) on the
fetchJson('data/feedback.json') call swallows all errors; change it to only
return an empty object for a missing file (HTTP 404 / ENOENT) and rethrow all
other errors. Specifically, update the Promise chain around
fetchJson('data/feedback.json') so the catch handler for that call inspects the
error (e.g., err.status, err.response?.status, err.code, or err.message includes
'404') and returns {} only when it indicates "not found"; otherwise rethrow the
error so real network/server issues surface. Ensure this change targets the
fetchJson invocation for 'data/feedback.json'.
In `@website/src/utils/data-loader.test.js`:
- Around line 283-294: Der Test should additionally assert that when the fetch
for 'data/feedback.json' returns 404 the returned payload contains the expected
fallback value for the feedback field: after calling fetchData() (function
fetchData) assert response.feedback equals the known fallback (e.g. [] or {}
depending on the implementation) so the test explicitly verifies the fallback
behavior for the 'feedback' key rather than only checking fetch call counts and
URLs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 55a95057-6d05-425d-9072-a7cc8c91a882
📒 Files selected for processing (10)
scripts/create-anchor-discussions.jsscripts/fetch-discussion-votes.jswebsite/public/data/feedback.jsonwebsite/src/components/anchor-modal.jswebsite/src/components/anchor-modal.test.jswebsite/src/components/card-grid.jswebsite/src/main.jswebsite/src/styles/main.csswebsite/src/utils/data-loader.jswebsite/src/utils/data-loader.test.js
- Use insertAdjacentHTML instead of innerHTML += for feedback section (innerHTML += destroys event listeners on sub-anchor links) - Fix ESLint no-undef for setTimeout in Node.js script Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add timer globals to ESLint nodeGlobals config (revert global.setTimeout workaround) - Track failed discussion creations and set non-zero exit code - Warn on duplicate anchor-id mappings in fetch-discussion-votes - Validate feedback URLs against GitHub Discussions allowlist pattern - Localize feedback texts (Vote/Discuss/Requires GitHub login) via i18n Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
scripts/create-anchor-discussions.js (1)
21-22: Fehlende Validierung beim Laden der Anchors-Datei.Das synchrone Laden von
anchors.jsonauf Modulebene führt bei fehlender Datei zu einem unverständlichen Fehler. Eine defensive Prüfung wäre hilfreich.🛡️ Vorgeschlagene Verbesserung
const anchorsPath = path.join(__dirname, '..', 'website', 'public', 'data', 'anchors.json') +if (!fs.existsSync(anchorsPath)) { + console.error(`Error: anchors.json not found at ${anchorsPath}`) + console.error('Run the build first to generate anchors.json') + process.exit(1) +} const anchors = JSON.parse(fs.readFileSync(anchorsPath, 'utf-8'))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/create-anchor-discussions.js` around lines 21 - 22, Das Skript liest die Anchors-Datei synchron auf Modulebene ohne Validierung (anchorsPath / anchors via fs.readFileSync + JSON.parse), was bei fehlender oder ungültiger Datei zu einem kryptischen Crash führt; fix: prüfen ob die Datei existiert (z. B. fs.existsSync(anchorsPath)) und falls nicht, logge eine klare Fehlermeldung und beende den Prozess, und/oder wrappe fs.readFileSync + JSON.parse in einen try/catch, logge den genauen Fehler (inkl. anchorsPath und Parse-Fehler) und handle ihn sauber (return/exit), sodass anchors nur gesetzt wird, wenn Lesen und Parsen erfolgreich sind.scripts/fetch-discussion-votes.js (1)
15-22: Fehlende Fehlerbehandlung bei GraphQL-Aufrufen.Die Funktion
ghGraphqlfängt keine Fehler ab. Wenn dergh-CLI-Aufruf fehlschlägt (z.B. Netzwerkfehler, Authentifizierungsprobleme), stürzt das Skript mit einem nicht aussagekräftigen Fehler ab.♻️ Vorgeschlagene Verbesserung
function ghGraphql(query) { const payload = JSON.stringify({ query }) - const result = execSync('gh api graphql --input -', { - input: payload, - encoding: 'utf-8', - }) - return JSON.parse(result) + try { + const result = execSync('gh api graphql --input -', { + input: payload, + encoding: 'utf-8', + }) + const parsed = JSON.parse(result) + if (parsed.errors) { + throw new Error(`GraphQL errors: ${JSON.stringify(parsed.errors)}`) + } + return parsed + } catch (err) { + console.error('GraphQL request failed:', err.message) + throw err + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/fetch-discussion-votes.js` around lines 15 - 22, The ghGraphql function lacks error handling for the execSync call and JSON.parse, so failures from the `gh` CLI (network/auth errors or non-JSON output) crash with unclear messages; wrap the execSync invocation and subsequent JSON.parse in a try/catch inside ghGraphql, capture the child process error (including stderr/exit code) and/or parse error, and either rethrow a new Error with a clear message that includes the captured details or return a rejected Promise/structured error; ensure you reference the ghGraphql function and include the original error.message/stderr in the logged/thrown error so callers can diagnose the failure.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/create-anchor-discussions.js`:
- Around line 118-121: The delay condition uses `created < toCreate.length`
which fails when creations error because `created` only increments on success;
change the check to base the sleep on the loop/index of attempts (e.g., the
iteration variable) or a separate `attempted`/`processed` counter that
increments for every iteration, and call `sleep(2000)` when there are still
remaining iterations (for example when currentIndex < toCreate.length - 1);
update the condition where `sleep` is invoked to reference that iteration
variable (or `processed`) instead of `created` so the delay always occurs
between attempts regardless of success.
---
Nitpick comments:
In `@scripts/create-anchor-discussions.js`:
- Around line 21-22: Das Skript liest die Anchors-Datei synchron auf Modulebene
ohne Validierung (anchorsPath / anchors via fs.readFileSync + JSON.parse), was
bei fehlender oder ungültiger Datei zu einem kryptischen Crash führt; fix:
prüfen ob die Datei existiert (z. B. fs.existsSync(anchorsPath)) und falls
nicht, logge eine klare Fehlermeldung und beende den Prozess, und/oder wrappe
fs.readFileSync + JSON.parse in einen try/catch, logge den genauen Fehler (inkl.
anchorsPath und Parse-Fehler) und handle ihn sauber (return/exit), sodass
anchors nur gesetzt wird, wenn Lesen und Parsen erfolgreich sind.
In `@scripts/fetch-discussion-votes.js`:
- Around line 15-22: The ghGraphql function lacks error handling for the
execSync call and JSON.parse, so failures from the `gh` CLI (network/auth errors
or non-JSON output) crash with unclear messages; wrap the execSync invocation
and subsequent JSON.parse in a try/catch inside ghGraphql, capture the child
process error (including stderr/exit code) and/or parse error, and either
rethrow a new Error with a clear message that includes the captured details or
return a rejected Promise/structured error; ensure you reference the ghGraphql
function and include the original error.message/stderr in the logged/thrown
error so callers can diagnose the failure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: cf771dfd-dd0e-411f-84a1-4d773189a4b9
📒 Files selected for processing (6)
scripts/create-anchor-discussions.jsscripts/eslint.config.mjsscripts/fetch-discussion-votes.jswebsite/src/components/anchor-modal.jswebsite/src/translations/de.jsonwebsite/src/translations/en.json
🚧 Files skipped from review as they are similar to previous changes (1)
- website/src/components/anchor-modal.js
Contributor
Author
CI StatusLint & Format Check: ✅ Fixed (ESLint nodeGlobals, CodeRabbit findings) E2E Tests: ❌ — but this is a pre-existing failure, not introduced by this PR. The same 2 Umbrella Anchor tests ( All other E2E tests pass. The umbrella anchor test failures should be tracked as a separate issue. |
The delay between discussion creations was skipped on failures because the condition used `created` (only incremented on success) instead of the loop index. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ests The sub-anchor-link elements need time to render after the sub-anchor-list container appears. Wait for the link itself to be visible instead of just the list container. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The tier scale is inverted (tier 3 = best, tier 1 = worst). The sub-anchor
rendering incorrectly treated tier 3 as "not a semantic anchor" and rendered
them as non-clickable spans. This caused all GoF pattern sub-anchors to be
unclickable, breaking the E2E umbrella tests.
- tier 1 → greyed out, not clickable ("not a semantic anchor")
- tier 2/3 → clickable link
- Update unit test mock data to match corrected tier semantics
- E2E test: check for tier-1 items instead of tier-3
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements #210 — anchor feedback using GitHub Discussions as backend.
What's included
scripts/create-anchor-discussions.js— idempotent script for creating discussions (detects existing ones, useful for new anchors)scripts/fetch-discussion-votes.js— fetches upvote/comment counts via GraphQL, generatesfeedback.jsonfeedback.jsonis missing, everything works without feedback dataScreenshots
Cards show badges when an anchor has upvotes or comments:
Modal shows feedback section at bottom:
Still TODO (separate PRs)
feedback.json(cron schedule)Test plan
feedback.jsonis missing🤖 Generated with Claude Code
Summary by CodeRabbit
Neue Funktionen
Daten & CLI
Design / Stil
Tests
Internationalisierung