Conversation
✅ Deploy Preview for graypaper-reader ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughAdds optional nightly version metadata and resolves matching hashes to the "nightly" channel across metadata URL builders, location hashing, and version labeling; metadata preload source switched to a remote URL. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Browser
participant MetadataProvider
participant LocationProvider
participant VersionComponent
Browser->>MetadataProvider: preload / fetch metadata.json
MetadataProvider->>MetadataProvider: parse metadata (versions, latest, nightly)
Browser->>LocationProvider: request resolve from URL params
LocationProvider->>MetadataProvider: read versions & nightly.hash
alt fullVersion matches a version
LocationProvider->>LocationProvider: resolve fullVersion -> version
else fullVersion matches nightly.hash
LocationProvider->>LocationProvider: resolve fullVersion -> nightly.hash (treated as "nightly")
end
Browser->>VersionComponent: render version UI with resolved fullVersion
VersionComponent->>MetadataProvider: request labels (latest, nightly)
MetadataProvider-->>VersionComponent: supply labels (Nightly/Latest/vX)
VersionComponent-->>Browser: render labels and options
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (4)src/**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{js,jsx,ts,tsx}⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (1)📚 Learning: 2025-12-28T13:25:05.152ZApplied to files:
🧬 Code graph analysis (1)src/components/Version/Version.tsx (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (5)
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/MetadataProvider/MetadataProvider.tsx (1)
14-20: Add runtime validation for thenightlyfield to prevent runtime errors.The
nightlyfield is required in theIMetadatainterface (line 16), and Version.tsx directly accessesmetadata.nightlywithout null checks (lines 20–22, 93). If the externalmetadata.jsondoesn't include thenightlyfield, the application will crash at runtime.The type assertion on line 52 (
as IMetadata) won't catch this at runtime. Add validation to ensure the field exists:useEffect(() => { const fetchMetadata = async () => { try { - setMetadata((await (await fetch(METADATA_JSON)).json()) as IMetadata); + const data = (await (await fetch(METADATA_JSON)).json()) as IMetadata; + if (!data.nightly) { + throw new Error("metadata.json missing required 'nightly' field"); + } + setMetadata(data); } catch (e) { console.error("Couldn't load metadata.", e); } };Alternatively, make the field optional only if you provide a fallback in Version.tsx for when it's missing.
🧹 Nitpick comments (2)
src/components/Version/Version.tsx (1)
46-54: Consider extracting duplicated label logic.The label construction logic (lines 49-52 and 113-116) is nearly identical. Extracting it into a shared helper would reduce duplication.
🔎 Proposed refactor
+function getVersionLabel(version: IVersionInfo, latest: string, nightly: string): string { + const isNightly = version.hash === nightly; + const isLatest = version.hash === latest; + let label = isNightly ? "Nightly" : isLatest ? "Latest" : "v"; + if (version.name && !isNightly) { + label += `: ${version.name}`; + } + return label; +} + function shortHash(h: string) {Then use
getVersionLabel(currentVersion, metadata.latest, metadata.nightly.hash)in both places.Also applies to: 113-116
src/components/MetadataProvider/MetadataProvider.tsx (1)
49-59: Add validation for the metadata structure.The fetch logic performs a type assertion without validating that the returned JSON includes all required fields. If
metadata.jsonis missing thenightlyfield, the error won't surface until the field is accessed elsewhere, making debugging harder.Consider adding validation after fetching:
🔎 Proposed validation logic
useEffect(() => { const fetchMetadata = async () => { try { - setMetadata((await (await fetch(METADATA_JSON)).json()) as IMetadata); + const data = (await (await fetch(METADATA_JSON)).json()) as IMetadata; + + // Validate required fields + if (!data.latest || !data.nightly || !data.versions) { + throw new Error("Invalid metadata structure: missing required fields"); + } + + setMetadata(data); } catch (e) { console.error("Couldn't load metadata.", e); } }; fetchMetadata(); }, []);Alternatively, if you make
nightlyoptional (as suggested in the previous comment), update the validation accordingly:useEffect(() => { const fetchMetadata = async () => { try { - setMetadata((await (await fetch(METADATA_JSON)).json()) as IMetadata); + const data = (await (await fetch(METADATA_JSON)).json()) as IMetadata; + + // Validate required fields + if (!data.latest || !data.versions) { + throw new Error("Invalid metadata structure: missing required fields"); + } + + setMetadata(data); } catch (e) { console.error("Couldn't load metadata.", e); } }; fetchMetadata(); }, []);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/MetadataProvider/MetadataProvider.tsxsrc/components/Version/Version.tsx
🧰 Additional context used
📓 Path-based instructions (4)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: Feature folders should keep components, hooks, and styles together insrc/
Prefer hooks over Higher-Order Components (HOCs)
Colocate helper functions with their consumer components
Use PascalCase for component names
Favor semantic wrappers over long Tailwind utility class strings; complement Tailwind with@fluffylabs/shared-uicomponents
Files:
src/components/MetadataProvider/MetadataProvider.tsxsrc/components/Version/Version.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript as the primary language with functional React components
Use camelCase for function and variable names
Use SCREAMING_SNAKE_CASE for constants
Files:
src/components/MetadataProvider/MetadataProvider.tsxsrc/components/Version/Version.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use 2-space indentation
Files:
src/components/MetadataProvider/MetadataProvider.tsxsrc/components/Version/Version.tsx
**/*.{js,jsx,ts,tsx}
⚙️ CodeRabbit configuration file
When reviewing Tailwind CSS classes, ensure they follow Tailwind 4.x conventions and suggest modern 4.x alternatives for deprecated patterns.
Files:
src/components/MetadataProvider/MetadataProvider.tsxsrc/components/Version/Version.tsx
🧠 Learnings (2)
📚 Learning: 2025-12-28T13:25:05.152Z
Learnt from: chmurson
Repo: FluffyLabs/graypaper-reader PR: 355
File: src/components/NoteManager/components/NoteLayout.tsx:39-39
Timestamp: 2025-12-28T13:25:05.152Z
Learning: In Tailwind CSS 4.x, max-h-68 is a valid utility that sets max-height: 17rem (272px) and is part of the extended spacing scale. When reviewing TSX files, verify components use this class consistently and that tailwind.config.js includes the 68 spacing key in the extended spacing scale if you rely on it.
Applied to files:
src/components/MetadataProvider/MetadataProvider.tsxsrc/components/Version/Version.tsx
📚 Learning: 2025-10-22T20:36:10.440Z
Learnt from: chmurson
Repo: FluffyLabs/graypaper-reader PR: 330
File: src/components/Outline/OutlineLink.tsx:17-21
Timestamp: 2025-10-22T20:36:10.440Z
Learning: This repo targets React 19+; prefer passing refs as a normal prop to function components and avoid React.forwardRef in new code. File context: src/components/Outline/OutlineLink.tsx.
Applied to files:
src/components/Version/Version.tsx
🧬 Code graph analysis (1)
src/components/Version/Version.tsx (1)
src/components/MetadataProvider/MetadataProvider.tsx (1)
IVersionInfo(7-12)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: visual-tests
🔇 Additional comments (3)
src/components/Version/Version.tsx (3)
68-77: Verify warning behavior for nightly versions.The warning "The current version is not the latest" will display when viewing the nightly version since
nightly.hash !== metadata.latest. Is this the intended UX? Nightly users might find this warning confusing since nightly is typically newer than latest.If nightly should not trigger this warning, consider:
- {currentVersionHash !== metadata.latest && ( + {currentVersionHash !== metadata.latest && !isNightly && (
107-123: LGTM!The
VersionOptioncomponent is well-structured with proper TypeScript typing and follows functional component patterns. The label logic correctly handles all three version states (nightly, latest, standard).
86-96: LGTM!The dropdown rendering correctly includes the nightly version in the list and properly propagates the
nightlyhash to eachVersionOptionfor accurate label display.
Visual Regression Test Report ✅ PassedGithub run id: 20621695892 🔗 Artifacts: Download |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/components/Version/Version.tsx (1)
24-28: Critical: Potential crash if locationParams.version is invalid.If
locationParams.versiondoesn't matchmetadata.nightly.hashAND isn't a key inmetadata.versions, thencurrentVersionwill beundefined. Line 28 will then crash when accessingcurrentVersion.hash.This issue was flagged in a previous review but remains unresolved.
🔎 Proposed fix with fallback to latest
- const currentVersion = - metadata.nightly && locationParams.version === metadata.nightly.hash - ? metadata.nightly - : metadata.versions[locationParams.version]; + const currentVersion = + metadata.nightly && locationParams.version === metadata.nightly.hash + ? metadata.nightly + : metadata.versions[locationParams.version] ?? metadata.versions[metadata.latest];
🧹 Nitpick comments (1)
index.html (1)
6-6: Consider using environment variables for metadata URL to support offline local development.The metadata URL is hardcoded as
https://gp.fluffylabs.dev/metadata.jsoninMetadataProvider.tsxand used for runtime fetching viafetch(METADATA_JSON)in auseEffect. While the preload inindex.htmlserves as a performance hint, the actual metadata load depends on this hardcoded remote URL. This would prevent local development if the remote host is unavailable or if developers need to test with custom or offline metadata.Introducing an environment variable (e.g.,
VITE_METADATA_URL) with a default production value would allow developers to override the metadata source during local development and testing, similar to how the test infrastructure already usesPLAYWRIGHT_HOSTandPLAYWRIGHT_PORTfor flexibility.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
index.htmlsrc/components/LocationProvider/LocationProvider.tsxsrc/components/LocationProvider/utils/locationParamsToHash.tssrc/components/MetadataProvider/MetadataProvider.tsxsrc/components/Version/Version.tsx
🧰 Additional context used
📓 Path-based instructions (4)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: Feature folders should keep components, hooks, and styles together insrc/
Prefer hooks over Higher-Order Components (HOCs)
Colocate helper functions with their consumer components
Use PascalCase for component names
Favor semantic wrappers over long Tailwind utility class strings; complement Tailwind with@fluffylabs/shared-uicomponents
Files:
src/components/LocationProvider/LocationProvider.tsxsrc/components/MetadataProvider/MetadataProvider.tsxsrc/components/Version/Version.tsxsrc/components/LocationProvider/utils/locationParamsToHash.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript as the primary language with functional React components
Use camelCase for function and variable names
Use SCREAMING_SNAKE_CASE for constants
Files:
src/components/LocationProvider/LocationProvider.tsxsrc/components/MetadataProvider/MetadataProvider.tsxsrc/components/Version/Version.tsxsrc/components/LocationProvider/utils/locationParamsToHash.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use 2-space indentation
Files:
src/components/LocationProvider/LocationProvider.tsxsrc/components/MetadataProvider/MetadataProvider.tsxsrc/components/Version/Version.tsxsrc/components/LocationProvider/utils/locationParamsToHash.ts
**/*.{js,jsx,ts,tsx}
⚙️ CodeRabbit configuration file
When reviewing Tailwind CSS classes, ensure they follow Tailwind 4.x conventions and suggest modern 4.x alternatives for deprecated patterns.
Files:
src/components/LocationProvider/LocationProvider.tsxsrc/components/MetadataProvider/MetadataProvider.tsxsrc/components/Version/Version.tsxsrc/components/LocationProvider/utils/locationParamsToHash.ts
🧠 Learnings (2)
📚 Learning: 2025-12-28T13:25:05.152Z
Learnt from: chmurson
Repo: FluffyLabs/graypaper-reader PR: 355
File: src/components/NoteManager/components/NoteLayout.tsx:39-39
Timestamp: 2025-12-28T13:25:05.152Z
Learning: In Tailwind CSS 4.x, max-h-68 is a valid utility that sets max-height: 17rem (272px) and is part of the extended spacing scale. When reviewing TSX files, verify components use this class consistently and that tailwind.config.js includes the 68 spacing key in the extended spacing scale if you rely on it.
Applied to files:
src/components/LocationProvider/LocationProvider.tsxsrc/components/MetadataProvider/MetadataProvider.tsxsrc/components/Version/Version.tsx
📚 Learning: 2025-12-26T10:17:47.372Z
Learnt from: CR
Repo: FluffyLabs/graypaper-reader PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-26T10:17:47.372Z
Learning: Gray Paper PDFs are managed via the `graypaper-archive/` submodule and copied to `public/` during predev/prebuild via `scripts/cp-gp.js`
Applied to files:
index.html
🧬 Code graph analysis (2)
src/components/LocationProvider/LocationProvider.tsx (1)
src/components/LocationProvider/utils/constants.ts (1)
VERSION_SEGMENT_INDEX(1-1)
src/components/Version/Version.tsx (1)
src/components/MetadataProvider/MetadataProvider.tsx (1)
IVersionInfo(7-12)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: visual-tests
🔇 Additional comments (5)
src/components/LocationProvider/LocationProvider.tsx (1)
81-85: LGTM! Nightly version fallback properly implemented.The extended fallback logic correctly checks for nightly hash when the primary version lookup fails. The use of optional chaining ensures safe access to
metadata.nightly?.hash.src/components/LocationProvider/utils/locationParamsToHash.ts (1)
16-19: LGTM! Version name derivation correctly handles nightly.The logic properly maps nightly hash to the "nightly" label and falls back appropriately through the chain: named version → nightly → latest → empty string.
src/components/MetadataProvider/MetadataProvider.tsx (2)
3-4: Remote metadata host is now hardcoded.The
METADATA_HOSTis now hardcoded tohttps://gp.fluffylabs.dev, matching the preload URL inindex.html. Ensure this works correctly in all environments (dev, staging, production).Related to the
index.htmlchange on line 6.
61-75: LGTM! Excellent abstraction for version resolution.The
resolveVersionhelper cleanly maps nightly hashes to the "nightly" label and applies it consistently across all URL getters. The restructured memoization correctly returns a populated context when metadata exists.src/components/Version/Version.tsx (1)
20-23: LGTM! Versions array correctly includes nightly.The versions array properly filters out legacy versions and conditionally includes nightly when it exists. Good use of optional spreading.
…per-reader into ms-add-nightly-version
2 participants
Summary by CodeRabbit
New Features
Behavior Changes
Chore
✏️ Tip: You can customize this high-level summary in your review settings.