fix: teaching-contract formatting, box height cap, anchor highlighting + dry ref#574
Conversation
- contracts.json: the 'Explaining and Teaching' template was one unbroken paragraph and rendered as a wall of text. Add paragraph breaks and a bulleted loop list (EN + DE), matching the sibling contracts. Wording unchanged. - contracts-page.js: cap each contract's definition box at max-h-64 with overflow-y-auto, so long contracts (e.g. Architecture Documentation) scroll instead of dominating the page. Verified in-browser: box maxHeight 256px, overflow-y auto, scrolls; template renders 6 paragraphs + 3 bullets. 98/98 unit tests pass.
Verbatim mentions of a contract's declared anchor titles are now highlighted and linked to the anchor within the rendered template box. Scoped to the contract's own anchors array (no global false positives), longest-title-first, word-boundary matched. Highlighting is applied only in the rendered DOM (renderContractCard). The copy/download export (generateMarkdown) uses the raw template string, so the export stays plain CLAUDE.md-ready text. A null-char token in the matcher prevents collisions with literal digits in other templates (e.g. 'at most 3 questions'). main.js now loads anchor titles (fetchAnchorsData) alongside contracts and passes an id->title map into initContractsPage.
…ple -> dry) The code-quality contract referenced anchor id 'dry-principle', which does not exist — the DRY anchor's id is 'dry' (added in LLM-Coding#569). The chip linked to a 404. Point it at the real anchor.
|
Warning Review limit reached
More reviews will be available in 8 minutes and 6 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughDie Contracts-Seite erkennt Anker-Titel/Aliase in Template-Texten und verlinkt sie. Dazu wurden contracts.json angepasst, in ChangesAnker-Highlighting für Vertrags-Templates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 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 unit tests (beta)
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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@website/src/components/contracts-page.js`:
- Around line 152-159: The cards and markdown are falling back to English
because code in renderContractCard/getLocalizedField and the highlightAnchors
helper reads i18n.currentLanguage (which is undefined); replace all uses of the
property with the function call i18n.currentLang() so localization uses the
active language—update highlightAnchors, renderContractCard, getLocalizedField
and buildContractsMarkdown to call i18n.currentLang() instead of accessing
i18n.currentLanguage.
In `@website/src/main.js`:
- Around line 336-340: The current Promise.all([fetchContractsData(),
fetchAnchorsData()]) makes the whole page fail if fetchAnchorsData rejects;
change the flow so that fetchContractsData failure still aborts/render error but
fetchAnchorsData failure is treated as non-fatal: call fetchContractsData() and
fetchAnchorsData() separately (or use Promise.allSettled), build anchorTitles =
{} only if anchors resolved otherwise keep it empty, and always call
initContractsPage(contracts, anchorTitles) when contracts succeeded; reference
Promise.all, fetchContractsData, fetchAnchorsData, anchorTitles, and
initContractsPage to locate and update the logic accordingly.
🪄 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: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: fe11a6bb-8219-4142-8a50-6f0938baac11
📒 Files selected for processing (3)
website/public/data/contracts.jsonwebsite/src/components/contracts-page.jswebsite/src/main.js
| const anchorIds = contract.anchors || [] | ||
| const templateHtml = template | ||
| .split('\n') | ||
| .map((line) => { | ||
| if (line.startsWith('- ')) { | ||
| return `<span class="text-[var(--color-text-secondary)]">• ${esc(line.slice(2))}</span>` | ||
| return `<span class="text-[var(--color-text-secondary)]">• ${highlightAnchors(line.slice(2), anchorIds)}</span>` | ||
| } | ||
| return `<span>${esc(line)}</span>` | ||
| return `<span>${highlightAnchors(line, anchorIds)}</span>` |
There was a problem hiding this comment.
Die neue Render-Strecke bleibt dadurch effektiv EN-only.
highlightAnchors() ist hier nicht das Problem — renderContractCard() zieht den Text weiter über getLocalizedField(), und der Helper liest in derselben Datei i18n.currentLanguage. In diesem Repo ist diese Property undefined, daher fallen die Karten immer auf EN zurück; die neu formatierten templateDe-Texte und auch der Copy/Download-Pfad in buildContractsMarkdown() werden auf der Contracts-Seite nicht korrekt lokalisiert. Bitte den Sprachzugriff konsistent auf i18n.currentLang() umstellen.
Based on learnings: In this repo’s i18n integration (website/src/i18n.js), the active language must be read via the method i18n.currentLang() rather than a property like i18n.currentLanguage.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@website/src/components/contracts-page.js` around lines 152 - 159, The cards
and markdown are falling back to English because code in
renderContractCard/getLocalizedField and the highlightAnchors helper reads
i18n.currentLanguage (which is undefined); replace all uses of the property with
the function call i18n.currentLang() so localization uses the active
language—update highlightAnchors, renderContractCard, getLocalizedField and
buildContractsMarkdown to call i18n.currentLang() instead of accessing
i18n.currentLanguage.
The verbatim-title matcher missed anchors whose prose mention differs from the canonical title (e.g. 'MECE Principle' written as 'MECE', 'arc42 Architecture Documentation' as 'arc42'). Add a curated ANCHOR_ALIASES map of surface forms, each verified to appear verbatim in at least one contract template (EN or DE) and to be unambiguous among a contract's declared anchors. Titles and aliases are matched uniformly, longest-first, with the existing overlap guard. Coverage now spans every contract (e.g. requirements-discovery gains MECE; architecture-documentation gains arc42/C4/ADRs/Nygard/Pugh Matrix; code-quality gains SOLID/DRY/KISS/DDD). Still display-only; copy/download remain raw.
Two issues from CodeRabbit review of LLM-Coding#574: 1. getLocalizedField and the markdown export read i18n.currentLanguage, which does not exist — i18n exposes currentLang() as a method. The property was always undefined, so the contracts page (and its copy/ download) always fell back to English even in the German UI. Switch both reads to i18n.currentLang(). Verified in-browser: the German templates now render in DE. 2. renderContractsPageHandler used Promise.all, so a fetchAnchorsData rejection would abort the whole contracts page. Anchor titles only power the highlight aliases, so make it non-fatal via Promise.allSettled: render contracts whenever they load, attach anchor titles only if that fetch succeeded. Lint clean, 98/98 unit tests pass.
|
Both CodeRabbit comments were valid — fixed in 3779b70:
Lint clean, 98/98 unit tests pass. |
2 participants
Recovers work that was stranded when #573 merged at its first commit — the formatting/height/highlight follow-ups never reached
main, so the live contracts page shows the teaching contract as an unbroken wall of text. This re-lands them on top of current main, plus a small bug fix.Changes
contracts.json: the 'Explaining and Teaching' template was one paragraph and rendered as a wall of text; now paragraphs + a bulleted loop (EN + DE). Wording unchanged.contracts-page.js: each definition box ismax-h-64 overflow-y-auto, so long contracts (e.g. Architecture Documentation) scroll instead of dominating the page.anchors(no global false positives), longest-first, word-boundary. Display-only: the copy/download export uses the raw template, so highlighting never leaks into the export. Implemented with a match-and-slice approach (no placeholder sentinel).code-qualitycontract pointed at anchor iddry-principle(404); the real id isdry(feat: add DRY and Event Storming anchors #569). Chip now resolves.Verification (in-browser)
#/anchor/dry.Note: in-text highlighting currently matches only verbatim anchor titles (the chips below already list every declared anchor). Broadening coverage to short forms (e.g. 'MECE', 'arc42') is a separate, optional follow-up.
Summary by CodeRabbit
New Features
Documentation