graypaper: pin reader URLs to latest hash + version - #255
Conversation
Embed the latest graypaper hash and version in result links (`/#/<shortHash>?v=<version>&...`) so the reader skips its client-side redirect. The latest ref is persisted in `versions.md` frontmatter and exposed via `GET /graypaper/latest`; both backend and frontend share a single URL composer in `shared/graypaper.ts`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
✅ Deploy Preview for jam-search2 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds dynamic graypaper latest metadata retrieval and caching (from Changes
Sequence DiagramsequenceDiagram
participant Client as React Client
participant Hook as useGraypaperLatest Hook
participant API as Backend API
participant DataLayer as graypaperLatest Module
participant FS as File System
Client->>Hook: mount -> request latest
activate Hook
Hook->>API: GET /graypaper/latest
activate API
API->>DataLayer: getGraypaperLatest(dataDir)
activate DataLayer
DataLayer->>FS: read versions.md (if exists)
FS-->>DataLayer: frontmatter ({latest_hash, latest_version})
DataLayer->>DataLayer: cache by dataDir (invalidate on mtime change)
DataLayer-->>API: {hash, version}
deactivate DataLayer
API-->>Hook: JSON {hash, version}
deactivate API
Hook-->>Client: latest (cached)
deactivate Hook
Client->>Client: buildGraypaperUrl(title, query, latest)
Client->>Client: render reader link (pinned or unpinned)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/src/data/graypaperLatest.ts`:
- Around line 23-27: The cached value in getGraypaperLatest (stored in cache)
never updates when the on-disk graypaper/versions.md is rewritten, so calls keep
returning stale data; fix by invalidating or updating that cache when the source
changes—either: (A) modify getGraypaperLatest to detect on-disk changes (stat
versions.md or include a timestamp from readFromDisk and compare it to cache)
and reload if changed, or (B, simpler) clear the cache after writes by resetting
cache in updateGraypaperVersions immediately after successfully writing the file
(ensure you reference cache, getGraypaperLatest, readFromDisk, and
updateGraypaperVersions when making the change).
In `@backend/src/data/writer.ts`:
- Around line 261-282: writeGraypaperVersions currently drops previously
persisted latest_hash/latest_version when called without the optional latest;
change it to preserve existing frontmatter by reading the current file at
filePath (versions.md) if it exists, parsing its frontmatter (using the same
frontmatter parser used elsewhere), and merge any existing
latest_hash/latest_version into the frontmatter object only when the incoming
latest argument is undefined; ensure you still overwrite the versions array from
the versions parameter, and update references to writeGraypaperVersions and any
callers (e.g., exportToMarkdown.ts) only as needed so omissions no longer
destroy the pinned metadata.
In `@backend/src/scripts/updateGraypaperVersions.ts`:
- Around line 63-69: The code currently only sets latestChanged when
resolveLatest() returns a non-null latest, so if resolveLatest() returns null
stale existing.latestHash/existing.latestVersion remain persisted; update the
logic around resolveLatest()/latestChanged to treat the transition from a
non-null persisted latest to null as a change (e.g., set latestChanged = true
when latest === null but existing.latestHash or existing.latestVersion are
present), and ensure the persistence/write path for the metadata (the code that
writes existing.latestHash/existing.latestVersion when hasNewVersion or
latestChanged is true) clears those fields when latest is null so the on-disk
versions.md drops pinned values and falls back to unpinned form.
In `@client/src/pages/viewall/graypaper.tsx`:
- Line 93: The buildGraypaperUrl call is passing raw user content (query and
section.title) into URL parameters which can break links when values contain
characters like & or #; update the shared builder (function buildGraypaperUrl in
shared/graypaper.ts) to URL-encode the query and section/section.title values
using encodeURIComponent (or an equivalent encoder) before concatenating them
into the search string so the generated url prop in
client/src/pages/viewall/graypaper.ts is safe and preserves the original values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4c372c9a-8a81-4804-8297-1f7bd07b984f
⛔ Files ignored due to path filters (1)
data/graypaper/versions.mdis excluded by!data/**
📒 Files selected for processing (15)
backend/src/__tests__/ask/tools.test.tsbackend/src/api.tsbackend/src/ask/agentLoop.tsbackend/src/ask/tools.tsbackend/src/data/graypaperLatest.tsbackend/src/data/writer.tsbackend/src/mcp/server.tsbackend/src/scripts/updateGraypaperVersions.tsclient/src/components/results/GraypaperResults.tsxclient/src/hooks/useGraypaperLatest.tsclient/src/lib/__tests__/graypaperUrl.test.tsclient/src/lib/api.tsclient/src/pages/viewall/graypaper.tsxshared/graypaper.tsshared/index.ts
- getGraypaperLatest invalidates its cache on versions.md mtime change (graypaperJob runs out-of-process and rewrites the file). - writeGraypaperVersions now treats `latest === undefined` as "preserve" so callers like exportToMarkdown.ts don't wipe the pinned ref, and `latest === null` as "clear" so the metadata-no-longer-has-latest case drops the stale pin. - updateGraypaperVersions detects null↔value transitions both ways. Declined the suggestion to URL-encode params: prior PR #250 explicitly removed encoding to match the reader's hash-router contract — changing that needs reader-side verification, not a drive-by edit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
https://graypaper.fluffylabs.dev/#/<shortHash>?v=<version>&search=...§ion=..., skipping the reader's client-side redirect.data/graypaper/versions.mdfrontmatter (captured byupdateGraypaperVersionsfrommetadata.json) and exposed viaGET /graypaper/latest.shared/graypaper.tsis shared between backend (tools.ts, AI agent citations) and frontend (results pages, via auseGraypaperLatesthook).Test plan
#/ab2cdbd?v=0.7.2&...and the reader opens directly with no redirect.GET /graypaper/latestreturns{hash, version}.latest_hashin versions.md), composer falls back to the un-pinned URL — links still work.npm run qa(lint + biome) andnpm testpass on both backend and client.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests