W3: Evidence Wall + 7 projects + L-001 Lab + Cmd+K#9
Merged
Conversation
…trip for Lab Reports
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (8)
📝 WalkthroughWalkthroughThis PR adds an evidence, labs, and projects portal with Pagefind-powered Cmd/Ctrl+K command palette, a D3 chart island and helpers, many reusable Astro UI components, collection-aware layouts and dynamic routes, and extensive MDX documentation and lab/project pages. ChangesInteractive Discovery & Visualization
Reusable Content Components
Content Collections & Page Routing
Evidence & Project Documentation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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: 16
♻️ Duplicate comments (3)
src/layouts/LabReportLayout.astro (1)
32-34:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftCritical: Same ID mismatch issue as EvidenceLayout.
Line 34 uses
entry.data.idto retrieve reverse links, butsrc/pages/labs/[...slug].astroline 8 usesentry.idas the slug param. This creates the same inconsistency flagged inEvidenceLayout.astro: ifentry.iddiffers fromentry.data.id, cross-reference links will break.Align the identifier used in the route and the reverse-link lookup.
🤖 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 `@src/layouts/LabReportLayout.astro` around lines 32 - 34, The reverse-link lookup in LabReportLayout.astro is using the wrong identifier; replace the use of the nested data id with the same identifier the route expects so IDs match across routing and reverse-linking. Specifically, when building reverse links with buildReverseIndex/entriesToContentEntries and calling getReverseLinks (the call that currently passes d.data.id), pass the top-level entry.id (the same slug used in src/pages/labs/[...slug].astro) so getReverseLinks and the route use the same identifier consistently.src/pages/evidence/[...slug].astro (1)
8-8:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftVerify slug consistency with EvidenceLayout reverse-link lookup.
This route uses
entry.idas the slug param (the collection entry's file-based identifier), butEvidenceLayout.astroline 33 retrieves reverse links usingentry.data.id(a frontmatter field). If these identifiers differ, "Cited by" links will point to non-existent routes.Ensure both use the same identifier. Recommend changing
EvidenceLayout.astroline 33 to useentry.idto match this route.🤖 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 `@src/pages/evidence/`[...slug].astro at line 8, The route sets the page slug from the collection entry's file-based identifier via params: { slug: entry.id }, but EvidenceLayout.astro currently looks up reverse links using the frontmatter id (entry.data.id), which can mismatch; update EvidenceLayout.astro to use entry.id (the file-based identifier) when building reverse-link URLs and lookups (replace usages of entry.data.id with entry.id) so the "Cited by" links resolve to the same slug generation as this route.src/pages/labs/[...slug].astro (1)
8-8:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftVerify slug consistency with LabReportLayout reverse-link lookup.
This route uses
entry.idas the slug param, butLabReportLayout.astroline 34 retrieves reverse links usingentry.data.id. Same mismatch issue as in the evidence collection—ensure both use the same identifier to avoid broken cross-references.🤖 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 `@src/pages/labs/`[...slug].astro at line 8, The slug param used in the route is set to entry.id but LabReportLayout.astro's reverse-link lookup expects entry.data.id, causing mismatched identifiers; update the route's params (where it's currently "params: { slug: entry.id }") to use the same identifier as LabReportLayout (e.g., entry.data.id) or alternatively change LabReportLayout to read entry.id so both places (the route and the reverse-link lookup in LabReportLayout.astro) use the exact same property name.
🧹 Nitpick comments (8)
src/components/universal/SectionAnchors.astro (1)
61-61: 💤 Low valueHardcoded sticky position assumes fixed header height.
The
top: 72pxvalue assumes a specific header height. If the site header changes size (e.g., responsive design, different layouts), this component will misalign. Consider using a CSS custom property likevar(--header-height, 72px)for maintainability.♻️ Proposed change to use design token
.section-anchors { position: sticky; - top: 72px; + top: var(--header-height, 72px); margin: var(--space-5) 0;🤖 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 `@src/components/universal/SectionAnchors.astro` at line 61, The sticky positioning in SectionAnchors.astro currently uses a hardcoded top: 72px which will break if header height changes; replace the literal with a CSS custom property (e.g., top: var(--header-height, 72px)) in the SectionAnchors.astro stylesheet and document/update the global/header component to set --header-height (or rely on the fallback 72px) so the anchor offset stays correct across layouts and responsive sizes.src/components/universal/MethodBlock.astro (1)
27-28: 💤 Low valueConsider removing unnecessary CSS overrides.
These declarations reset
padding-bottomandborder-bottomto0, but no initial values are defined for these properties in the component. Unless these are defensive overrides against inherited global styles, they may be unnecessary.♻️ Proposed cleanup
margin: 0 0 var(--space-3); font-weight: 500; - padding-bottom: 0; - border-bottom: 0; }🤖 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 `@src/components/universal/MethodBlock.astro` around lines 27 - 28, Remove the unnecessary CSS overrides "padding-bottom: 0;" and "border-bottom: 0;" from the MethodBlock.astro style block (or, if they are intended as defensive overrides against global styles, replace them with a more specific selector or add an inline comment explaining the intent) so the component does not reset properties unnecessarily; target the declarations shown ("padding-bottom: 0;" and "border-bottom: 0;") in MethodBlock.astro and either delete them or document/strengthen them with a selector specificity change.src/pages/evidence/index.astro (1)
14-18: ⚡ Quick winConsider deriving
heroStatsfrom actual evidence data.The hardcoded stats (3.4pp, 2.4×, 2.2×) could drift out of sync as evidence entries are added or updated. If these values represent aggregate metrics across the
evidencecollection, consider computing them dynamically fromsortedentries to ensure accuracy.🤖 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 `@src/pages/evidence/index.astro` around lines 14 - 18, The heroStats array is hardcoded and should be derived from the evidence data: compute the three aggregate metrics from the existing sorted evidence entries (use the variable sorted or the evidence collection used in this file) instead of fixed literals; update the place where heroStats is defined (replace the static const heroStats declaration) to calculate values (e.g., compute workflow improvement in percentage points, cost ratio, and latency ratio) from sorted entries, format them into the same string shapes ('3.4pp', '2.4×', '2.2×'), and preserve the color/label structure so the rest of the page (references to heroStats) continues to work. Ensure numeric computations handle empty arrays and types match the expected shape (remove or adjust the 'as const' if necessary) and keep the identifier heroStats for downstream usage.src/layouts/EvidenceLayout.astro (2)
32-32: ⚡ Quick winAvoid
as anycast—preserves type safety.The
as anycast bypasses TypeScript's type checking forallEntries. This could hide type mismatches when collections have different schemas. Consider typingallEntriesexplicitly or using a union type to maintain type safety while accommodating multiple collection types.♻️ Refactor to preserve types
-const allEntries = [...chapters, ...fieldNotes, ...recipes, ...labs, ...evidence, ...projects]; -const reverseIndex = buildReverseIndex(entriesToContentEntries(allEntries as any)); +const allEntries: Array<CollectionEntry<'chapters'> | CollectionEntry<'fieldNotes'> | CollectionEntry<'recipes'> | CollectionEntry<'labs'> | CollectionEntry<'evidence'> | CollectionEntry<'projects'>> = [...chapters, ...fieldNotes, ...recipes, ...labs, ...evidence, ...projects]; +const reverseIndex = buildReverseIndex(entriesToContentEntries(allEntries));Or adjust
entriesToContentEntriesto accept a union type if the cast is currently required.🤖 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 `@src/layouts/EvidenceLayout.astro` at line 32, The line using an unsafe cast should be changed to preserve type safety: update the type of allEntries (where it's defined) to the correct collection type or a union of allowed collection types and then call entriesToContentEntries(allEntries) without casting, or alternatively update entriesToContentEntries to accept that union type; ensure callers like buildReverseIndex(reverseIndex) still receive the expected ContentEntry type so replace "const reverseIndex = buildReverseIndex(entriesToContentEntries(allEntries as any));" with a call that uses the properly typed allEntries and/or an updated entriesToContentEntries signature (refer to reverseIndex, buildReverseIndex, entriesToContentEntries, and allEntries to locate and adjust the code).
35-43: ⚡ Quick winExtract duplicated
collectionRoutemapping to a shared module.The
collectionRouteobject is duplicated identically inEvidenceLayout.astro,LabReportLayout.astro, andProjectLayout.astro. Extracting this to a shared constant in~/lib/routes.tsor similar will eliminate duplication and ensure consistency if routes change.♻️ Proposed extraction
Create
src/lib/routes.ts:export const collectionRoute: Record<string, string> = { chapters: '/agentic-ai/book', fieldNotes: '/agentic-ai/field-notes', recipes: '/agentic-ai/recipes', projects: '/agentic-ai/projects', evidence: '/agentic-ai/evidence', labs: '/agentic-ai/labs', patterns: '/agentic-ai/patterns', };Then import in each layout:
+import { collectionRoute } from '~/lib/routes'; -const collectionRoute: Record<string, string> = { - chapters: '/agentic-ai/book', - ... -};🤖 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 `@src/layouts/EvidenceLayout.astro` around lines 35 - 43, Duplicate collectionRoute objects in EvidenceLayout.astro, LabReportLayout.astro and ProjectLayout.astro should be extracted to a single shared module: create a new module that exports export const collectionRoute: Record<string,string> = { chapters: '/agentic-ai/book', fieldNotes: '/agentic-ai/field-notes', recipes: '/agentic-ai/recipes', projects: '/agentic-ai/projects', evidence: '/agentic-ai/evidence', labs: '/agentic-ai/labs', patterns: '/agentic-ai/patterns' }; then remove the local collectionRoute declarations in EvidenceLayout.astro, LabReportLayout.astro and ProjectLayout.astro and import the shared collectionRoute constant from the new module in each layout (replace the local variable with the imported symbol). Ensure the export name is exactly collectionRoute so existing references in those layouts continue to work.src/components/islands/D3Chart.svelte (2)
43-44: 💤 Low valueClarify precedence when both
dataandsrcare provided.The condition
if (!src || loadedData) return;means ifdatais provided (initializingloadedDataon line 41),srcwill never be fetched. This precedence is reasonable but not documented. Consider adding a comment or validating that only one prop is provided.🤖 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 `@src/components/islands/D3Chart.svelte` around lines 43 - 44, The effect block using $effect with the condition "if (!src || loadedData) return;" hides that when both data (which sets loadedData) and src are provided, src fetch is skipped; update D3Chart.svelte by either adding a clear comment next to the $effect explaining that explicit data prop takes precedence over src, or add a runtime validation that ensures only one of the two props is supplied (e.g., check data and src in onMount or at top of the component and throw/log a warning) so callers know to provide only one; reference symbols: $effect, src, loadedData, and the data prop to locate where to add the comment or validation.
98-98: 💤 Low valueConsider using the
colorForValuehelper.The chart uses inline color expressions (
p.accent ? '#9b4a3f' : '#1a1a1a') whiled3-helpers.tsexports acolorForValuehelper for the same purpose. Using the helper would centralize color logic, though the current implementation is clear and functional.Also applies to: 111-112
🤖 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 `@src/components/islands/D3Chart.svelte` at line 98, Replace the inline ternary color expressions with the centralized helper: import colorForValue from d3-helpers and change fill={p.accent ? '#9b4a3f' : '#1a1a1a'} to use fill={colorForValue(p.accent)} (and do the same for the other occurrences mentioned); this consolidates color logic into the colorForValue function used by the chart rendering code in D3Chart.svelte.package.json (1)
21-23: ⚡ Quick winRemove redundant D3 subpackage dependencies.
The full
d3package already includesd3-arrayandd3-scaleas transitive dependencies. Sincesrc/lib/d3-helpers.tsimports from'd3'directly (not from the subpackages), these explicit subpackage entries are redundant and may cause version conflicts or confusion.♻️ Proposed fix
"dependencies": { "@astrojs/mdx": "^4.3.0", "@astrojs/svelte": "^7.2.0", "astro": "^5.7.0", "d3": "^7.9.0", - "d3-array": "^3.2.4", - "d3-scale": "^4.0.2", "motion": "^12.38.0" },🤖 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 `@package.json` around lines 21 - 23, The package.json lists redundant d3 subpackages (d3-array, d3-scale) while your code imports from the umbrella 'd3' (see src/lib/d3-helpers.ts), so remove the explicit "d3-array" and "d3-scale" entries from package.json to avoid duplicate/ conflicting versions, then reinstall (npm/yarn install) to update the lockfile/lock entries; verify imports in d3-helpers.ts still import from 'd3' and run the test/build to confirm no breakage.
🤖 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 `@src/components/islands/CommandPalette.svelte`:
- Around line 72-74: The ArrowDown handler can set activeIdx to -1 when
results.length is 0; fix by clamping the upper bound to at least 0 so activeIdx
never becomes negative. In the ArrowDown branch (in CommandPalette.svelte)
compute a safe maxIndex = Math.max(results.length - 1, 0) and then set activeIdx
= Math.min(activeIdx + 1, maxIndex) so activeIdx is always >= 0 even when
results is empty.
In `@src/components/islands/D3Chart.svelte`:
- Around line 46-51: The fetch handler should validate the HTTP response before
calling r.json(): in the promise chain that starts with fetch(src) (the block
that sets loadedData when not aborted), check r.ok and if false throw or reject
with an Error containing r.status and r.statusText (and src) so the .catch sees
a meaningful message; only call r.json() when r.ok is true and assign to
loadedData if not aborted, and ensure the catch logs the HTTP status information
alongside the existing error handling.
In `@src/components/universal/HypothesisBlock.astro`:
- Line 9: The HypothesisBlock component currently wraps the slotted content in a
<p> which can break when the slot contains block-level elements; change the
wrapper in HypothesisBlock (look for the element with class
hypothesis-block__body in src/components/universal/HypothesisBlock.astro) from a
paragraph to a block container (e.g., a div) so the <slot /> can accept both
inline and block content without producing invalid HTML.
In `@src/components/universal/ProductCardHero.astro`:
- Line 14: The default props for caseStudy and failures currently create
always-present anchors that can point to nonexistent IDs; change ProductCardHero
to make all four quadrants consistently optional by removing the defaults in the
destructure (replace const { runIt, caseStudy = '#case-study', failures =
'#failures', codeRepo } with no defaults) and update the template rendering
logic that uses runIt, caseStudy, failures, and codeRepo so each quadrant is
rendered only if its corresponding prop is truthy (i.e., wrap the anchor/button
output for caseStudy and failures in the same conditional checks used for runIt
and codeRepo).
In `@src/components/universal/ReproduceStrip.astro`:
- Around line 17-18: Update the two external anchor elements that use
target="_blank" (the ones rendering repo and data links with class
reproduce-strip__link) to include rel="noopener noreferrer" so they both retain
tab-nabbing protection and also omit referrer information; locate the anchor
tags that reference the repo and data props in the ReproduceStrip component and
add "noreferrer" to the existing rel attribute (or replace rel value with
"noopener noreferrer" if currently just "noopener").
In `@src/components/universal/SectionAnchors.astro`:
- Around line 36-43: The IntersectionObserver callback currently clears all
active states for every entry and then applies the last-processed one, causing
race conditions; instead, collect all entries from the callback, filter to the
ones with entry.isIntersecting, choose a single winner (e.g., the entry with the
largest entry.intersectionRatio or the smallest entry.boundingClientRect.top)
and only activate that corresponding link (use the existing id ->
querySelector(`.section-anchors__link[data-anchor="${id}"]`) logic). After
selecting the winner, remove 'section-anchors__link--active' from links and add
it only to the winner's link to avoid flicker and incorrect states.
- Around line 30-55: The setupScrollSpy function creates a new
IntersectionObserver each time it's called (and on the 'astro:page-load' event)
but never disconnects the previous observer, causing memory leaks and duplicate
handlers; fix this by storing the active observer in a module-scoped variable
(e.g., let activeObserver), and at the start of setupScrollSpy check if
activeObserver exists and call activeObserver.disconnect() before creating a new
one, then assign the newly created observer to activeObserver and ensure any
observed targets are re-observed from links; also remove or rebind the
'astro:page-load' listener cleanly if you add a teardown (optional: disconnect
activeObserver on window unload/unmount to be safe).
In `@src/content/evidence/baseline-eval-report.mdx`:
- Around line 34-36: Update the inconsistent pass/fail counts: recount the
per-case results table (the table listing IDs like SR-001, TD-001, etc.) and
correct the Summary table (Total cases / Passed / Failed) and the Category
breakdown table so they match the actual per-case counts (18 PASS, 12 FAIL, 30
total), then recalc and update the pass-rate percentage accordingly wherever
referenced (failure-cases.mdx, workflow-vs-agent-comparison.mdx, homepage
stats). Ensure the per-case table entries remain unchanged and only numbers in
the Summary and Category breakdown tables and any derived pass-rate values are
edited to reflect the corrected counts.
In `@src/content/evidence/failure-cases.mdx`:
- Around line 6-8: The hero stat entry with value '37%' and label 'Baseline
failure rate (11/30)' is relying on inconsistent baseline counts; open
baseline-eval-report.mdx, reconcile the pass counts (18 vs 19) against the total
tests to determine the correct number of failures, recalc the failure rate and
update both the value field and the label string (the '11/30' part) in
src/content/evidence/failure-cases.mdx so the displayed percentage and failure
count match the verified baseline data.
In `@src/content/evidence/workflow-vs-agent-comparison.mdx`:
- Line 34: Confirm and correct the single-agent baseline before publishing: open
baseline-eval-report.mdx, reconcile the inconsistent counts (summary 19/30 vs
per-case 18/30) to determine the true single-agent pass rate, then update the
Pass rate entry in src/content/evidence/workflow-vs-agent-comparison.mdx (the
table row containing "Pass rate | 56.7% | 63.3% | 66.7%") to the verified value;
ensure the baseline file's summary and per-case table match (or add a correction
note) and, if the source data changed, update any references or citations that
depend on the baseline.
In `@src/layouts/EvidenceLayout.astro`:
- Around line 32-33: The reverse-link lookup is using the frontmatter field
d.data.id (shown as d.id in the diff) which can differ from the collection entry
identifier used as the route slug; update the call to getReverseLinks to use the
collection entry identifier (entry.id) that matches the dynamic route slug, i.e.
pass entry.id into getReverseLinks (or alternatively change the route param
generation to use entry.data.id everywhere) so
buildReverseIndex/entriesToContentEntries and getReverseLinks use the same
identifier convention for cross-reference lookups.
In `@src/lib/d3-helpers.ts`:
- Around line 9-12: The function colorForValue currently declares an unused
parameter `value`; remove that parameter from the function signature (keep the
behavior based solely on `isAccent`) and update any call sites (e.g., usages in
D3Chart.svelte) to stop passing a value argument or to pass only the `isAccent`
boolean; ensure the exported function name `colorForValue` remains the same and
adjust any imports or type annotations accordingly.
- Around line 14-17: formatTick currently uses a dead ternary (unit === '×' ? 1
: 1) so all units are formatted with the same precision; update the function
formatTick to select decimals based on unit (for example, use 1 decimal for '×'
and 0 decimals for '%' and 'pp' and maybe ''), compute decimals once and call
value.toFixed(decimals) so different units render with appropriate precision.
In `@src/pages/projects/index.astro`:
- Line 30: The anchor href currently hardcodes the base path in the expression
href={`/agentic-ai/projects/${e.data.slug}/`}; replace that with a dynamic base
(e.g. use import.meta.env.BASE_URL or a computed baseUrl variable) so the link
becomes href={`${baseUrl}projects/${e.data.slug}/`} (or a relative path) and
ensure you normalize trailing slashes to avoid double-slashes; update the <a
class="proj-card" ...> construction accordingly.
- Around line 34-38: The template directly accesses e.data.evalStats.accuracy,
e.data.evalStats.avgCost, and e.data.evalStats.latencyP50 which can be
null/undefined; update these references to use optional chaining and sensible
fallbacks (e.g., e.data.evalStats?.accuracy ?? '—') so missing evalStats won't
throw at render time, applying the same pattern to accuracy, avgCost, and
latencyP50 in the project list rendering.
- Line 10: The projects collection is fetched as const entries = await
getCollection('projects') but not sorted, causing unpredictable order; update
the retrieval to sort entries consistently (e.g., by title, date, or priority) —
either by using getCollection's query/sort options if available or by calling a
deterministic sort on the returned array (e.g., sort by entry.data.title or
entry.data.date) before rendering; refer to the symbol entries and the
getCollection call to locate where to apply the sort and ensure the chosen key
(title/date/priority) exists on each entry.
---
Duplicate comments:
In `@src/layouts/LabReportLayout.astro`:
- Around line 32-34: The reverse-link lookup in LabReportLayout.astro is using
the wrong identifier; replace the use of the nested data id with the same
identifier the route expects so IDs match across routing and reverse-linking.
Specifically, when building reverse links with
buildReverseIndex/entriesToContentEntries and calling getReverseLinks (the call
that currently passes d.data.id), pass the top-level entry.id (the same slug
used in src/pages/labs/[...slug].astro) so getReverseLinks and the route use the
same identifier consistently.
In `@src/pages/evidence/`[...slug].astro:
- Line 8: The route sets the page slug from the collection entry's file-based
identifier via params: { slug: entry.id }, but EvidenceLayout.astro currently
looks up reverse links using the frontmatter id (entry.data.id), which can
mismatch; update EvidenceLayout.astro to use entry.id (the file-based
identifier) when building reverse-link URLs and lookups (replace usages of
entry.data.id with entry.id) so the "Cited by" links resolve to the same slug
generation as this route.
In `@src/pages/labs/`[...slug].astro:
- Line 8: The slug param used in the route is set to entry.id but
LabReportLayout.astro's reverse-link lookup expects entry.data.id, causing
mismatched identifiers; update the route's params (where it's currently "params:
{ slug: entry.id }") to use the same identifier as LabReportLayout (e.g.,
entry.data.id) or alternatively change LabReportLayout to read entry.id so both
places (the route and the reverse-link lookup in LabReportLayout.astro) use the
exact same property name.
---
Nitpick comments:
In `@package.json`:
- Around line 21-23: The package.json lists redundant d3 subpackages (d3-array,
d3-scale) while your code imports from the umbrella 'd3' (see
src/lib/d3-helpers.ts), so remove the explicit "d3-array" and "d3-scale" entries
from package.json to avoid duplicate/ conflicting versions, then reinstall
(npm/yarn install) to update the lockfile/lock entries; verify imports in
d3-helpers.ts still import from 'd3' and run the test/build to confirm no
breakage.
In `@src/components/islands/D3Chart.svelte`:
- Around line 43-44: The effect block using $effect with the condition "if (!src
|| loadedData) return;" hides that when both data (which sets loadedData) and
src are provided, src fetch is skipped; update D3Chart.svelte by either adding a
clear comment next to the $effect explaining that explicit data prop takes
precedence over src, or add a runtime validation that ensures only one of the
two props is supplied (e.g., check data and src in onMount or at top of the
component and throw/log a warning) so callers know to provide only one;
reference symbols: $effect, src, loadedData, and the data prop to locate where
to add the comment or validation.
- Line 98: Replace the inline ternary color expressions with the centralized
helper: import colorForValue from d3-helpers and change fill={p.accent ?
'#9b4a3f' : '#1a1a1a'} to use fill={colorForValue(p.accent)} (and do the same
for the other occurrences mentioned); this consolidates color logic into the
colorForValue function used by the chart rendering code in D3Chart.svelte.
In `@src/components/universal/MethodBlock.astro`:
- Around line 27-28: Remove the unnecessary CSS overrides "padding-bottom: 0;"
and "border-bottom: 0;" from the MethodBlock.astro style block (or, if they are
intended as defensive overrides against global styles, replace them with a more
specific selector or add an inline comment explaining the intent) so the
component does not reset properties unnecessarily; target the declarations shown
("padding-bottom: 0;" and "border-bottom: 0;") in MethodBlock.astro and either
delete them or document/strengthen them with a selector specificity change.
In `@src/components/universal/SectionAnchors.astro`:
- Line 61: The sticky positioning in SectionAnchors.astro currently uses a
hardcoded top: 72px which will break if header height changes; replace the
literal with a CSS custom property (e.g., top: var(--header-height, 72px)) in
the SectionAnchors.astro stylesheet and document/update the global/header
component to set --header-height (or rely on the fallback 72px) so the anchor
offset stays correct across layouts and responsive sizes.
In `@src/layouts/EvidenceLayout.astro`:
- Line 32: The line using an unsafe cast should be changed to preserve type
safety: update the type of allEntries (where it's defined) to the correct
collection type or a union of allowed collection types and then call
entriesToContentEntries(allEntries) without casting, or alternatively update
entriesToContentEntries to accept that union type; ensure callers like
buildReverseIndex(reverseIndex) still receive the expected ContentEntry type so
replace "const reverseIndex =
buildReverseIndex(entriesToContentEntries(allEntries as any));" with a call that
uses the properly typed allEntries and/or an updated entriesToContentEntries
signature (refer to reverseIndex, buildReverseIndex, entriesToContentEntries,
and allEntries to locate and adjust the code).
- Around line 35-43: Duplicate collectionRoute objects in EvidenceLayout.astro,
LabReportLayout.astro and ProjectLayout.astro should be extracted to a single
shared module: create a new module that exports export const collectionRoute:
Record<string,string> = { chapters: '/agentic-ai/book', fieldNotes:
'/agentic-ai/field-notes', recipes: '/agentic-ai/recipes', projects:
'/agentic-ai/projects', evidence: '/agentic-ai/evidence', labs:
'/agentic-ai/labs', patterns: '/agentic-ai/patterns' }; then remove the local
collectionRoute declarations in EvidenceLayout.astro, LabReportLayout.astro and
ProjectLayout.astro and import the shared collectionRoute constant from the new
module in each layout (replace the local variable with the imported symbol).
Ensure the export name is exactly collectionRoute so existing references in
those layouts continue to work.
In `@src/pages/evidence/index.astro`:
- Around line 14-18: The heroStats array is hardcoded and should be derived from
the evidence data: compute the three aggregate metrics from the existing sorted
evidence entries (use the variable sorted or the evidence collection used in
this file) instead of fixed literals; update the place where heroStats is
defined (replace the static const heroStats declaration) to calculate values
(e.g., compute workflow improvement in percentage points, cost ratio, and
latency ratio) from sorted entries, format them into the same string shapes
('3.4pp', '2.4×', '2.2×'), and preserve the color/label structure so the rest of
the page (references to heroStats) continues to work. Ensure numeric
computations handle empty arrays and types match the expected shape (remove or
adjust the 'as const' if necessary) and keep the identifier heroStats for
downstream usage.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: f4e3412a-58a7-4cea-8655-9ed93b5990a9
⛔ Files ignored due to path filters (48)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlpublic/assets/diagrams/agent-anatomy.svgis excluded by!**/*.svgpublic/assets/diagrams/agent-loop-foundations.svgis excluded by!**/*.svgpublic/assets/diagrams/agent-tax-comparison.svgis excluded by!**/*.svgpublic/assets/diagrams/agent-trace-waterfall.svgis excluded by!**/*.svgpublic/assets/diagrams/api-contract.svgis excluded by!**/*.svgpublic/assets/diagrams/before-after-guardrails.svgis excluded by!**/*.svgpublic/assets/diagrams/complexity-value-quadrant.svgis excluded by!**/*.svgpublic/assets/diagrams/context-pipeline-layers.svgis excluded by!**/*.svgpublic/assets/diagrams/context-window-bucket.svgis excluded by!**/*.svgpublic/assets/diagrams/defense-layers.svgis excluded by!**/*.svgpublic/assets/diagrams/escalation-decision-tree.svgis excluded by!**/*.svgpublic/assets/diagrams/eval-bucket-distribution.svgis excluded by!**/*.svgpublic/assets/diagrams/eval-loop.svgis excluded by!**/*.svgpublic/assets/diagrams/failure-surfaces.svgis excluded by!**/*.svgpublic/assets/diagrams/framework-decision-tree.svgis excluded by!**/*.svgpublic/assets/diagrams/framework-layers.svgis excluded by!**/*.svgpublic/assets/diagrams/full-spectrum-decision.svgis excluded by!**/*.svgpublic/assets/diagrams/function-calling-cycle.svgis excluded by!**/*.svgpublic/assets/diagrams/hallucination-mental-model.svgis excluded by!**/*.svgpublic/assets/diagrams/hitl-approval-flow.svgis excluded by!**/*.svgpublic/assets/diagrams/incident-runbook-architecture.svgis excluded by!**/*.svgpublic/assets/diagrams/mcp-architecture.svgis excluded by!**/*.svgpublic/assets/diagrams/mcp-before-after.svgis excluded by!**/*.svgpublic/assets/diagrams/mcp-lifecycle.svgis excluded by!**/*.svgpublic/assets/diagrams/memory-hierarchy.svgis excluded by!**/*.svgpublic/assets/diagrams/message-contract-flow.svgis excluded by!**/*.svgpublic/assets/diagrams/multi-agent-coordination.svgis excluded by!**/*.svgpublic/assets/diagrams/multi-agent-cost.svgis excluded by!**/*.svgpublic/assets/diagrams/multi-agent-error-amplification.svgis excluded by!**/*.svgpublic/assets/diagrams/observe-think-act.svgis excluded by!**/*.svgpublic/assets/diagrams/poisoning-attack-flow.svgis excluded by!**/*.svgpublic/assets/diagrams/risk-tier-matrix.svgis excluded by!**/*.svgpublic/assets/diagrams/shared-memory-scopes.svgis excluded by!**/*.svgpublic/assets/diagrams/single-turn-ceiling.svgis excluded by!**/*.svgpublic/assets/diagrams/single-vs-multi-agent.svgis excluded by!**/*.svgpublic/assets/diagrams/state-management.svgis excluded by!**/*.svgpublic/assets/diagrams/system-architecture.svgis excluded by!**/*.svgpublic/assets/diagrams/system-types-spectrum.svgis excluded by!**/*.svgpublic/assets/diagrams/three-failure-modes.svgis excluded by!**/*.svgpublic/assets/diagrams/three-way-comparison.svgis excluded by!**/*.svgpublic/assets/diagrams/token-cost-calculator.svgis excluded by!**/*.svgpublic/assets/diagrams/tool-selection-comparison.svgis excluded by!**/*.svgpublic/assets/diagrams/trace-waterfall.svgis excluded by!**/*.svgpublic/assets/diagrams/truncation-strategies.svgis excluded by!**/*.svgpublic/assets/diagrams/trust-boundaries.svgis excluded by!**/*.svgpublic/assets/diagrams/two-pass-retrieval.svgis excluded by!**/*.svgpublic/assets/diagrams/workflow-vs-agent.svgis excluded by!**/*.svg
📒 Files selected for processing (38)
astro.config.mjspackage.jsonsrc/components/islands/CommandPalette.sveltesrc/components/islands/D3Chart.sveltesrc/components/universal/ArchitectureDiagram.astrosrc/components/universal/DownloadList.astrosrc/components/universal/EvalStats.astrosrc/components/universal/HeroStatGrid.astrosrc/components/universal/HypothesisBlock.astrosrc/components/universal/MethodBlock.astrosrc/components/universal/ProductCardHero.astrosrc/components/universal/Provenance.astrosrc/components/universal/ReproduceStrip.astrosrc/components/universal/SectionAnchors.astrosrc/components/universal/StatHero.astrosrc/content/evidence/baseline-eval-report.mdxsrc/content/evidence/failure-cases.mdxsrc/content/evidence/trace-example.mdxsrc/content/evidence/workflow-vs-agent-comparison.mdxsrc/content/labs/multi-agent-vs-router-100-queries.mdxsrc/content/projects/doc-intelligence-agent.mdxsrc/content/projects/framework-comparison.mdxsrc/content/projects/incident-runbook-agent.mdxsrc/content/projects/llm-explorer.mdxsrc/content/projects/memory-agent.mdxsrc/content/projects/research-agent.mdxsrc/content/projects/tool-using-assistant.mdxsrc/layouts/EvidenceLayout.astrosrc/layouts/LabReportLayout.astrosrc/layouts/PageLayout.astrosrc/layouts/ProjectLayout.astrosrc/lib/d3-helpers.tssrc/pages/evidence/[...slug].astrosrc/pages/evidence/index.astrosrc/pages/labs/[...slug].astrosrc/pages/labs/index.astrosrc/pages/projects/[...slug].astrosrc/pages/projects/index.astro
| ) : ( | ||
| <MagazineGrid cols={3}> | ||
| {entries.map((e) => ( | ||
| <a class="proj-card" href={`/agentic-ai/projects/${e.data.slug}/`}> |
There was a problem hiding this comment.
Avoid hardcoding the /agentic-ai/ base path.
The URL hardcodes /agentic-ai/projects/ which will break if the site is deployed at a different base path. Consider using Astro's import.meta.env.BASE_URL or relative paths.
🔗 Proposed fix to use dynamic base path
- <a class="proj-card" href={`/agentic-ai/projects/${e.data.slug}/`}>
+ <a class="proj-card" href={`${import.meta.env.BASE_URL}projects/${e.data.slug}/`}>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <a class="proj-card" href={`/agentic-ai/projects/${e.data.slug}/`}> | |
| <a class="proj-card" href={`${import.meta.env.BASE_URL}projects/${e.data.slug}/`}> |
🤖 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 `@src/pages/projects/index.astro` at line 30, The anchor href currently
hardcodes the base path in the expression
href={`/agentic-ai/projects/${e.data.slug}/`}; replace that with a dynamic base
(e.g. use import.meta.env.BASE_URL or a computed baseUrl variable) so the link
becomes href={`${baseUrl}projects/${e.data.slug}/`} (or a relative path) and
ensure you normalize trailing slashes to avoid double-slashes; update the <a
class="proj-card" ...> construction accordingly.
Owner
Author
There was a problem hiding this comment.
Pushing back on this one. The hardcoded /agentic-ai/ prefix is the production deploy path baked into astro.config.mjs base: '/agentic-ai'. Same pattern is used across all routes/links in W1/W2/W3 (homepage, manifesto, field-notes, recipes, evidence, labs). If we ever change the base, a single find/replace across the codebase handles it.
Using import.meta.env.BASE_URL would be more idiomatic but adds runtime concatenation at every link site without any current benefit (we have no other deploy targets planned). YAGNI applies; can refactor if/when the base actually changes.
There was a problem hiding this comment.
@sunilp, fair point — the YAGNI argument holds when there's a single well-known deploy target and the pattern is already consistent across the entire codebase. I'll drop this concern.
✏️ Learnings added
Learnt from: sunilp
Repo: sunilp/agentic-ai PR: 9
File: src/pages/projects/index.astro:30-30
Timestamp: 2026-05-13T14:37:11.515Z
Learning: In the `sunilp/agentic-ai` repo (Astro 5, deployed to GitHub Pages), the base path `/agentic-ai/` is intentionally hardcoded in all internal hrefs across pages, layouts, and components (W1/W2/W3). This is a deliberate YAGNI decision: the Astro `base` config is set to `/agentic-ai` and there are no other planned deploy targets. Do not flag hardcoded `/agentic-ai/` prefixes in links as an issue; the team prefers a single find/replace if the base ever changes over `import.meta.env.BASE_URL` at every link site.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
Comment on lines
+34
to
+38
| <span>{e.data.evalStats.accuracy}</span> | ||
| <span>·</span> | ||
| <span>{e.data.evalStats.avgCost}</span> | ||
| <span>·</span> | ||
| <span>{e.data.evalStats.latencyP50}</span> |
There was a problem hiding this comment.
Add null safety checks for evalStats fields.
The code assumes evalStats.accuracy, evalStats.avgCost, and evalStats.latencyP50 always exist. If any project entry has missing or incomplete evalStats, this will cause a runtime error.
🛡️ Proposed fix to add optional chaining and fallbacks
<div class="proj-card__stats">
- <span>{e.data.evalStats.accuracy}</span>
+ <span>{e.data.evalStats?.accuracy ?? 'N/A'}</span>
<span>·</span>
- <span>{e.data.evalStats.avgCost}</span>
+ <span>{e.data.evalStats?.avgCost ?? 'N/A'}</span>
<span>·</span>
- <span>{e.data.evalStats.latencyP50}</span>
+ <span>{e.data.evalStats?.latencyP50 ?? 'N/A'}</span>
</div>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <span>{e.data.evalStats.accuracy}</span> | |
| <span>·</span> | |
| <span>{e.data.evalStats.avgCost}</span> | |
| <span>·</span> | |
| <span>{e.data.evalStats.latencyP50}</span> | |
| <span>{e.data.evalStats?.accuracy ?? 'N/A'}</span> | |
| <span>·</span> | |
| <span>{e.data.evalStats?.avgCost ?? 'N/A'}</span> | |
| <span>·</span> | |
| <span>{e.data.evalStats?.latencyP50 ?? 'N/A'}</span> |
🤖 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 `@src/pages/projects/index.astro` around lines 34 - 38, The template directly
accesses e.data.evalStats.accuracy, e.data.evalStats.avgCost, and
e.data.evalStats.latencyP50 which can be null/undefined; update these references
to use optional chaining and sensible fallbacks (e.g.,
e.data.evalStats?.accuracy ?? '—') so missing evalStats won't throw at render
time, applying the same pattern to accuracy, avgCost, and latencyP50 in the
project list rendering.
Owner
Author
There was a problem hiding this comment.
Pushing back. The projects collection schema enforces evalStats as a required object with three required string fields (accuracy, avgCost, latencyP50). Astro's Content Layer fails the build if any are missing — verified against src/content.config.ts:69-73. Null safety at the call site would be unreachable code and obscure the schema guarantee. If schema ever changes to make these optional, we'll add guards then.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
cat -n src/content.config.ts | sed -n '60,85p'
Length of output: 1071
@sunilp, you're correct — confirmed against src/content.config.ts lines 69–73. evalStats and all three of its fields (accuracy, avgCost, latencyP50) are required Zod strings with no .optional(), so Astro's Content Layer will reject any entry missing them at build time. Optional chaining at the call site would be dead code and would obscure the schema contract. I'll retract this comment.
✏️ Learnings added
Learnt from: sunilp
Repo: sunilp/agentic-ai PR: 9
File: src/pages/projects/index.astro:34-38
Timestamp: 2026-05-13T14:37:24.549Z
Learning: In the `sunilp/agentic-ai` Astro project, the `projects` content collection schema (defined in `src/content.config.ts` lines 69–73) enforces `evalStats` as a required `z.object` with three required `z.string()` fields: `accuracy`, `avgCost`, and `latencyP50`. Astro's Content Layer validates this at build time, so null/undefined guards for these fields in page templates (e.g., `src/pages/projects/index.astro`) are unnecessary and misleading — do not flag them as missing null safety.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
… fix formatTick dead ternary
… failuresAnchor schema fields
…secting (fix leak + race)
8 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
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.
Summary
Week 3 of the mkdocs → Astro 5 migration. Builds on W1 (PR #7) and W2 (PR #8). This PR:
/evidence/with the homepage's signature 3.4pp / 2.4× / 2.2× as page-level hero.src/lib/d3-helpers.tsutility module.docs/diagrams/topublic/assets/diagrams/.client:idle. Pagefind index loads on first open.What's NOT in this PR (intentional)
docs/,mkdocs.yml,.github/workflows/deploy.ymlare untouched. Live site continues serving mkdocs./start/audience routing page — W4./patterns/cross-cut hub — W4.og-default.pngand L-001 OG image — documented as follow-up in W3 ship summary. Currentog-default.pngis the W2 placeholder.Test Plan
pnpm install && pnpm test— expect 11/11 passingpnpm astro check— expect 0 errors, 0 warnings (1 pre-existing hint in d3-helpers.ts is harmless)pnpm build— expect 22 pages built + 5 redirect stubs + Pagefind index (22 pages / 2803 words)pnpm devand visit:/agentic-ai/projects/— magazine grid with 7 project cards/agentic-ai/projects/doc-intelligence-agent/— flagship: 4-quadrant ProductCardHero, ArchitectureDiagram (system-architecture.svg), EvalStats row, case study body/agentic-ai/projects/research-agent/— confirm real source-numbers in EvalStats (100% / $0.000276 / 73.4ms)/agentic-ai/evidence/— Evidence Wall with 3 brick-red hero stats + 4 evidence cards/agentic-ai/evidence/workflow-vs-agent-comparison/— backs the homepage 3.4pp claim; HeroStatGrid with brick-red 3.4pp + 2.4× + 2.2×/agentic-ai/labs/— Labs index with L-001 card/agentic-ai/labs/multi-agent-vs-router-100-queries/— full Lab: hypothesis, "87% vs 74%" StatHero, 6 section anchors, reproduce strip with seed=42Cmd+K(orCtrl+K): command palette slides in with backdrop blur. Type "agent" — Pagefind returns hits across multiple content types. Arrow keys navigate; Enter goes to result.prefers-reduced-motion: reduce— confirm scroll-driven animations (ArchitectureDiagram trace-in, Banner parallax) stop, CommandPalette panel doesn't slideSummary by CodeRabbit
New Features
Documentation