Conversation
…nd content - Added dynamic language-based routing (`[lang=lang]`) for English and German. - Integrated localized content using `src/lib/content/`. - Implemented language toggling with URL updates, and canonical/hreflang metadata for SEO. - Created utility scripts: - `scripts/check-i18n-consistency.mjs` to validate bilingual content consistency. - Added localized components (`+layout.ts`, `+page.svelte`) and updated `README.md` with workflow details. - Refactored `loadPortfolioContent` and `loadCvContent` to support multilingual data.
- Introduced `isPortfolioHomeRoute` derived store to simplify repeated `pathname` checks. - Updated scroll and active section logic to use new store for cleaner conditionals. - Enhanced `goto` parameters by adding `noScroll` and `keepFocus` options for smoother navigation.
- Dynamically load experience and education content based on selected locale (EN/DE). - Refactor imports and locale handling to enhance bilingual support for CV content.
|
Warning Rate limit exceeded
⌛ 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)
📝 WalkthroughWalkthroughAdds full bilingual (en/de) support: locale-prefixed routes, localized Markdown content and typed i18n utilities, a centralized content loader with validation and an i18n consistency checker, UI wiring (language switcher, hreflang, localized downloads), tests/README updates, and CI/scripts revised to generate both EN and DE CV PDFs. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser
participant Router as SvelteKit Router
participant LangLayout as [lang] Layout
participant PageServer as PageServerLoad
participant ContentLoader as Content Loader
participant FS as File System
Browser->>Router: GET /de/cv/
Router->>LangLayout: extract lang="de"
LangLayout->>Router: provide locale context
Router->>PageServer: invoke load(params.lang="de")
PageServer->>ContentLoader: loadCvContent(locale="de")
ContentLoader->>FS: read src/lib/content/de/*.md
FS-->>ContentLoader: markdown contents
ContentLoader->>PageServer: structured CV data
PageServer-->>Router: return data + locale
Router->>Browser: render localized CV page
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/routes/+page.svelte (1)
101-325:⚠️ Potential issue | 🟠 MajorLocalization is still incomplete on the portfolio page.
Several user-facing strings remain hardcoded in English, so
/derenders mixed-language UI (notably the skills subheading, form labels, and dev-form alert text). These should also come fromui.home.*.💡 Suggested direction
- Basic knowledge + {ui.home.basicKnowledge} - <label for="name" class="text-sm font-medium">Name</label> + <label for="name" class="text-sm font-medium">{ui.home.formNameLabel}</label> - <label for="email" class="text-sm font-medium">Email</label> + <label for="email" class="text-sm font-medium">{ui.home.formEmailLabel}</label> - <label for="message" class="text-sm font-medium">Message</label> + <label for="message" class="text-sm font-medium">{ui.home.formMessageLabel}</label> - alert("Form submissions are handled by Netlify in deployed environments."); + alert(ui.home.formSubmissionDevNotice);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/`+page.svelte around lines 101 - 325, The page has several hardcoded UI strings (e.g. the "Basic knowledge" heading, form labels "Name"/"Email"/"Message", the dev-mode alert in the onsubmit handler, and social link texts "LinkedIn"/"GitHub") that must be localized; replace those literals with properties from the existing ui.home object (e.g. ui.home.basicKnowledge, ui.home.nameLabel, ui.home.emailLabel, ui.home.messageLabel, ui.home.devFormAlert, ui.home.linkedInLabel, ui.home.githubLabel) and ensure the onsubmit handler uses ui.home.devFormAlert instead of the hardcoded alert; update the template elements that reference familiarityItems, the contact form inputs, and the social link anchor contents accordingly so all user-facing strings come from ui.home.
🧹 Nitpick comments (13)
src/tests/page.svelte.spec.ts (1)
10-41: Add a DE-locale smoke assertion to cover the new bilingual behavior.This test now passes locale data but only validates English headings. Add a second render/assertion path for
locale: "de"to catch missing or regressed German copy in critical sections.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tests/page.svelte.spec.ts` around lines 10 - 41, Add a second render/assertion that passes locale: "de" (mirroring the existing render call that sets locale: "en") and assert the German headings exist using the same helpers (page.getByRole and expect.element); specifically, re-render with locale: "de" and add expectations for the German strings for the two critical headings (e.g., "Technische Expertise" and "Kontakt") to catch regressions in bilingual copy.scripts/generate-cv-pdf.mjs (1)
16-58: Consider reusing a single Chromium instance for all targets.Current behavior is correct, but launching one browser per locale adds avoidable overhead in CI.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/generate-cv-pdf.mjs` around lines 16 - 58, The code launches a new Chromium instance inside generatePdfForTarget for every target, which is inefficient; refactor to launch the browser once (e.g., in generateAllPdfs), pass the shared browser instance into generatePdfForTarget (add a parameter like browser or open pages inside generateAllPdfs), create a new page per target, call page.goto, page.pdf, and close each page after use, and finally close the browser once after Promise.all completes to avoid per-target browser launches and reduce CI overhead.package.json (1)
13-13: Optional: move the long local PDF command into a script file.This would make troubleshooting and future edits much easier than keeping all logic inline in
package.json.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` at line 13, The long inline npm script "generate:cv-pdf:local" should be moved into a standalone shell script (e.g., generate-cv-pdf-local.sh) to simplify editing and debugging; create a script that runs the same steps (bun run build, start preview via "bun run preview:ci" in background, trap to kill PID on exit, poll the two endpoints /en/cv/ and /de/cv/ up to 30s, fail with the same message if not ready, then run "bun run generate:cv-pdf") and make the package.json "generate:cv-pdf:local" entry a short call to that script (e.g., "bash scripts/generate-cv-pdf-local.sh"), ensuring executable bit and identical behavior to the original inline command.src/routes/[lang=lang]/+layout.ts (1)
3-6: Add JSDoc for the layoutloadfunction.This new function is missing inline API documentation.
Suggested fix
+/** + * Exposes the current locale from the matched `[lang=lang]` route segment. + */ export const load: LayoutLoad = ({ params }) => { return { lang: params.lang as "en" | "de" }; };As per coding guidelines,
**/*.{js,ts,svelte}: “Always write sensible JSDoc comments for functions and components.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/`[lang=lang]/+layout.ts around lines 3 - 6, Add a JSDoc block above the exported load function (export const load: LayoutLoad) that describes the purpose of the loader (returns the current language for the layout), documents the function parameter (params with params.lang possible values "en" | "de"), and documents the returned object shape (object with lang: "en" | "de"); reference the load function and the LayoutLoad type in the comment and keep the wording concise and actionable.src/tests/content.integrity.spec.ts (1)
12-12: Extend integrity assertions to both locales.Line 12 hardcodes
en, so this suite won’t catch missing/empty German content files.Suggested fix
-function readContentFile(fileName: string): string { - const filePath = path.resolve("src/lib/content/en", fileName); +function readContentFile(locale: "en" | "de", fileName: string): string { + const filePath = path.resolve("src/lib/content", locale, fileName); return fs.readFileSync(filePath, "utf-8"); } @@ - for (const fileName of requiredFiles) { - const content = readContentFile(fileName).trim(); - expect(content.length, `${fileName} should not be empty`).toBeGreaterThan(0); + for (const locale of ["en", "de"] as const) { + for (const fileName of requiredFiles) { + const content = readContentFile(locale, fileName).trim(); + expect(content.length, `${locale}/${fileName} should not be empty`).toBeGreaterThan(0); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tests/content.integrity.spec.ts` at line 12, The test currently hardcodes the English locale when resolving content files (see filePath and path.resolve("src/lib/content/en", fileName)), so it misses German files; update the test to iterate over both locales (e.g., ['en','de']) and run the same integrity assertions for each locale by building filePath with path.resolve("src/lib/content", locale, fileName) (or equivalent) and reusing fileName/assertion logic so both locales are checked. Ensure any test descriptions reflect the locale being checked so failures indicate which locale failed.src/routes/[lang=lang]/+page.server.ts (1)
12-15: Add JSDoc for the localizedloadfunction.Please document what this loader returns for the language-prefixed route.
As per coding guidelines: `**/*.{js,ts,svelte}`: Always write sensible JSDoc comments for functions and components.📝 Proposed update
+/** + * Loads portfolio content for the locale from the language route parameter. + */ export const load: PageServerLoad = ({ params }) => { const locale = params.lang as Locale; return loadPortfolioContent(locale); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/`[lang=lang]/+page.server.ts around lines 12 - 15, Add a JSDoc comment above the exported PageServerLoad function `load` describing its purpose, parameters and return value: explain that it accepts the route params object and extracts `params.lang` (typed as Locale), that it returns the result of `loadPortfolioContent(locale)` (describe the shape or type of that returned portfolio content), and mention any thrown errors or edge cases (e.g., invalid locale). Place the JSDoc immediately above the `export const load: PageServerLoad = ({ params }) => { ... }` declaration and use plain/concise descriptions for param and return tags.src/routes/+page.server.ts (1)
5-6: Add JSDoc toloadfor consistency with TS file conventions.The implementation is good; it just needs a brief function doc block.
As per coding guidelines: `**/*.{js,ts,svelte}`: Always write sensible JSDoc comments for functions and components.📝 Proposed update
+/** + * Loads portfolio content for the default locale. + */ export const load: PageServerLoad = () => { return loadPortfolioContent(DEFAULT_LOCALE); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/`+page.server.ts around lines 5 - 6, Add a concise JSDoc block above the exported load function describing its purpose (loads portfolio content for the default locale), the fact that it uses DEFAULT_LOCALE and calls loadPortfolioContent, and what it returns (the PageServerLoad data/Promise). Place the comment immediately above the symbol `load` (export const load: PageServerLoad) and mention `DEFAULT_LOCALE` and `loadPortfolioContent` in the description so the intent and return shape are clear and consistent with TS file conventions.src/routes/[lang=lang]/cv/+page.server.ts (1)
12-15: Add JSDoc for the CV localeloadfunction.Please add a short description for what data is loaded from the language route param.
As per coding guidelines: `**/*.{js,ts,svelte}`: Always write sensible JSDoc comments for functions and components.📝 Proposed update
+/** + * Loads CV content for the locale from the language route parameter. + */ export const load: PageServerLoad = ({ params }) => { const locale = params.lang as Locale; return loadCvContent(locale); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/`[lang=lang]/cv/+page.server.ts around lines 12 - 15, Add a JSDoc block immediately above the export const load function that documents that load is a PageServerLoad handler which reads the route parameter params.lang (cast to Locale) and returns the CV content by calling loadCvContent(locale); include a short description, the param description for params (or locale) and the return type/what is returned (CV content), referencing the function names load and loadCvContent and the Locale type so reviewers can find the implementation quickly.src/routes/cv/+page.server.ts (1)
5-6: Add JSDoc forloadto match repository standards.The function is concise and correct, but it still needs a short JSDoc block.
As per coding guidelines: `**/*.{js,ts,svelte}`: Always write sensible JSDoc comments for functions and components.📝 Proposed update
+/** + * Loads CV content for the default locale. + */ export const load: PageServerLoad = () => { return loadCvContent(DEFAULT_LOCALE); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/cv/`+page.server.ts around lines 5 - 6, Add a short JSDoc block above the exported load function to match repository standards: document that export const load: PageServerLoad returns the CV content by calling loadCvContent with DEFAULT_LOCALE, include a one-line description and an `@returns` tag describing the returned PageServerLoad data shape (or Promise) and optionally reference loadCvContent/DEFAULT_LOCALE for context; place the JSDoc immediately above the load declaration so it applies to the load symbol.src/routes/cv/+page.svelte (2)
1-14: Add a brief component-level doc comment for the locale/render contract.A short comment near Line 1 describing locale derivation and locale-specific Markdown component selection will make this component easier to maintain.
As per coding guidelines
**/*.{js,ts,svelte}: “Always write sensible JSDoc comments for functions and components.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/cv/`+page.svelte around lines 1 - 14, Add a short JSDoc-style component comment at the top of the file explaining that this Svelte page reads props via $props(), derives the current locale into the locale store using $derived (defaulting to "en"), uses t(locale) to build a UI translation store (ui), and conditionally selects the Markdown components Experience and Education based on locale === "de"; reference the relevant symbols: $props, locale, $derived, t, ui, Experience, Education, ExperienceDe, ExperienceEn, EducationDe, EducationEn so future maintainers understand the locale/render contract and markdown component selection.
25-37: Move download button labels intoui.cvtranslation keys.Line 29 and Line 36 are still hardcoded copy. Localizing via
uikeeps all UI text in one place and avoids drift.♻️ Proposed change
- CV (EN) + {ui.cv.downloadEn} @@ - Lebenslauf (DE) + {ui.cv.downloadDe}// i18n-copy.ts (outside this range) cv: { // ... downloadEn: "CV (EN)", downloadDe: "Lebenslauf (DE)" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/cv/`+page.svelte around lines 25 - 37, The two download anchor labels ("CV (EN)" and "Lebenslauf (DE)") are hardcoded; replace them with the corresponding i18n keys ui.cv.downloadEn and ui.cv.downloadDe from the shared copy (i18n-copy.ts) using the project's existing translation helper (the same callsite used elsewhere in this page/component), leaving the anchor attributes (href, target, rel, classes) intact and ensuring those keys are added under the cv block in i18n-copy.ts.src/lib/server/content-loader.ts (2)
195-198: Add inline intent comments for the regex parsing rules.Line 195 and Line 198 use non-trivial regex transformations; one-line inline comments will reduce maintenance risk.
As per coding guidelines
**/*.{js,ts,svelte}: “Use inline comments to explain complex logic, such as Regex patterns or intricate state management.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/server/content-loader.ts` around lines 195 - 198, Add brief inline intent comments explaining the two regex operations around the block that extracts front-matter-like key/value pairs: annotate the RegExp(/^- \*\*(.+?):\*\*\s*(.+)$/) used in the match = ... line to state it captures lines like "- **Key:** Value" and what each capture group represents, and annotate the .replaceAll(/\[(.*?)]\((.*?)\)/g, "$2") on fields[key] to explain it strips markdown links to keep only the URL (capture group 2). Place the comments immediately above or to the right of the expressions near the variables match and fields[key] so future readers understand intent without changing logic.
108-109: Decouple link parsing from hardcoded anchor text labels.Line 108 and Line 137 through Line 138 currently depend on specific link text. Parsing by markdown link shape is more locale-safe and less brittle.
♻️ Suggested direction
- const link = lines.find(l => l.includes("[View Certificate]"))?.match(/\(([^)]+)\)/)?.[1]; + const link = lines + .map(line => line.match(/\[[^\]]+]\(([^)]+)\)/)?.[1]) + .find((url): url is string => Boolean(url));Also applies to: 137-138
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/server/content-loader.ts` around lines 108 - 109, The current parsing for certificate links relies on hardcoded anchor text like "[View Certificate]" (the variable link assigned via lines.find(...)) which is brittle; change the logic in content-loader.ts to detect markdown links by shape instead of label by matching the markdown link pattern /\[.*?\]\(([^)]+)\)/ across the relevant lines (the code that assigns link on the first occurrence and the similar logic at lines ~137-138). Update both places to extract the URL from any markdown link match (optionally filtering by file-extension or context if multiple links exist) rather than checking for specific link text so parsing works regardless of locale/label changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/generate-cv-pdf.yml:
- Around line 73-78: The current for-loop over the two localized PDFs
(static/colin-moerbe-cv-en.pdf and static/colin-moerbe-cv-de.pdf) silently skips
missing files; change the loop (the shell block iterating "for file in ...") to
fail fast by checking existence and, if a file is missing, printing a clear
error message including the filename and exiting non‑zero (e.g., echo "Missing
artifact: $file" && exit 1) before attempting qpdf normalization; keep the qpdf
--linearize and mv steps for existing files as-is.
In `@src/lib/content/de/education.md`:
- Line 7: Replace the English/US decimal formatting in the degree note by
changing the grade in the string "B.Sc. Medien- und Kommunikationsinformatik
(Note: 2.0)" to use a German decimal comma, i.e. update "(Note: 2.0)" to "(Note:
2,0)" so the content in src/lib/content/de/education.md uses German numeric
formatting.
In `@src/lib/content/de/experience.md`:
- Around line 17-21: Two list items in experience.md ("Verbesserung der
CI/CD-Pipelines und nachhaltige Reduzierung wiederkehrender Produktionsprobleme
durch Refaktorisierung und verbesserte Protokollierungs- und
Überwachungsstrategien" and "Leitung einer unternehmensweiten Lerninitiative,
einschließlich Design und Entwicklung eines internen Tools zur Abbildung von
Lernpfaden und Kompetenzen") are missing terminal periods; update those bullet
lines to end with a period so punctuation is consistent with the surrounding
full-sentence bullets.
In `@src/lib/content/de/languages.md`:
- Line 4: The proficiency text "Englisch (Verhandlungssicher)" uses an
incorrectly capitalized adjective; update the string "Englisch
(Verhandlungssicher)" to "Englisch (verhandlungssicher)" by lowercasing the V in
the parenthetical adjective so the entry reads "Englisch (verhandlungssicher)".
In `@src/lib/content/de/profile.md`:
- Line 3: The German compound in profile.md uses "ereignis-basierten"; replace
it with the closed compound "ereignisbasierten" so the sentence reads
"...modularen, ereignisbasierten Systemen in Cloud-Umgebungen." Locate the
phrase in the file (occurrence of "ereignis-basierten") and update it to
"ereignisbasierten" preserving surrounding punctuation and capitalization.
In `@src/lib/content/de/projects.md`:
- Around line 9-12: Replace the awkward phrase "Statische Website-Hosting auf
Netlify." with a natural German phrasing such as "Statisches Website-Hosting auf
Netlify." and convert the two inline links
("[GitHub](https://github.com/Colin23/portfolio)" and "[Live
Demo](https://colinmoerbe.com)") into a simple bullet list for clearer
rendering; locate these exact strings in the projects.md content and update them
accordingly so the description reads smoothly and the links appear as separate
list items.
In `@src/lib/content/de/skills.md`:
- Line 1: Update the mixed-language heading "Technical Schwerpunkte" in the file
(the top-line heading string) to a fully German title such as "Technische
Schwerpunkte" so the locale is consistent; locate the heading text "Technical
Schwerpunkte" and replace it with "Technische Schwerpunkte".
In `@src/lib/i18n-copy.ts`:
- Around line 50-52: The German locale's nav object still uses the English label
"Tech Stack" for the key nav.skills; update that value to a German translation
(e.g., "Technologie-Stack" or "Technologie" or "Tech-Stack" per project style)
so nav.skills in the DE copy matches the other German labels.
In `@src/lib/server/content-loader.ts`:
- Around line 302-310: The code currently hardcodes English section titles in
familiarityTitles, causing non-English titles (e.g. German "Grundkenntnisse") to
be misclassified; update the logic in content-loader.ts so familiarityTitles is
populated from locale-aware data or a translation map instead of literal English
strings (or detect familiarity sections dynamically by a metadata flag on
groups). Specifically, replace the static familiarityTitles Set used by the
skills.filter calls that produce coreSkills and familiarity with a locale-driven
lookup (or a group property check) so group.title comparisons correctly
recognize translated titles like "Grundkenntnisse" when computing coreSkills and
familiarity.
- Around line 49-55: The code currently uses synchronous disk I/O (fs.existsSync
and fs.readFileSync) in resolveContentPath, loadPortfolioContent, and
loadCvContent which blocks the event loop; update these to use asynchronous APIs
(fs.promises.access or fs.promises.stat and fs.promises.readFile or non-blocking
helpers) and convert resolveContentPath to an async function (e.g.,
resolveContentPathAsync) so callers (loadPortfolioContent/loadCvContent) await
it, or alternatively implement a startup-time caching/prerender step to read all
markdown into memory and have the request handlers read from that cache; ensure
you update the unique symbols resolveContentPath, loadPortfolioContent, and
loadCvContent to use async/await and remove all readFileSync/existsSync usages.
In `@src/routes/`+layout.svelte:
- Around line 34-38: The localeAgnosticPath computation can produce a
double-slash for locale root routes (e.g. `/en/` -> `//`); update the return
logic in the localeAgnosticPath callback to build a base path first (const path
= `/${segments.join("/")}`) and then append a trailing slash only when
appropriate (e.g. return path === "/" ? "/" : page.url.pathname.endsWith("/") ?
`${path}/` : path), using the existing symbols localeAgnosticPath,
page.url.pathname, and segments to locate and replace the current return
expression.
- Around line 185-188: The observer initialization in onMount() should be moved
into the existing route-reactive $effect so setupObserver() runs whenever
isPortfolioHomeRoute changes; remove the setupObserver() call from onMount() and
instead call setupObserver() inside the $effect that watches
isPortfolioHomeRoute, and ensure you also call the corresponding cleanup (e.g.,
teardownObserver or returned cleanup function from setupObserver) inside that
effect when isPortfolioHomeRoute becomes false or when the effect re-runs to
avoid duplicate observers.
In `@src/routes/contact-success/`+page.svelte:
- Around line 20-25: The page title is not localized; add a dynamic <title> that
uses the same localized string as the body (e.g. ui.success.title) in
src/routes/contact-success/+page.svelte so the browser tab reflects the current
locale—wrap a <svelte:head><title>{ui.success.title}</title></svelte:head> (or
the appropriate localized key if you use a different field) near where locale/ui
are available to ensure the tab title matches the displayed text.
---
Outside diff comments:
In `@src/routes/`+page.svelte:
- Around line 101-325: The page has several hardcoded UI strings (e.g. the
"Basic knowledge" heading, form labels "Name"/"Email"/"Message", the dev-mode
alert in the onsubmit handler, and social link texts "LinkedIn"/"GitHub") that
must be localized; replace those literals with properties from the existing
ui.home object (e.g. ui.home.basicKnowledge, ui.home.nameLabel,
ui.home.emailLabel, ui.home.messageLabel, ui.home.devFormAlert,
ui.home.linkedInLabel, ui.home.githubLabel) and ensure the onsubmit handler uses
ui.home.devFormAlert instead of the hardcoded alert; update the template
elements that reference familiarityItems, the contact form inputs, and the
social link anchor contents accordingly so all user-facing strings come from
ui.home.
---
Nitpick comments:
In `@package.json`:
- Line 13: The long inline npm script "generate:cv-pdf:local" should be moved
into a standalone shell script (e.g., generate-cv-pdf-local.sh) to simplify
editing and debugging; create a script that runs the same steps (bun run build,
start preview via "bun run preview:ci" in background, trap to kill PID on exit,
poll the two endpoints /en/cv/ and /de/cv/ up to 30s, fail with the same message
if not ready, then run "bun run generate:cv-pdf") and make the package.json
"generate:cv-pdf:local" entry a short call to that script (e.g., "bash
scripts/generate-cv-pdf-local.sh"), ensuring executable bit and identical
behavior to the original inline command.
In `@scripts/generate-cv-pdf.mjs`:
- Around line 16-58: The code launches a new Chromium instance inside
generatePdfForTarget for every target, which is inefficient; refactor to launch
the browser once (e.g., in generateAllPdfs), pass the shared browser instance
into generatePdfForTarget (add a parameter like browser or open pages inside
generateAllPdfs), create a new page per target, call page.goto, page.pdf, and
close each page after use, and finally close the browser once after Promise.all
completes to avoid per-target browser launches and reduce CI overhead.
In `@src/lib/server/content-loader.ts`:
- Around line 195-198: Add brief inline intent comments explaining the two regex
operations around the block that extracts front-matter-like key/value pairs:
annotate the RegExp(/^- \*\*(.+?):\*\*\s*(.+)$/) used in the match = ... line to
state it captures lines like "- **Key:** Value" and what each capture group
represents, and annotate the .replaceAll(/\[(.*?)]\((.*?)\)/g, "$2") on
fields[key] to explain it strips markdown links to keep only the URL (capture
group 2). Place the comments immediately above or to the right of the
expressions near the variables match and fields[key] so future readers
understand intent without changing logic.
- Around line 108-109: The current parsing for certificate links relies on
hardcoded anchor text like "[View Certificate]" (the variable link assigned via
lines.find(...)) which is brittle; change the logic in content-loader.ts to
detect markdown links by shape instead of label by matching the markdown link
pattern /\[.*?\]\(([^)]+)\)/ across the relevant lines (the code that assigns
link on the first occurrence and the similar logic at lines ~137-138). Update
both places to extract the URL from any markdown link match (optionally
filtering by file-extension or context if multiple links exist) rather than
checking for specific link text so parsing works regardless of locale/label
changes.
In `@src/routes/`[lang=lang]/+layout.ts:
- Around line 3-6: Add a JSDoc block above the exported load function (export
const load: LayoutLoad) that describes the purpose of the loader (returns the
current language for the layout), documents the function parameter (params with
params.lang possible values "en" | "de"), and documents the returned object
shape (object with lang: "en" | "de"); reference the load function and the
LayoutLoad type in the comment and keep the wording concise and actionable.
In `@src/routes/`[lang=lang]/+page.server.ts:
- Around line 12-15: Add a JSDoc comment above the exported PageServerLoad
function `load` describing its purpose, parameters and return value: explain
that it accepts the route params object and extracts `params.lang` (typed as
Locale), that it returns the result of `loadPortfolioContent(locale)` (describe
the shape or type of that returned portfolio content), and mention any thrown
errors or edge cases (e.g., invalid locale). Place the JSDoc immediately above
the `export const load: PageServerLoad = ({ params }) => { ... }` declaration
and use plain/concise descriptions for param and return tags.
In `@src/routes/`[lang=lang]/cv/+page.server.ts:
- Around line 12-15: Add a JSDoc block immediately above the export const load
function that documents that load is a PageServerLoad handler which reads the
route parameter params.lang (cast to Locale) and returns the CV content by
calling loadCvContent(locale); include a short description, the param
description for params (or locale) and the return type/what is returned (CV
content), referencing the function names load and loadCvContent and the Locale
type so reviewers can find the implementation quickly.
In `@src/routes/`+page.server.ts:
- Around line 5-6: Add a concise JSDoc block above the exported load function
describing its purpose (loads portfolio content for the default locale), the
fact that it uses DEFAULT_LOCALE and calls loadPortfolioContent, and what it
returns (the PageServerLoad data/Promise). Place the comment immediately above
the symbol `load` (export const load: PageServerLoad) and mention
`DEFAULT_LOCALE` and `loadPortfolioContent` in the description so the intent and
return shape are clear and consistent with TS file conventions.
In `@src/routes/cv/`+page.server.ts:
- Around line 5-6: Add a short JSDoc block above the exported load function to
match repository standards: document that export const load: PageServerLoad
returns the CV content by calling loadCvContent with DEFAULT_LOCALE, include a
one-line description and an `@returns` tag describing the returned PageServerLoad
data shape (or Promise) and optionally reference loadCvContent/DEFAULT_LOCALE
for context; place the JSDoc immediately above the load declaration so it
applies to the load symbol.
In `@src/routes/cv/`+page.svelte:
- Around line 1-14: Add a short JSDoc-style component comment at the top of the
file explaining that this Svelte page reads props via $props(), derives the
current locale into the locale store using $derived (defaulting to "en"), uses
t(locale) to build a UI translation store (ui), and conditionally selects the
Markdown components Experience and Education based on locale === "de"; reference
the relevant symbols: $props, locale, $derived, t, ui, Experience, Education,
ExperienceDe, ExperienceEn, EducationDe, EducationEn so future maintainers
understand the locale/render contract and markdown component selection.
- Around line 25-37: The two download anchor labels ("CV (EN)" and "Lebenslauf
(DE)") are hardcoded; replace them with the corresponding i18n keys
ui.cv.downloadEn and ui.cv.downloadDe from the shared copy (i18n-copy.ts) using
the project's existing translation helper (the same callsite used elsewhere in
this page/component), leaving the anchor attributes (href, target, rel, classes)
intact and ensuring those keys are added under the cv block in i18n-copy.ts.
In `@src/tests/content.integrity.spec.ts`:
- Line 12: The test currently hardcodes the English locale when resolving
content files (see filePath and path.resolve("src/lib/content/en", fileName)),
so it misses German files; update the test to iterate over both locales (e.g.,
['en','de']) and run the same integrity assertions for each locale by building
filePath with path.resolve("src/lib/content", locale, fileName) (or equivalent)
and reusing fileName/assertion logic so both locales are checked. Ensure any
test descriptions reflect the locale being checked so failures indicate which
locale failed.
In `@src/tests/page.svelte.spec.ts`:
- Around line 10-41: Add a second render/assertion that passes locale: "de"
(mirroring the existing render call that sets locale: "en") and assert the
German headings exist using the same helpers (page.getByRole and
expect.element); specifically, re-render with locale: "de" and add expectations
for the German strings for the two critical headings (e.g., "Technische
Expertise" and "Kontakt") to catch regressions in bilingual copy.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
static/colin-moerbe-cv-de.pdfis excluded by!**/*.pdfstatic/colin-moerbe-cv-en.pdfis excluded by!**/*.pdf
📒 Files selected for processing (41)
.github/workflows/generate-cv-pdf.ymlREADME.mdpackage.jsonscripts/check-i18n-consistency.mjsscripts/generate-cv-pdf.mjssrc/lib/content/de/certificates.mdsrc/lib/content/de/contact.mdsrc/lib/content/de/education.mdsrc/lib/content/de/experience.mdsrc/lib/content/de/languages.mdsrc/lib/content/de/profile.mdsrc/lib/content/de/projects.mdsrc/lib/content/de/skills.mdsrc/lib/content/en/certificates.mdsrc/lib/content/en/contact.mdsrc/lib/content/en/education.mdsrc/lib/content/en/experience.mdsrc/lib/content/en/languages.mdsrc/lib/content/en/profile.mdsrc/lib/content/en/projects.mdsrc/lib/content/en/skills.mdsrc/lib/i18n-copy.tssrc/lib/i18n.tssrc/lib/server/content-loader.tssrc/params/lang.tssrc/routes/+layout.sveltesrc/routes/+page.server.tssrc/routes/+page.sveltesrc/routes/[lang=lang]/+layout.tssrc/routes/[lang=lang]/+page.server.tssrc/routes/[lang=lang]/+page.sveltesrc/routes/[lang=lang]/contact-success/+page.sveltesrc/routes/[lang=lang]/contact-success/+page.tssrc/routes/[lang=lang]/cv/+page.server.tssrc/routes/[lang=lang]/cv/+page.sveltesrc/routes/contact-success/+page.sveltesrc/routes/cv/+page.server.tssrc/routes/cv/+page.sveltesrc/tests/content.integrity.spec.tssrc/tests/content.svelte.spec.tssrc/tests/page.svelte.spec.ts
Greptile SummaryAdds comprehensive German language support with bilingual routing ( Key Changes:
Translation Quality: Architecture Notes: Confidence Score: 5/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
Start([User visits site]) --> CheckPath{Path has<br/>locale prefix?}
CheckPath -->|Yes: /de/... or /en/...| ValidateLocale[Validate locale via<br/>lang.ts matcher]
CheckPath -->|No: /| DefaultRoute[Root route loads<br/>DEFAULT_LOCALE content]
ValidateLocale --> LocalizedRoute[Route: /lang=lang/...]
DefaultRoute --> RootRoute[Route: /+page]
LocalizedRoute --> LoadContent[PageServerLoad calls<br/>loadPortfolioContent locale]
RootRoute --> LoadContentDefault[PageServerLoad calls<br/>loadPortfolioContent 'en']
LoadContent --> ResolveContent[content-loader.ts:<br/>resolveContentPathAsync]
LoadContentDefault --> ResolveContent
ResolveContent --> CheckFile{Localized file<br/>exists?}
CheckFile -->|Yes| ReadLocalized[Read from<br/>src/lib/content/locale/]
CheckFile -->|No| Fallback[Fallback to<br/>src/lib/content/]
ReadLocalized --> ParseContent[Parse markdown content]
Fallback --> ParseContent
ParseContent --> FilterSkills{Processing<br/>skills?}
FilterSkills -->|Yes| NormalizeFilter[Normalize titles to lowercase<br/>using FAMILIARITY_TITLES_BY_LOCALE]
FilterSkills -->|No| ReturnData[Return parsed data]
NormalizeFilter --> ReturnData
ReturnData --> RenderPage[Render page with<br/>localized UI strings]
RenderPage --> LayoutLocale[Layout extracts locale<br/>from URL path]
LayoutLocale --> HreflangTags[Generate hreflang tags<br/>for EN and DE]
HreflangTags --> LangSwitcher[Language switcher<br/>button shows other locale]
LangSwitcher --> End([Page rendered])
Last reviewed commit: 1106ea5 |
Additional Comments (1)
|
- Standardized locale-based content filtering using `normalizedFamiliarityTitles` and refactored title normalization logic. - Enhanced CV content loading with async functions (`resolveContentPathAsync`, `readContent`) for better performance and maintainability. - Adjusted multiple DE content files for consistency in grammar, punctuation, and terminology. - Updated CI workflow to handle missing CV artifacts more gracefully.
…lio home logic - Refined `localeAgnosticPath` computation to handle edge cases more effectively. - Moved scroll and observer setup logic into `$effect` for better reactivity and cleanup. - Updated `/contact-success/` page to use localized title via `ui.success.title`.
Colin23
force-pushed
the
support-german-language
branch
from
724ed11 to
4b586a9
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/lib/server/content-loader.ts (2)
118-119: Avoid English-only link label matching in parsers.Lines 118, 138, 147, and 148 rely on exact English link labels. In localized markdown, certificate/project URLs can be missed even when links exist.
♻️ Proposed refactor
- const link = lines.find(l => l.includes("[View Certificate]"))?.match(/\(([^)]+)\)/)?.[1]; + const link = lines.join("\n").match(/\[[^\]]+\]\(([^)]+)\)/)?.[1];- const linksStart = lines.findIndex(l => l.startsWith("[GitHub]") || l.startsWith("[Live Demo]")); + const linksStart = lines.findIndex(l => /\[[^\]]+\]\(([^)]+)\)/.test(l)); const rawContentLines = lines.slice(contentStart, linksStart > -1 ? linksStart : undefined); const rawContent = rawContentLines.join("\n").trim(); @@ - const github = lines.find(l => l.startsWith("[GitHub]"))?.match(/\(([^)]+)\)/)?.[1]; - const liveDemo = lines.find(l => l.startsWith("[Live Demo]"))?.match(/\(([^)]+)\)/)?.[1]; + const links = lines.flatMap(line => + [...line.matchAll(/\[[^\]]+\]\(([^)]+)\)/g)].map(match => match[1]) + ); + const github = links.find(url => /github\.com/i.test(url)); + const liveDemo = links.find(url => !/github\.com/i.test(url));Also applies to: 138-148
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/server/content-loader.ts` around lines 118 - 119, The current parser uses an English-only label match (e.g., lines.find(... includes("[View Certificate]") ) which misses localized link labels; replace these exact-label checks (the code that computes the variable link in content-loader.ts and the similar occurrences later) with a language-agnostic link extraction: parse all markdown links (e.g., capture all \[text\]\(url\) occurrences) or use a markdown-link extractor, then filter the extracted URLs by their path/hostname patterns (case-insensitive) that identify certificates/projects (for example match /certificate|cert|project|portfolio/ or your app’s known URL segments) to pick the correct URL; apply this same change to the other spots that rely on exact English link text.
205-219: Make contact key parsing locale-agnostic.Line 207 stores raw markdown keys, but Lines 212-218 only read English canonical names. This is brittle for translated labels (e.g.,
Rolle,Telefon,Standort).♻️ Proposed refactor
- const fields: Record<string, string> = {}; + const fields: Partial<Record<keyof Contact, string>> = {}; + const contactKeyAliases: Record<string, keyof Contact> = { + name: "name", + role: "role", + rolle: "role", + location: "location", + standort: "location", + phone: "phone", + telefon: "phone", + email: "email", + "e-mail": "email", + linkedin: "linkedin", + github: "github" + }; for (const line of lines) { const match = new RegExp(/^- \*\*(.+?):\*\*\s*(.+)$/).exec(line); if (!match) continue; - const key = match[1].toLowerCase(); - fields[key] = match[2].replaceAll(/\[(.*?)]\((.*?)\)/g, "$2").trim(); + const rawKey = match[1].toLowerCase().trim(); + const key = contactKeyAliases[rawKey]; + if (!key) continue; + fields[key] = match[2].replaceAll(/\[(.*?)]\((.*?)\)/g, "$2").trim(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/server/content-loader.ts` around lines 205 - 219, The parsing currently stores raw lowercased markdown keys into fields (via match and key) but the return reads only English keys (fields.name, fields.role, etc.), so make the parser locale-agnostic by normalizing label text to canonical keys before assigning: add a small mapping (e.g., labelMap) from known localized variants (e.g., "rolle","position","telefon","telefonnummer","standort", etc.) to canonical names ("role","role","phone","phone","location"), sanitize match[1] (trim, remove trailing colons/whitespace) and look up the canonical key (fallback to the cleaned key itself) and then assign fields[canonicalKey] = ...; finally leave the return as-is so fields.name/role/... resolve regardless of input language.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/routes/`+layout.svelte:
- Around line 34-40: The return in localeAgnosticPath uses a nested ternary
causing the no-nested-ternary warning; replace the nested ternary with a simple
conditional branch: compute path as currently done, then if path === "/" return
"/", else if page.url.pathname.endsWith("/") return `${path}/`, else return
path. Update the localeAgnosticPath definition (the $derived callback) to use
this explicit if/else logic so you remove the nested ternary while preserving
the same outputs for page.url.pathname and segments handling.
In `@src/routes/`+page.svelte:
- Around line 27-29: Add a JSDoc block above the normalizeTitle function
describing its purpose and parameters to satisfy jsdoc/require-jsdoc; include a
short description like "Normalize a title string by trimming whitespace and
converting to lower case", a `@param` {string} title description, and a `@returns`
{string} description indicating the normalized title; ensure the JSDoc is placed
immediately above function normalizeTitle(title: string): string so CI lint
passes.
---
Nitpick comments:
In `@src/lib/server/content-loader.ts`:
- Around line 118-119: The current parser uses an English-only label match
(e.g., lines.find(... includes("[View Certificate]") ) which misses localized
link labels; replace these exact-label checks (the code that computes the
variable link in content-loader.ts and the similar occurrences later) with a
language-agnostic link extraction: parse all markdown links (e.g., capture all
\[text\]\(url\) occurrences) or use a markdown-link extractor, then filter the
extracted URLs by their path/hostname patterns (case-insensitive) that identify
certificates/projects (for example match /certificate|cert|project|portfolio/ or
your app’s known URL segments) to pick the correct URL; apply this same change
to the other spots that rely on exact English link text.
- Around line 205-219: The parsing currently stores raw lowercased markdown keys
into fields (via match and key) but the return reads only English keys
(fields.name, fields.role, etc.), so make the parser locale-agnostic by
normalizing label text to canonical keys before assigning: add a small mapping
(e.g., labelMap) from known localized variants (e.g.,
"rolle","position","telefon","telefonnummer","standort", etc.) to canonical
names ("role","role","phone","phone","location"), sanitize match[1] (trim,
remove trailing colons/whitespace) and look up the canonical key (fallback to
the cleaned key itself) and then assign fields[canonicalKey] = ...; finally
leave the return as-is so fields.name/role/... resolve regardless of input
language.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
.github/workflows/generate-cv-pdf.ymlsrc/lib/content/de/education.mdsrc/lib/content/de/experience.mdsrc/lib/content/de/languages.mdsrc/lib/content/de/profile.mdsrc/lib/content/de/projects.mdsrc/lib/content/de/skills.mdsrc/lib/i18n-copy.tssrc/lib/server/content-loader.tssrc/routes/+layout.sveltesrc/routes/+page.sveltesrc/routes/contact-success/+page.svelte
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/generate-cv-pdf.yml
- src/routes/contact-success/+page.svelte
…t in hero section - Introduced `heroGreeting` key in EN and DE locales. - Updated hero section in `+page.svelte` to use dynamic `heroGreeting`.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…components - Moved `FAMILIARITY_TITLES_BY_LOCALE` to a dedicated module for better maintainability. - Updated `+page.svelte` and `content-loader.ts` to import shared constant.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/routes/+layout.svelte (2)
25-25: Redundant conditional for identical values.
localizedSiteTitlereturns the same string for both locales. Either simplify to a constant or implement actual localization if different titles are planned.Simplify if titles remain identical
- const localizedSiteTitle = $derived(locale === "de" ? "Colin Mörbe | Portfolio" : "Colin Mörbe | Portfolio"); + const localizedSiteTitle = "Colin Mörbe | Portfolio";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/`+layout.svelte at line 25, The conditional assigning localizedSiteTitle is redundant because both branches return the same string; replace the conditional expression in localizedSiteTitle with a simple constant assignment of "Colin Mörbe | Portfolio" or, if you intend real localization, wire localizedSiteTitle to a proper localization lookup (e.g., use your i18n/gettext function or a translations map) so the title is resolved via locale instead of the current ternary in localizedSiteTitle.
321-325: "Skip to content" link is not localized.This accessibility text should be translated for German users.
Suggested localization
<a href="#main-content" class="sr-only z-60 rounded bg-blue-600 px-3 py-2 text-white focus:not-sr-only focus:fixed focus:top-3 focus:left-3 focus-visible:ring-2 focus-visible:ring-blue-400"> - Skip to content + {ui.layout.skipToContent} </a>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/`+layout.svelte around lines 321 - 325, Replace the hardcoded "Skip to content" anchor text with a call to the app's i18n/localization helper (use the existing localization pattern in the project) and use a key like "skip_to_content"; update the anchor in +layout.svelte (the <a ...> element with class "sr-only z-60 ...") to render the localized string instead of the literal text and add the corresponding German translation entry (e.g., "skip_to_content": "Zum Inhalt springen") to the German locale file so German users see the translated label.src/routes/+page.svelte (1)
293-322: Form field labels are not localized.The contact form labels ("Name", "Email", "Message") remain hardcoded in English while other UI strings use
ui.home.*translations. Consider adding these to the i18n copy for consistency.Suggested localization
<div class="flex flex-col gap-1"> - <label for="name" class="text-sm font-medium">Name</label> + <label for="name" class="text-sm font-medium">{ui.home.formName}</label> ... </div> <div class="flex flex-col gap-1"> - <label for="email" class="text-sm font-medium">Email</label> + <label for="email" class="text-sm font-medium">{ui.home.formEmail}</label> ... </div> <div class="flex flex-col gap-1"> - <label for="message" class="text-sm font-medium">Message</label> + <label for="message" class="text-sm font-medium">{ui.home.formMessage}</label> ... </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/`+page.svelte around lines 293 - 322, Replace the hardcoded labels "Name", "Email", "Message" with translation keys and use the existing i18n lookup in the Svelte template; add keys ui.home.name, ui.home.email, and ui.home.message to your locales (all language JSON/YAML files) and then update the labels in +page.svelte to render the translations for the inputs with id/name "name", "email", and "message" (e.g., use your project's translation helper like $t('ui.home.name') or $_('ui.home.name') depending on your i18n setup).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/routes/contact-success/`+page.svelte:
- Around line 1-12: Extract the anonymous pathname parsing into a named function
(e.g., resolveLocaleFromPath or getLocaleFromPath) and add a JSDoc comment
describing its purpose, params and return type (Locale) so it conforms to
project guidelines; then replace the inline arrow in const locale =
$derived.by<Locale>(() => { ... }) with a call to that named function and ensure
const ui = $derived(t(locale)) still uses the derived locale. Include references
to the function name (resolveLocaleFromPath/getLocaleFromPath), the derived
variable locale, and ui so reviewers can locate the changes.
---
Nitpick comments:
In `@src/routes/`+layout.svelte:
- Line 25: The conditional assigning localizedSiteTitle is redundant because
both branches return the same string; replace the conditional expression in
localizedSiteTitle with a simple constant assignment of "Colin Mörbe |
Portfolio" or, if you intend real localization, wire localizedSiteTitle to a
proper localization lookup (e.g., use your i18n/gettext function or a
translations map) so the title is resolved via locale instead of the current
ternary in localizedSiteTitle.
- Around line 321-325: Replace the hardcoded "Skip to content" anchor text with
a call to the app's i18n/localization helper (use the existing localization
pattern in the project) and use a key like "skip_to_content"; update the anchor
in +layout.svelte (the <a ...> element with class "sr-only z-60 ...") to render
the localized string instead of the literal text and add the corresponding
German translation entry (e.g., "skip_to_content": "Zum Inhalt springen") to the
German locale file so German users see the translated label.
In `@src/routes/`+page.svelte:
- Around line 293-322: Replace the hardcoded labels "Name", "Email", "Message"
with translation keys and use the existing i18n lookup in the Svelte template;
add keys ui.home.name, ui.home.email, and ui.home.message to your locales (all
language JSON/YAML files) and then update the labels in +page.svelte to render
the translations for the inputs with id/name "name", "email", and "message"
(e.g., use your project's translation helper like $t('ui.home.name') or
$_('ui.home.name') depending on your i18n setup).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/lib/familiarity-titles.tssrc/lib/i18n-copy.tssrc/lib/server/content-loader.tssrc/routes/+layout.sveltesrc/routes/+page.sveltesrc/routes/contact-success/+page.svelte
🚧 Files skipped from review as they are similar to previous changes (2)
- src/lib/server/content-loader.ts
- src/lib/i18n-copy.ts
…bility and reuse - Simplified locale resolution logic by introducing `resolveLocaleFromPath`. - Replaced inline locale handling with the new helper function in `/contact-success/` page.
- Added `ctaLabel` prop to `CertificateCard` with default value for improved flexibility. - Introduced new localization keys (`skipToContent`, `viewCertificate`, `name`, `email`, `message`) for enhanced i18n support. - Updated multiple components to use localized strings (`+layout.svelte`, `+page.svelte`). - Refactored `content-loader.ts` to improve link extraction logic for better robustness.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/lib/components/CertificateCard.svelte (1)
1-10: Add a brief component/props JSDoc blockThis component currently has no JSDoc/TSDoc despite updated public props. Add a short doc block describing purpose and prop semantics (
title,info,link,ctaLabel).As per coding guidelines, "
**/*.{js,ts,svelte}: Always write sensible JSDoc comments for functions and components."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/components/CertificateCard.svelte` around lines 1 - 10, Add a short JSDoc/TSDoc block at the top of the CertificateCard.svelte component describing the component's purpose and each prop in the Props interface (title, info, link, ctaLabel) so it follows the project guideline to always include JSDoc for components; update the block near the existing interface/props declaration (reference: Props interface and the destructured const { title, info, link, ctaLabel = "View Certificate" }) and include one-line descriptions for title (certificate title), info (supporting text), link (optional URL to the certificate), and ctaLabel (optional call-to-action label).src/routes/+page.svelte (1)
74-76: Avoid duplicate/static page title overriding localized layout metadataThe hardcoded
<title>here can conflict with the localized title/meta already managed in+layout.svelte. Prefer a single source of truth in layout for SEO/i18n consistency.Proposed fix
-<svelte:head> - <title>Colin Mörbe | Portfolio</title> -</svelte:head>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/`+page.svelte around lines 74 - 76, The page currently hardcodes a <svelte:head> block with a static <title> which conflicts with the localized metadata provided by +layout.svelte; remove the <svelte:head> / <title> from src/routes/+page.svelte (or make it conditional) so the layout remains the single source of truth for page title/metadata, and if you need page-specific titles instead expose/consume a prop or exported value from +layout.svelte to set the localized title rather than overriding it directly in +page.svelte.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/components/CertificateCard.svelte`:
- Around line 6-9: The component currently hardcodes a fallback ctaLabel = "View
Certificate" in the Props destructure; remove this hardcoded English fallback by
making ctaLabel a required prop (remove the default in the Props destructure and
update Props type to make ctaLabel non-optional) or resolve the fallback from
the i18n/content layer (e.g., read the CTA string from the certificate Markdown
or i18n helper instead of embedding "View Certificate"). Update any consumers of
CertificateCard to pass ctaLabel where missing, and ensure the Props interface
and the destructuring (ctaLabel) are consistent with the new required source.
In `@src/routes/`+layout.svelte:
- Around line 242-248: Update the stale JSDoc above the downloadCV function to
reflect its current behavior: state that downloadCV selects a locale-specific
PDF (using the file variable determined by locale) and opens it in a new browser
tab via window.open with "_blank" and "noopener,noreferrer"; remove any mention
of a hidden iframe or printing and ensure the docblock references the function
name downloadCV and the locale-based file selection.
In `@src/routes/`+page.svelte:
- Around line 20-31: The code casts data.locale to Locale without runtime
validation, which can cause FAMILIARITY_TITLES_BY_LOCALE[locale] to throw;
update the locale derivation so it validates data.locale at runtime (e.g., check
that data.locale is a key in FAMILIARITY_TITLES_BY_LOCALE or use an isLocale
type-guard) and fall back to "en" when invalid, then use that validated value
when creating locale (the variable used by $derived) and when computing
normalizedFamiliarityTitles; keep normalizeTitle as-is but ensure
normalizedFamiliarityTitles references the validated locale rather than the
unchecked data.locale.
---
Nitpick comments:
In `@src/lib/components/CertificateCard.svelte`:
- Around line 1-10: Add a short JSDoc/TSDoc block at the top of the
CertificateCard.svelte component describing the component's purpose and each
prop in the Props interface (title, info, link, ctaLabel) so it follows the
project guideline to always include JSDoc for components; update the block near
the existing interface/props declaration (reference: Props interface and the
destructured const { title, info, link, ctaLabel = "View Certificate" }) and
include one-line descriptions for title (certificate title), info (supporting
text), link (optional URL to the certificate), and ctaLabel (optional
call-to-action label).
In `@src/routes/`+page.svelte:
- Around line 74-76: The page currently hardcodes a <svelte:head> block with a
static <title> which conflicts with the localized metadata provided by
+layout.svelte; remove the <svelte:head> / <title> from src/routes/+page.svelte
(or make it conditional) so the layout remains the single source of truth for
page title/metadata, and if you need page-specific titles instead expose/consume
a prop or exported value from +layout.svelte to set the localized title rather
than overriding it directly in +page.svelte.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/lib/components/CertificateCard.sveltesrc/lib/i18n-copy.tssrc/lib/server/content-loader.tssrc/routes/+layout.sveltesrc/routes/+page.svelte
🚧 Files skipped from review as they are similar to previous changes (2)
- src/lib/i18n-copy.ts
- src/lib/server/content-loader.ts
- Made `ctaLabel` a required prop in `CertificateCard` for clearer prop expectations. - Enhanced locale resolution logic in `+page.svelte` by introducing fallback support for invalid locales. - Updated CV download logic in `+layout.svelte` to open locale-specific PDFs in a new tab.
Owner
Author
|
@coderabbitai resume |
✅ Actions performedReviews resumed. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/routes/+layout.svelte (1)
25-30: Centralize meta description text in the i18n copy object.This locale ternary duplicates translation ownership inside layout while the rest of UI copy is already centralized via
t(locale). Moving this text into the same copy source reduces drift and translation misses.Proposed refactor
- const localizedSiteDescription = $derived( - locale === "de" - ? "Backend Software Engineer Portfolio mit Projekten, Berufserfahrung, Tech-Stack, Zertifikaten und downloadbarem Lebenslauf." - : "Backend Software Engineer portfolio with projects, professional experience, technical skills, certificates, and a downloadable CV." - ); + const localizedSiteDescription = $derived(ui.layout.siteDescription);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/`+layout.svelte around lines 25 - 30, The meta description string is hard-coded with a locale ternary in localizedSiteDescription; move both German and English description texts into the existing i18n copy object used by t(locale) (e.g., add keys like site.description.de and site.description.en), then replace the ternary with a lookup through t(locale) (use the same t and locale variables already in scope) so localizedSiteDescription obtains its value from the centralized copy instead of inline conditional strings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/routes/`+layout.svelte:
- Around line 25-30: The meta description string is hard-coded with a locale
ternary in localizedSiteDescription; move both German and English description
texts into the existing i18n copy object used by t(locale) (e.g., add keys like
site.description.de and site.description.en), then replace the ternary with a
lookup through t(locale) (use the same t and locale variables already in scope)
so localizedSiteDescription obtains its value from the centralized copy instead
of inline conditional strings.
- Added `site.description` key in EN and DE locales for better metadata handling. - Updated `+layout.svelte` to use localized site descriptions dynamically. - Refactored `content-loader.ts` to normalize familiarity titles for improved filtering accuracy.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/lib/server/content-loader.ts (1)
107-122: Consider defensive handling for empty sections.If a malformed Markdown file has an empty section after
##,lines[0]at line 111 could beundefined, and calling.trim()would throw. While the content is controlled, a defensive check would improve robustness.Suggested defensive fix
function parseCertificates(content: string): CertificateEntry[] { const sections = content.split(/^## /m).slice(1); return sections.map(section => { const lines = section.split("\n"); - const title = lines[0].trim(); + const title = lines[0]?.trim() ?? ""; const infoLine = lines.find(l => l.startsWith("_") && l.endsWith("_"));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/server/content-loader.ts` around lines 107 - 122, In parseCertificates, empty sections produced by splitting on /^## /m can yield lines[0] === undefined, so guard against that by skipping empty sections or setting safe defaults: check section.trim() and continue/filter out empty sections before mapping, or within the map ensure const title = (lines[0] ?? "").trim(); likewise protect infoLine and link extraction by only processing when lines.length > 0; update parseCertificates to return valid CertificateEntry objects (or omit entries) when section content is missing to avoid calling .trim() on undefined.src/routes/+layout.svelte (1)
13-16: Consider using the shared locale validation.The locale derivation hardcodes the check for
"de"and defaults to"en". While functional, consider using theisLocalehelper from$lib/i18nfor consistency with the route parameter matcher, which would make the code more maintainable if additional locales are added later.Suggested refactor
+ import { getAltLocale, isLocale, type Locale } from "$lib/i18n"; - import { getAltLocale, type Locale } from "$lib/i18n"; ... const locale = $derived.by<Locale>(() => { const first = page.url.pathname.split("/").filter(Boolean)[0]; - return first === "de" ? "de" : "en"; + return isLocale(first) ? first : "en"; });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/`+layout.svelte around lines 13 - 16, The locale derivation currently hardcodes "de" vs "en" in the const locale = $derived.by<Locale>(...) block; replace that manual check by importing and using the isLocale helper (from $lib/i18n) to validate the first path segment (extracted from page.url.pathname) and return the validated Locale (or fallback to "en"). Update the derived function to call isLocale(first) and return first when true else "en", keeping the Locale type and $derived.by<Locale>() wrapper intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/lib/server/content-loader.ts`:
- Around line 107-122: In parseCertificates, empty sections produced by
splitting on /^## /m can yield lines[0] === undefined, so guard against that by
skipping empty sections or setting safe defaults: check section.trim() and
continue/filter out empty sections before mapping, or within the map ensure
const title = (lines[0] ?? "").trim(); likewise protect infoLine and link
extraction by only processing when lines.length > 0; update parseCertificates to
return valid CertificateEntry objects (or omit entries) when section content is
missing to avoid calling .trim() on undefined.
In `@src/routes/`+layout.svelte:
- Around line 13-16: The locale derivation currently hardcodes "de" vs "en" in
the const locale = $derived.by<Locale>(...) block; replace that manual check by
importing and using the isLocale helper (from $lib/i18n) to validate the first
path segment (extracted from page.url.pathname) and return the validated Locale
(or fallback to "en"). Update the derived function to call isLocale(first) and
return first when true else "en", keeping the Locale type and
$derived.by<Locale>() wrapper intact.
- Updated `+layout.svelte` to validate locale using `isLocale` for better fallback logic. - Refined `content-loader.ts` to filter out empty sections and trim lines in certificate parsing for improved data robustness.
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
1 participant
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.
🚀 Pull Request
🔗 Linked Issue
Closes #[Issue Number]
📝 Description
🛠️ Changes Made
🧪 Testing
📸 Screenshots (if applicable)
🔍 Additional Notes
✅ Checklist
👥 Reviewer Checklist
Summary by CodeRabbit
New Features
Bug Fixes / Quality
Documentation
Tests
Chores