Move PHOTO-TEXTE to Cloudflare and refine bilingual workflow - #33
Move PHOTO-TEXTE to Cloudflare and refine bilingual workflow#33mani1261790 wants to merge 11 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR migrates the application to Cloudflare and Better Auth, replaces learning highlights with correction annotations, adds a staged per-photo editor, introduces PDF export, updates PPTX export, persists language selection, and enforces photo state transitions. ChangesCloudflare platform and authentication
Editor and export workflow
Photo state validation
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Editor
participant ExportAPI
participant Workflow
participant Generator
participant Storage
Editor->>ExportAPI: Request PDF or PPTX export
ExportAPI->>Workflow: Pass format and memo option
Workflow->>Generator: Generate annotated presentation
Generator-->>Workflow: Return file buffer
Workflow->>Storage: Upload format-specific file
Storage-->>Editor: Return download metadata
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dfad837f6e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }, | ||
| ); | ||
| setExportUrl(`/api/exports/${result.token}/download`); | ||
| const result = await apiFetch<{ token: string }>(`/api/entries/${id}/export/${format}`, { |
There was a problem hiding this comment.
When a learner edits the final French text and then exports without pressing save, or clicks export while saveFinalText is still in flight, this request reads the saved DB value because exportFile only waits for memos and dirty annotations. The on-screen comparison is driven by activeFinalDraft, so the generated PPTX/PDF can silently contain the previous final text; disable/export-save while final drafts differ from photo.final_fr or while finalSavingId is set before making this API call.
Useful? React with 👍 / 👎.
![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
🧹 Nitpick comments (10)
lib/exports/presentation.ts (1)
86-92: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
computePhotoGridsupports at most 10 photos.For
n > 10, the returned grid holds only 10 cells. Both generators iterate every photo and computerow = Math.floor(index / cols), so photo 11 lands on a third row that falls outside the grid box and off the slide. The 10-photo limit is enforced elsewhere, so this is an edge case, but a clamp keeps the layout self-consistent.♻️ Proposed change
export function computePhotoGrid(n: number): { cols: number; rows: number } { if (n <= 3) return { cols: 3, rows: 1 }; if (n === 4) return { cols: 2, rows: 2 }; if (n <= 6) return { cols: 3, rows: 2 }; if (n <= 9) return { cols: 3, rows: 3 }; - return { cols: 5, rows: 2 }; + if (n <= 10) return { cols: 5, rows: 2 }; + const cols = 5; + return { cols, rows: Math.ceil(n / cols) }; }🤖 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 `@lib/exports/presentation.ts` around lines 86 - 92, Update computePhotoGrid so values of n greater than 10 are clamped to the supported 10-photo layout before selecting the grid dimensions, preserving the existing layouts for counts from 1 through 10 and ensuring the returned grid always has capacity for every generated photo.app/api/exports/[token]/download/route.ts (1)
47-47: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueConsider persisting the export format.
The extension check works today because
lib/workflows/export.tsbuildsobject_pathas${hash}.${format}. The contract is implicit: any path that is notformatcolumn on theexportstable would make the response MIME type follow the recorded format instead of a filename convention. This is safe to defer.🤖 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 `@app/api/exports/`[token]/download/route.ts at line 47, Defer changes to the format-detection logic in the download route; the current ExportFormat derivation from file.object_path remains acceptable. If implementing this later, persist the export format on the exports record and use that recorded value instead of inferring it from the filename extension.tests/export-content.test.ts (1)
165-194: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a PDF case with Japanese text.
All PDF inputs here are Latin-only, so the test cannot reach the standard-font encoding limit described in the
lib/pdf/generator.tscomment. Add one case wheretitleFranddraftFrcontain Japanese characters. That case fails today and passes once font selection follows the text content.💚 Proposed additional case
+ it('generates a PDF when the title and draft contain Japanese', async () => { + const pdfBuffer = await generatePhotoTextePdf({ + titleFr: '公園の写真', + photos: [{ + position: 1, + draftFr: 'Je visite un parc 公園.', + jpAuto: '私は公園を訪れます。', + jpIntent: '私は静かに公園を訪れます。', + finalFr: 'Je visite calmement un parc 公園.', + }], + }); + expect(pdfBuffer.subarray(0, 5).toString()).toBe('%PDF-'); + }, 20_000);🤖 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 `@tests/export-content.test.ts` around lines 165 - 194, Add a Japanese-text PDF test alongside the existing generatePhotoTextePdf coverage, using Japanese characters in titleFr and draftFr while preserving the current PDF/PPTX page-count, dimensions, and header assertions. Ensure the case exercises font selection based on text content and fails with the current standard-font path before the generator fix.lib/pptx/generator.ts (1)
366-379: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUse one text box for the corrected text.
This loop adds one
addTextshape for every placement, so longer corrected text can become hundreds of separate shapes on one slide. Keep the segment layout for the known-range outlines, but render words as text runs in a single box. pptxgenjs supports per-runhighlightfor the annotation background color.Severity/labels:
🤖 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 `@lib/pptx/generator.ts` around lines 366 - 379, Update the rendering loop around measured.placements to create one slide.addText text box containing text runs rather than one shape per placement. Preserve the existing placement-based layout for known-range outlines, and assign each run’s position-independent styling plus its annotation background through pptxgenjs highlight using the appropriate placement color.lib/api/schemas.ts (1)
18-23: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBind the version literal to the shared constant.
learningHighlightsSchemahardcodes2, whilelib/learning/annotations.tsexportsCORRECTION_ANNOTATION_VERSION. If the version increments, the schema and the normalizer can drift, and valid payloads would be rejected or stale payloads accepted.♻️ Proposed refactor
+import { CORRECTION_ANNOTATION_VERSION } from "`@/lib/learning/annotations`"; + const learningHighlightsSchema = z.object({ - version: z.literal(2), + version: z.literal(CORRECTION_ANNOTATION_VERSION), textSignature: z.string().min(1).max(120),🤖 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 `@lib/api/schemas.ts` around lines 18 - 23, Update learningHighlightsSchema to derive its z.literal version value from the shared CORRECTION_ANNOTATION_VERSION exported by annotations.ts instead of hardcoding 2, keeping schema validation aligned with the normalizer.lib/learning/annotations.ts (1)
36-43: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAlign the signature prefix with the annotation version.
The signature string starts with
v1:, butCORRECTION_ANNOTATION_VERSIONis2. The value is only compared against itself, so behavior is correct today. The mismatch is still misleading for future version changes.♻️ Proposed refactor
- return `v1:${text.length}:${(hash >>> 0).toString(36)}`; + return `v${CORRECTION_ANNOTATION_VERSION}:${text.length}:${(hash >>> 0).toString(36)}`;Note: this changes stored signatures, so existing annotations reset on first load. If that is not acceptable, keep the literal and add a comment that the prefix is independent of the annotation version.
🤖 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 `@lib/learning/annotations.ts` around lines 36 - 43, Update getAnnotationTextSignature to derive its signature prefix from CORRECTION_ANNOTATION_VERSION instead of the hardcoded v1 literal, ensuring the prefix remains aligned with the annotation version and preserving the existing signature format.app/globals.css (1)
52-57: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe base
--minimum-font-sizevalue has no effect.Every consumer wraps the variable in
max(11px, var(--minimum-font-size))(lines 181, 1615, 1633). The root value of10.5pxis therefore always overridden by the11pxfloor, and only the French override of12pxchanges rendering. Either raise the root value to the intended floor and drop themax()calls, or set the root value to11pxso the declaration matches the behavior.🤖 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 `@app/globals.css` around lines 52 - 57, Update the base --minimum-font-size declaration in the global styles to 11px so it matches the effective minimum enforced by all max(11px, var(--minimum-font-size)) consumers, while preserving the French 12px override.components/EntryWizard.tsx (1)
158-164: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider debouncing the diff computation.
activeDiffTokensrecomputescomputeReadOnlyDiffon every keystroke in the final-text textarea.draft_frandfinal_fraccept up to 8000 characters each, anddiffWordsruns on the main thread. On long texts this can add input lag. DebounceactiveFinalDraftbefore the diff, or compute the diff only against the savedfinal_fr.🤖 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 `@components/EntryWizard.tsx` around lines 158 - 164, Reduce input lag in the activeDiffTokens computation by debouncing activeFinalDraft before passing it to computeReadOnlyDiff, or by using the saved activePhoto.final_fr value for the diff instead. Preserve the existing empty-token behavior when no active final text exists and update the relevant memo dependencies accordingly.components/CorrectionAnnotationEditor.tsx (1)
125-145: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftEvery word becomes a separate tab stop.
Each token renders with
role="button"andtabIndex={0}. The final French text can hold up to 8000 characters, so keyboard users must press Tab through hundreds of tokens to reach the toolbar and the save button. Use a roving tabindex: keeptabIndex={0}on one token andtabIndex={-1}on the rest, and move focus with the arrow keys. Alternatively, make the surface one focusable region and select ranges with keyboard commands inside it.🤖 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 `@components/CorrectionAnnotationEditor.tsx` around lines 125 - 145, The token list in CorrectionAnnotationEditor is creating a tab stop for every word. Implement roving tabindex so only the active token has tabIndex={0}, all others use tabIndex={-1}, and arrow-key handlers move focus and update the active token while preserving Enter/Space range selection. Ensure the toolbar and save button remain reachable without tabbing through every token.app/entries/new/page.tsx (1)
564-575: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMark the active photo tab for assistive technology.
The tab buttons show the active state only through the
pill-activeclass. Addaria-currentso screen-reader users learn which photo is active.♻️ Proposed refactor
className={index === activeIndex ? "pill pill-active" : "pill"} + aria-current={index === activeIndex ? "true" : undefined} onClick={() => setActiveIndex(index)}🤖 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 `@app/entries/new/page.tsx` around lines 564 - 575, Update the photo tab buttons rendered in the photos.map block to expose the active state through aria-current, setting it for the button whose index matches activeIndex and leaving inactive buttons unset. Preserve the existing className and setActiveIndex behavior.
🤖 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 `@app/api/entries/`[id]/export/pdf/route.ts:
- Line 17: Use the shared `export` rate-limit key in both the PDF route’s
`assertRateLimit` call and the PPTX route’s corresponding call, preserving the
existing budget and interval so the limit applies across all export formats.
In `@app/entries/new/page.tsx`:
- Around line 658-664: Update the input rendered by replaceInputRef to remove it
from keyboard and screen-reader navigation while preserving programmatic
activation from the “Remplacer cette photo” button; add the appropriate
non-interactive tab/accessibility handling alongside the existing
visually-hidden class.
In `@app/globals.css`:
- Line 1817: Update the font-family declaration to remove quotes from the valid
family names Manrope and Avenir Next, while preserving the existing fallback
order and sans-serif fallback.
- Around line 70-80: Update the .visually-hidden rule by replacing the
deprecated clip declaration with clip-path: inset(50%). Preserve overflow:
hidden and all other existing accessibility-related declarations unchanged.
In `@components/EntryWizard.tsx`:
- Around line 231-238: Update the useEffect around loadAll to invalidate the
previous load during cleanup when id or router changes. Add an ignore flag (or
request-abort mechanism) and ensure loadAll checks it before every state setter
for photos, drafts, annotations, errors, and related loading state, preventing
stale responses from updating the current entry.
- Around line 373-401: Update saveFinalText so unchanged final text preserves
pending annotation edits: include the current activeAnnotations in the PATCH
when textChanged is false, or avoid replacing local annotations and clearing
annotationDirtyByPhotoId in that branch. Ensure exportFile still recognizes
those edits as dirty until they are successfully persisted.
In `@components/LanguageProvider.tsx`:
- Around line 59-66: Serialize the PATCH requests initiated by setLanguage so
each language persistence write waits for the previous one to settle before
starting, preserving selection order on the server. Keep the immediate
setLanguageState update and unauthenticated early return unchanged, and maintain
the existing error-swallowing behavior for failed requests.
In `@lib/diff/read-only.ts`:
- Around line 27-36: The token reconstruction path in reconstructDiffSides must
preserve original whitespace exactly. Replace the whitespace-insensitive
diffWords usage at lib/diff/read-only.ts:27-36 with diffWordsWithSpace, and add
a whitespace-only reconstruction case at tests/diff-readonly.test.ts:19-29
covering differences such as repeated spaces or newline versus space.
In `@lib/pdf/generator.ts`:
- Around line 230-241: Update embedPhoto and its drawPhoto call sites to cache
each embedded photo per PDFDocument and reuse the cached PDFImage across the
grid, Étape 1, and Étape 4 renders. Perform the base64 decode, sharp PNG
conversion, and pdf.embedPng only once per document/photo, and resize the PNG to
the maximum dimensions needed by the 6-inch panel before embedding.
- Around line 23-30: Resolve the font used by regularFontPath through the module
system or move it to a repository-relative asset instead of constructing a
process.cwd()-based node_modules path. Update next.config.mjs tracing includes
to reference the resulting font location, and adjust package.json only as needed
to remove or retain the font dependency consistently with the chosen approach.
- Around line 214-227: Replace heading-based font selection in the rendered text
path with a shared content-based font selection helper, reusing the existing CJK
detection behavior. Apply this helper consistently to text drawn by
drawTextPanel, drawSlideTitle, drawAnnotatedTextPanel, and the shown body block,
ensuring user text with CJK characters uses fonts.japanese instead of
fonts.regular or fonts.bold.
In `@supabase/migrations/202608050001_allow_final_fr_edits.sql`:
- Around line 37-47: Freeze final_fr during export by updating the trigger logic
in supabase/migrations/202608050001_allow_final_fr_edits.sql lines 37-47 to
reject changes whenever new.status is EXPORTED, including transitions from
FINAL_FR_READY, while preserving the existing immutable source-field checks. Add
a state-machine test in tests/state-machine.test.ts lines 27-34 that changes
final_fr while setting status to EXPORTED and asserts the trigger rejects the
update.
In `@tests/learning-notes.test.ts`:
- Around line 25-27: Update the cleanup in the test’s finally block to restore
OPENAI_API_KEY whenever originalKey is defined, including an empty string; only
delete the environment variable when originalKey is undefined.
---
Nitpick comments:
In `@app/api/exports/`[token]/download/route.ts:
- Line 47: Defer changes to the format-detection logic in the download route;
the current ExportFormat derivation from file.object_path remains acceptable. If
implementing this later, persist the export format on the exports record and use
that recorded value instead of inferring it from the filename extension.
In `@app/entries/new/page.tsx`:
- Around line 564-575: Update the photo tab buttons rendered in the photos.map
block to expose the active state through aria-current, setting it for the button
whose index matches activeIndex and leaving inactive buttons unset. Preserve the
existing className and setActiveIndex behavior.
In `@app/globals.css`:
- Around line 52-57: Update the base --minimum-font-size declaration in the
global styles to 11px so it matches the effective minimum enforced by all
max(11px, var(--minimum-font-size)) consumers, while preserving the French 12px
override.
In `@components/CorrectionAnnotationEditor.tsx`:
- Around line 125-145: The token list in CorrectionAnnotationEditor is creating
a tab stop for every word. Implement roving tabindex so only the active token
has tabIndex={0}, all others use tabIndex={-1}, and arrow-key handlers move
focus and update the active token while preserving Enter/Space range selection.
Ensure the toolbar and save button remain reachable without tabbing through
every token.
In `@components/EntryWizard.tsx`:
- Around line 158-164: Reduce input lag in the activeDiffTokens computation by
debouncing activeFinalDraft before passing it to computeReadOnlyDiff, or by
using the saved activePhoto.final_fr value for the diff instead. Preserve the
existing empty-token behavior when no active final text exists and update the
relevant memo dependencies accordingly.
In `@lib/api/schemas.ts`:
- Around line 18-23: Update learningHighlightsSchema to derive its z.literal
version value from the shared CORRECTION_ANNOTATION_VERSION exported by
annotations.ts instead of hardcoding 2, keeping schema validation aligned with
the normalizer.
In `@lib/exports/presentation.ts`:
- Around line 86-92: Update computePhotoGrid so values of n greater than 10 are
clamped to the supported 10-photo layout before selecting the grid dimensions,
preserving the existing layouts for counts from 1 through 10 and ensuring the
returned grid always has capacity for every generated photo.
In `@lib/learning/annotations.ts`:
- Around line 36-43: Update getAnnotationTextSignature to derive its signature
prefix from CORRECTION_ANNOTATION_VERSION instead of the hardcoded v1 literal,
ensuring the prefix remains aligned with the annotation version and preserving
the existing signature format.
In `@lib/pptx/generator.ts`:
- Around line 366-379: Update the rendering loop around measured.placements to
create one slide.addText text box containing text runs rather than one shape per
placement. Preserve the existing placement-based layout for known-range
outlines, and assign each run’s position-independent styling plus its annotation
background through pptxgenjs highlight using the appropriate placement color.
In `@tests/export-content.test.ts`:
- Around line 165-194: Add a Japanese-text PDF test alongside the existing
generatePhotoTextePdf coverage, using Japanese characters in titleFr and draftFr
while preserving the current PDF/PPTX page-count, dimensions, and header
assertions. Ensure the case exercises font selection based on text content and
fails with the current standard-font path before the generator fix.
🪄 Autofix
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 Plus
Run ID: ef27f2aa-f104-4108-865a-e7f53d7d7734
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (41)
app/api/entries/[id]/diff/route.tsapp/api/entries/[id]/export/pdf/route.tsapp/api/entries/[id]/export/pptx/route.tsapp/api/entries/[id]/memos/auto/route.tsapp/api/entries/[id]/photos/[photoId]/diff/route.tsapp/api/exports/[token]/download/route.tsapp/api/me/route.tsapp/entries/new/page.tsxapp/globals.cssapp/layout.tsxapp/settings/page.tsxcomponents/CorrectionAnnotationEditor.tsxcomponents/DiffReadOnly.tsxcomponents/EntriesDashboard.tsxcomponents/EntryDiffComparison.tsxcomponents/EntryWizard.tsxcomponents/LanguageProvider.tsxcomponents/TopNav.tsxcomponents/UnknownWords.tsxlib/ai/client.tslib/api/response.tslib/api/schemas.tslib/diff/read-only.tslib/exports/download.tslib/exports/presentation.tslib/learning/annotations.tslib/learning/context.tslib/learning/highlight.tslib/pdf/generator.tslib/pptx/download.tslib/pptx/generator.tslib/workflows/export.tsnext.config.mjspackage.jsonsupabase/migrations/202608050001_allow_final_fr_edits.sqltests/correction-annotations.test.tstests/diff-readonly.test.tstests/export-content.test.tstests/learning-highlight.test.tstests/learning-notes.test.tstests/state-machine.test.ts
💤 Files with no reviewable changes (6)
- components/UnknownWords.tsx
- components/DiffReadOnly.tsx
- lib/pptx/download.ts
- lib/learning/context.ts
- tests/learning-highlight.test.ts
- lib/learning/highlight.ts
| try { | ||
| const { id } = await context.params; | ||
| const { user, client } = await authedClient(req); | ||
| assertRateLimit(user.id, 'pdf_export', 12, 60_000); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Share one rate-limit bucket across export formats.
The PPTX route uses the key pptx_export with the same budget. Because the keys differ, one user can start 24 exports per minute across the two routes. Each export renders a full presentation and converts every photo through sharp on the request thread. Use one export key so the budget covers total export cost.
🛡️ Proposed change
- assertRateLimit(user.id, 'pdf_export', 12, 60_000);
+ assertRateLimit(user.id, 'export', 12, 60_000);Apply the same key in app/api/entries/[id]/export/pptx/route.ts.
🤖 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 `@app/api/entries/`[id]/export/pdf/route.ts at line 17, Use the shared `export`
rate-limit key in both the PDF route’s `assertRateLimit` call and the PPTX
route’s corresponding call, preserving the existing budget and interval so the
limit applies across all export formats.
| <input | ||
| ref={replaceInputRef} | ||
| type="file" | ||
| accept="image/*" | ||
| onChange={handleReplaceFileChange} | ||
| className="visually-hidden" | ||
| /> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the hidden replace input from the tab order.
.visually-hidden hides the input visually but keeps it focusable. The input has no label and no accessible name, so keyboard and screen-reader users reach an unnamed file control. The visible "Remplacer cette photo" button already triggers it.
🛡️ Proposed fix
<input
ref={replaceInputRef}
type="file"
accept="image/*"
onChange={handleReplaceFileChange}
className="visually-hidden"
+ tabIndex={-1}
+ aria-hidden="true"
/>📝 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.
| <input | |
| ref={replaceInputRef} | |
| type="file" | |
| accept="image/*" | |
| onChange={handleReplaceFileChange} | |
| className="visually-hidden" | |
| /> | |
| <input | |
| ref={replaceInputRef} | |
| type="file" | |
| accept="image/*" | |
| onChange={handleReplaceFileChange} | |
| className="visually-hidden" | |
| tabIndex={-1} | |
| aria-hidden="true" | |
| /> |
🤖 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 `@app/entries/new/page.tsx` around lines 658 - 664, Update the input rendered
by replaceInputRef to remove it from keyboard and screen-reader navigation while
preserving programmatic activation from the “Remplacer cette photo” button; add
the appropriate non-interactive tab/accessibility handling alongside the
existing visually-hidden class.
| .visually-hidden { | ||
| position: absolute !important; | ||
| width: 1px !important; | ||
| height: 1px !important; | ||
| padding: 0 !important; | ||
| margin: -1px !important; | ||
| overflow: hidden !important; | ||
| clip: rect(0, 0, 0, 0) !important; | ||
| white-space: nowrap !important; | ||
| border: 0 !important; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Replace the deprecated clip property.
Stylelint reports clip as deprecated. Use clip-path: inset(50%) instead. Keep overflow: hidden so the content stays hidden in browsers that ignore clip-path.
♻️ Proposed fix
overflow: hidden !important;
- clip: rect(0, 0, 0, 0) !important;
+ clip-path: inset(50%) !important;
white-space: nowrap !important;📝 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.
| .visually-hidden { | |
| position: absolute !important; | |
| width: 1px !important; | |
| height: 1px !important; | |
| padding: 0 !important; | |
| margin: -1px !important; | |
| overflow: hidden !important; | |
| clip: rect(0, 0, 0, 0) !important; | |
| white-space: nowrap !important; | |
| border: 0 !important; | |
| } | |
| .visually-hidden { | |
| position: absolute !important; | |
| width: 1px !important; | |
| height: 1px !important; | |
| padding: 0 !important; | |
| margin: -1px !important; | |
| overflow: hidden !important; | |
| clip-path: inset(50%) !important; | |
| white-space: nowrap !important; | |
| border: 0 !important; | |
| } |
🧰 Tools
🪛 Stylelint (17.14.0)
[error] 77-77: Deprecated property "clip" (property-no-deprecated)
(property-no-deprecated)
🤖 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 `@app/globals.css` around lines 70 - 80, Update the .visually-hidden rule by
replacing the deprecated clip declaration with clip-path: inset(50%). Preserve
overflow: hidden and all other existing accessibility-related declarations
unchanged.
Source: Linters/SAST tools
| border-radius: 14px; | ||
| background: #f8fafc; | ||
| color: #0f172a; | ||
| font-family: "Manrope", "Avenir Next", sans-serif; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the quotes around the font family names.
Stylelint reports font-family-name-quotes for "Manrope". Manrope and Avenir Next are valid unquoted CSS identifiers.
♻️ Proposed fix
- font-family: "Manrope", "Avenir Next", sans-serif;
+ font-family: Manrope, Avenir Next, sans-serif;📝 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.
| font-family: "Manrope", "Avenir Next", sans-serif; | |
| font-family: Manrope, Avenir Next, sans-serif; |
🧰 Tools
🪛 Stylelint (17.14.0)
[error] 1817-1817: Expected no quotes around "Manrope" (font-family-name-quotes)
(font-family-name-quotes)
🤖 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 `@app/globals.css` at line 1817, Update the font-family declaration to remove
quotes from the valid family names Manrope and Avenir Next, while preserving the
existing fallback order and sans-serif fallback.
Source: Linters/SAST tools
| useEffect(() => { | ||
| if (!getAccessToken()) { | ||
| router.replace("/login"); | ||
| return; | ||
| } | ||
| setMemoDraft(""); | ||
| setMemoDraftTouched(false); | ||
| setMemoPendingSave(false); | ||
| setMemoSavedAt(null); | ||
| memoAutoRequestedRef.current = null; | ||
| loadAll().catch((err) => setError((err as Error).message)); | ||
| void loadAll().catch((err) => setError((err as Error).message)); | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [id, router]); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Cancel the initial load when the effect re-runs.
loadAll calls several state setters after await. The effect depends on id and router. If id changes before the requests resolve, the earlier response still writes photos, drafts, and annotations for the previous entry. Add an ignore flag and check it before each setter, or abort the requests in the cleanup function.
🛡️ Proposed fix
useEffect(() => {
if (!getAccessToken()) {
router.replace("/login");
return;
}
- void loadAll().catch((err) => setError((err as Error).message));
+ let active = true;
+ void loadAll(() => active).catch((err) => {
+ if (active) setError((err as Error).message);
+ });
+ return () => {
+ active = false;
+ };
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [id, router]);loadAll must then check the predicate before it calls each setter.
🧰 Tools
🪛 React Doctor (0.9.3)
[error] 231-231: This setter runs after await, so overlapping re-runs of the effect can resolve out of order and write stale state; gate it behind a cancellation/ignore flag or return a cleanup that cancels the work.
In a useEffect whose dependencies can change, guard any setter call that runs after an await behind a cancellation/ignore flag, or return a cleanup that cancels the async work.
(no-set-state-after-await-in-effect)
🤖 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 `@components/EntryWizard.tsx` around lines 231 - 238, Update the useEffect
around loadAll to invalidate the previous load during cleanup when id or router
changes. Add an ignore flag (or request-abort mechanism) and ensure loadAll
checks it before every state setter for photos, drafts, annotations, errors, and
related loading state, preventing stale responses from updating the current
entry.
Source: Linters/SAST tools
| const fitted = fitPresentationText(content || " ", 16, maxChars, 7); | ||
| const bodyFont = heading.includes("japonais") || heading.includes("(JP)") | ||
| ? fonts.japanese | ||
| : fonts.regular; | ||
| drawTextBlock(page, fitted.text, { | ||
| x: area.x + layout.textPad, | ||
| y: area.y + 0.45, | ||
| w: area.w - layout.textPad * 2, | ||
| h: area.h - 0.55, | ||
| }, { | ||
| font: bodyFont, | ||
| size: fitted.size, | ||
| minSize: 5, | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
pdf-lib StandardFonts Helvetica WinAnsi cannot encode error non-Latin characters
💡 Result:
The error "WinAnsi cannot encode" occurs in pdf-lib because the library's built-in Standard Fonts (like Helvetica, Times-Roman, Courier) use the WinAnsi encoding, which is restricted to a limited subset of Latin characters [1][2][3]. These fonts do not support Unicode characters beyond this legacy encoding, and the library throws an error when it encounters unsupported characters—such as those from Cyrillic, Asian, or other non-Latin scripts—instead of rendering them as empty boxes [4][2]. To resolve this issue, you must embed a custom font that supports the required Unicode characters [5][4][2]. Recommended Solutions: 1. Embed a Custom Font: Use the embedFont method with a TrueType (.ttf) or OpenType (.otf) font file that includes the necessary character glyphs [5][4]. const fontBytes = await fs.promises.readFile('path/to/your/font.ttf'); const customFont = await pdfDoc.embedFont(fontBytes); page.drawText('Your text here', { font: customFont }); 2. Filter Unsupported Characters: If you must use Standard Fonts and cannot embed a custom font, you can filter the input string to remove or replace unsupported characters [1]. You can check which characters are supported by a font using the getCharacterSet method: const charSet = font.getCharacterSet; // Filter or replace unsupported characters in your string before drawing const sanitizedText = text.split('').map(char => charSet.includes(char)? char: '?').join(''); Note: Simply replacing characters with a fallback (like "?") is a common workaround but will result in the loss of original data [1][6]. Embedding a comprehensive font (e.g., Noto Sans) is the standard approach for full Unicode support [5][7].
Citations:
- 1: Error: WinAnsi cannot encode Hopding/pdf-lib#548
- 2: Error when using emojis or special characters with standard font Hopding/pdf-lib#217
- 3: https://github.com/Hopding/pdf-lib/blob/master/src/core/embedders/StandardFontEmbedder.ts
- 4: Draw text with non english alphabet Hopding/pdf-lib#211
- 5: Standard fonts in pdf-lib cannot encode certain characters outside WinAnsi Hopding/pdf-lib#1759
- 6: Improve unicode support Hopding/pdf-lib#1528
- 7: Error: WinAnsi cannot encode with some non-latin text dropdown value Hopding/pdf-lib#1152
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)lib/pdf/generator\.ts$|(^|/)package\.json$|package-lock\.json$|pnpm-lock\.yaml$|yarn\.lock$' || true
echo "== generator outline =="
ast-grep outline lib/pdf/generator.ts --view compact || true
echo "== relevant draws and font selection =="
rg -n "drawTextPanel|drawSlideTitle|drawAnnotatedTextPanel|fitPresentationText|fonts\\.(regular|bold|japanese)|japonais|\\(JP\\)|widthOfTextAtSize|wrapParagraph" lib/pdf/generator.ts
echo "== font/loading section =="
sed -n '1,80p' lib/pdf/generator.ts
echo "== relevant page render sections =="
sed -n '150,250p' lib/pdf/generator.ts
sed -n '280,330p' lib/pdf/generator.ts
sed -n '380,420p' lib/pdf/generator.ts
sed -n '470,530p' lib/pdf/generator.tsRepository: mani1261790/PHOTO-TEXTE
Length of output: 13918
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== exact drawAnnotatedTextPanel =="
sed -n '338,415p' lib/pdf/generator.ts
echo "== exact drawPhotoSteps section =="
sed -n '300,360p' lib/pdf/generator.ts
sed -n '430,485p' lib/pdf/generator.ts
echo "== package pdf-lib version =="
node - <<'JS'
const fs = require('fs');
const p = JSON.parse(fs.readFileSync('package.json','utf8'));
console.log({ dependencies: p.dependencies?.['pdf-lib'], devDependencies: p.devDependencies?.['pdf-lib'] });
const lock = JSON.parse(fs.readFileSync('package-lock.json','utf8'));
for (const key of ['dependencies/pdf-lib','node_modules/pdf-lib']) {
console.log(key, lock?.packages?.[key]?.version ?? lock?.dependencies?.['pdf-lib']?.version);
}
JS
echo "== deterministic content/font selection probe from source =="
python3 - <<'PY'
from pathlib import Path
src = Path('lib/pdf/generator.ts').read_text()
for fn in ['drawTextPanel', 'drawSlideTitle', 'drawAnnotatedTextPanel']:
i = src.index(f'function {fn}')
j = src.find('\nfunction ', i+1)
if j == -1:
j = src.find('\nasync function ', i+1)
body = src[i:j if j != -1 else len(src)]
print(f'-- {fn} --')
for line in body.splitlines()[:35]:
if 'fonts.' in line or 'drawTextPanel' in line or 'drawTextBlock' in line or 'drawHeading' in line or 'placement.text' in line:
print(line.strip())
print('-- CJK helpers/tests in source --')
for s in ['hasCJK', /[\p{Script=Han}\p{Script=Hiragana}\p{Script=Katakana}]/u.test', 'japonais', '(JP)']:
if isinstance(s,str):
print(s, "=>", s in src)
else:
print(s, "=>", src.count(str(s)))
PYRepository: mani1261790/PHOTO-TEXTE
Length of output: 7721
Select the font from rendered text, not from the heading.
fonts.regular and fonts.bold map to pdf-lib Standard Fonts, which throw for characters outside WinAnsi, including CJK. User text can reach these fonts through draftFr and finalFr in drawTextPanel, titles in drawSlideTitle, and placement text in drawAnnotatedTextPanel. Use a single content-based font selection helper for all draw calls, as the existing CJK check does elsewhere.
🤖 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 `@lib/pdf/generator.ts` around lines 214 - 227, Replace heading-based font
selection in the rendered text path with a shared content-based font selection
helper, reusing the existing CJK detection behavior. Apply this helper
consistently to text drawn by drawTextPanel, drawSlideTitle,
drawAnnotatedTextPanel, and the shown body block, ensuring user text with CJK
characters uses fonts.japanese instead of fonts.regular or fonts.bold.
| async function embedPhoto(pdf: PDFDocument, data?: string): Promise<PDFImage | null> { | ||
| if (!data) return null; | ||
| const match = data.match(/^data:[^;]+;base64,(.+)$/s); | ||
| if (!match) return null; | ||
| try { | ||
| const source = Buffer.from(match[1], "base64"); | ||
| const png = await sharp(source).png().toBuffer(); | ||
| return await pdf.embedPng(png); | ||
| } catch { | ||
| return null; | ||
| } | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Cache embedded photos per document.
embedPhoto runs on every drawPhoto call. Each photo is drawn three times: on the grid page (line 323), on the Étape 1 slide (line 443), and on the Étape 4 slide (line 461). Each call re-decodes the base64 data, re-encodes it through sharp, and embeds a separate image object. For 10 photos this triples the sharp work and stores 30 image objects instead of 10, which inflates the PDF size. The PNG re-encode also has no resize, so a full-resolution photo is stored far larger than the 6-inch panel needs.
⚡ Proposed change
-async function embedPhoto(pdf: PDFDocument, data?: string): Promise<PDFImage | null> {
+const imageCache = new WeakMap<PDFDocument, Map<string, PDFImage | null>>();
+
+async function embedPhoto(pdf: PDFDocument, data?: string): Promise<PDFImage | null> {
if (!data) return null;
+ let cache = imageCache.get(pdf);
+ if (!cache) {
+ cache = new Map();
+ imageCache.set(pdf, cache);
+ }
+ if (cache.has(data)) return cache.get(data)!;
const match = data.match(/^data:[^;]+;base64,(.+)$/s);
if (!match) return null;
try {
const source = Buffer.from(match[1], "base64");
- const png = await sharp(source).png().toBuffer();
- return await pdf.embedPng(png);
+ const png = await sharp(source)
+ .resize({ width: 1600, height: 1600, fit: "inside", withoutEnlargement: true })
+ .png()
+ .toBuffer();
+ const image = await pdf.embedPng(png);
+ cache.set(data, image);
+ return image;
} catch {
+ cache.set(data, null);
return null;
}
}📝 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.
| async function embedPhoto(pdf: PDFDocument, data?: string): Promise<PDFImage | null> { | |
| if (!data) return null; | |
| const match = data.match(/^data:[^;]+;base64,(.+)$/s); | |
| if (!match) return null; | |
| try { | |
| const source = Buffer.from(match[1], "base64"); | |
| const png = await sharp(source).png().toBuffer(); | |
| return await pdf.embedPng(png); | |
| } catch { | |
| return null; | |
| } | |
| } | |
| const imageCache = new WeakMap<PDFDocument, Map<string, PDFImage | null>>(); | |
| async function embedPhoto(pdf: PDFDocument, data?: string): Promise<PDFImage | null> { | |
| if (!data) return null; | |
| let cache = imageCache.get(pdf); | |
| if (!cache) { | |
| cache = new Map(); | |
| imageCache.set(pdf, cache); | |
| } | |
| if (cache.has(data)) return cache.get(data)!; | |
| const match = data.match(/^data:[^;]+;base64,(.+)$/s); | |
| if (!match) return null; | |
| try { | |
| const source = Buffer.from(match[1], "base64"); | |
| const png = await sharp(source) | |
| .resize({ width: 1600, height: 1600, fit: "inside", withoutEnlargement: true }) | |
| .png() | |
| .toBuffer(); | |
| const image = await pdf.embedPng(png); | |
| cache.set(data, image); | |
| return image; | |
| } catch { | |
| cache.set(data, null); | |
| return null; | |
| } | |
| } |
🤖 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 `@lib/pdf/generator.ts` around lines 230 - 241, Update embedPhoto and its
drawPhoto call sites to cache each embedded photo per PDFDocument and reuse the
cached PDFImage across the grid, Étape 1, and Étape 4 renders. Perform the
base64 decode, sharp PNG conversion, and pdf.embedPng only once per
document/photo, and resize the PNG to the maximum dimensions needed by the
6-inch panel before embedding.
| if old.status = 'FINAL_FR_READY' then | ||
| if new.entry_id is distinct from old.entry_id | ||
| or new.user_id is distinct from old.user_id | ||
| or new.position is distinct from old.position | ||
| or new.photo_asset_id is distinct from old.photo_asset_id | ||
| or new.draft_fr is distinct from old.draft_fr | ||
| or new.jp_auto is distinct from old.jp_auto | ||
| or new.jp_intent is distinct from old.jp_intent then | ||
| raise exception 'source fields are immutable after FINAL_FR_READY'; | ||
| end if; | ||
| end if; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Freeze final_fr during the export transition.
A single update can change final_fr and set status to EXPORTED. The old.status = 'EXPORTED' check runs only on later updates. This violates the requirement that final French text is editable only before export.
supabase/migrations/202608050001_allow_final_fr_edits.sql#L37-L47: Reject afinal_frchange whenevernew.status = 'EXPORTED', including a transition fromFINAL_FR_READY.tests/state-machine.test.ts#L27-L34: Add a case that changesfinal_frwhile settingstatustoEXPORTEDand expects the trigger to reject the update.
📍 Affects 2 files
supabase/migrations/202608050001_allow_final_fr_edits.sql#L37-L47(this comment)tests/state-machine.test.ts#L27-L34
🤖 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 `@supabase/migrations/202608050001_allow_final_fr_edits.sql` around lines 37 -
47, Freeze final_fr during export by updating the trigger logic in
supabase/migrations/202608050001_allow_final_fr_edits.sql lines 37-47 to reject
changes whenever new.status is EXPORTED, including transitions from
FINAL_FR_READY, while preserving the existing immutable source-field checks. Add
a state-machine test in tests/state-machine.test.ts lines 27-34 that changes
final_fr while setting status to EXPORTED and asserts the trigger rejects the
update.
| } finally { | ||
| if (originalKey) process.env.OPENAI_API_KEY = originalKey; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Restore OPENAI_API_KEY exactly.
If the original value is '', the cleanup leaves the variable deleted. Restore the original value unless it was undefined.
Proposed fix
} finally {
- if (originalKey) process.env.OPENAI_API_KEY = originalKey;
+ if (originalKey === undefined) {
+ delete process.env.OPENAI_API_KEY;
+ } else {
+ process.env.OPENAI_API_KEY = originalKey;
+ }
}📝 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.
| } finally { | |
| if (originalKey) process.env.OPENAI_API_KEY = originalKey; | |
| } | |
| } finally { | |
| if (originalKey === undefined) { | |
| delete process.env.OPENAI_API_KEY; | |
| } else { | |
| process.env.OPENAI_API_KEY = originalKey; | |
| } | |
| } |
🤖 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 `@tests/learning-notes.test.ts` around lines 25 - 27, Update the cleanup in the
test’s finally block to restore OPENAI_API_KEY whenever originalKey is defined,
including an empty string; only delete the environment variable when originalKey
is undefined.
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (5)
lib/cloudflare/client.ts (1)
401-404: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
Object.hasOwnfor the table allowlist check.
table in TABLESalso returns true for inheritedObject.prototypekeys such asconstructorandtoString.TABLES['constructor']then resolves toObject, and the builder proceeds with an undefinedscope. The path fails later with aTypeErrorinstead of the intendedUnknown D1 tableerror. All current callers pass literal table names, so this is not reachable today.♻️ Proposed fix
from(table: string): QueryBuilderLike<any[]> { - if (!(table in TABLES)) throw new Error(`Unknown D1 table: ${table}`); + if (!Object.hasOwn(TABLES, table)) throw new Error(`Unknown D1 table: ${table}`); return new QueryBuilder(this.env.DB, table as TableName, this.userId) as unknown as QueryBuilderLike<any[]>; }🤖 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 `@lib/cloudflare/client.ts` around lines 401 - 404, Update the allowlist check in QueryBuilderLike.from to use an own-property check via Object.hasOwn instead of the in operator, so inherited keys such as constructor and toString are rejected with the existing Unknown D1 table error.app/api/storage/[bucket]/[...path]/route.ts (1)
21-29: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winWrap the handler body in error handling.
getAppEnv()at line 21 andCONTENT_BUCKET.getat line 26 can reject. This handler has notry/catch, unlike the other routes in this migration, which route failures throughhandleApiError. An unhandled rejection returns a framework 500 that does not match the{ error: { code } }shape used at lines 18, 23, and 28.🤖 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 `@app/api/storage/`[bucket]/[...path]/route.ts around lines 21 - 29, The storage route handler should wrap the environment lookup, signature verification, and bucket fetch in try/catch handling, routing failures through the existing handleApiError utility so rejected operations return the standard error shape. Preserve the current INVALID_SIGNATURE and OBJECT_NOT_FOUND responses for expected conditions.app/api/me/route.ts (2)
106-112: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚖️ Poor tradeoffThe email rollback is unchecked and the two writes are not atomic.
The email lives in the D1
usertable and the encrypted copy lives inuser_profiles. These are two separate writes with no transaction. Lines 108-111 attempt a compensating write but discard its result. If the rollback fails, theusertable holds the new email whileuser_profilesholds the encrypted old email.Consider writing both through a single
env.DB.batchcall so the pair commits or fails together.🤖 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 `@app/api/me/route.ts` around lines 106 - 112, Update the email-change flow around the `nextEmail` branch to replace the separate profile and `user` writes with one `env.DB.batch` operation containing both updates, preserving their existing parameters and ordering so the plaintext and encrypted email changes commit or fail together. Remove the compensating rollback write and its unchecked result.
78-92: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueNormalize the new email lookup and write.
user.emailalready has a unique SQLite constraint, so this cannot create duplicate rows. The read then write is still TOCTOU, but theUNIQUEconstraint enforces the final invariant; the remaining issue is that case-insensitive lookup can write mixed-case emails and diverge from migrated lowercase values. Use lowercasenextEmailfor both theSELECTcheck and theUPDATEbind, and let the constraint handle the uniqueness result.🤖 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 `@app/api/me/route.ts` around lines 78 - 92, Normalize nextEmail to lowercase before the duplicate check and update in the profile update flow. Use the normalized value for the SELECT bind and the UPDATE bind, while retaining the existing uniqueness handling and success checks.components/TopNav.tsx (1)
42-47: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winGuard repeated logout submissions.
handleLogoutremains active whilefetchwaits. Two clicks can send twoPOST /api/auth/sign-outrequests. Add aloggingOutguard and disable the button while the request is pending.🤖 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 `@components/TopNav.tsx` around lines 42 - 47, Update handleLogout in TopNav to guard against concurrent submissions by tracking a loggingOut state, returning early when logout is already pending, and setting the guard before the sign-out fetch and clearing it after completion. Bind the logout button’s disabled state to loggingOut so it remains disabled while the request is pending.Source: Linters/SAST tools
🤖 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 @.env.example:
- Around line 1-15: Reorder the environment variables in the dotenv template
according to dotenv-linter’s expected key order, including the newly added
Supabase-related keys, while preserving every key and its current value or empty
placeholder.
In `@app/api/me/route.ts`:
- Around line 153-161: Check the results of all six delete operations in the
account-deletion flow before removing the user row. Use each returned result’s
error field, fail the request when any deletion reports an error, and only
execute the user deletion and success response after every table deletion
succeeds.
In `@components/TopNav.tsx`:
- Around line 42-47: Update handleLogout so local state clearing and navigation
to /login occur only after the /api/auth/sign-out request succeeds with a
successful response. Remove the catch that suppresses failures, check the fetch
response status, and preserve the current logout transition for successful
sign-outs while leaving it incomplete when the request fails or returns non-2xx.
In `@lib/workflows/export.ts`:
- Around line 49-55: Update loadJapanesePdfFont to read the R2ObjectBody using
its supported arrayBuffer() method instead of bytes(), while preserving the
existing missing-font validation and return type behavior.
In `@package.json`:
- Around line 17-26: Update the package.json scripts.lint entry to invoke the
repository’s chosen linter directly instead of next lint, and add the
corresponding linter dependency and configuration required for it to run. Keep
the existing lint script interface intact while ensuring it targets the project
source files.
In `@README.md`:
- Line 145: Update the README migration instructions by replacing `9利用者全員` with
the appropriate Japanese phrasing: use `9人の利用者全員` for a fixed count, or `全利用者`
when the procedure applies to all users.
In `@scripts/migrate-supabase-to-cloudflare.mjs`:
- Around line 54-80: Update fetchTable to request a stable ascending id order on
every table query before applying the range, and stop pagination when a batch is
empty rather than only when it is short. Also add pre-migration count
comparisons between the source Supabase users/table data and the destination
database, failing or reporting a mismatch before inserts proceed; do not rely on
INSERT OR IGNORE to validate completeness.
- Line 22: Remove the literal fallback from the cloudflareAccountId
initialization and require CLOUDFLARE_ACCOUNT_ID to be present, matching the
validation behavior used for NEXT_PUBLIC_SUPABASE_URL and
SUPABASE_SERVICE_ROLE_KEY. Ensure the migration exits or throws before launching
the wrangler child process when the variable is unset.
---
Nitpick comments:
In `@app/api/me/route.ts`:
- Around line 106-112: Update the email-change flow around the `nextEmail`
branch to replace the separate profile and `user` writes with one `env.DB.batch`
operation containing both updates, preserving their existing parameters and
ordering so the plaintext and encrypted email changes commit or fail together.
Remove the compensating rollback write and its unchecked result.
- Around line 78-92: Normalize nextEmail to lowercase before the duplicate check
and update in the profile update flow. Use the normalized value for the SELECT
bind and the UPDATE bind, while retaining the existing uniqueness handling and
success checks.
In `@app/api/storage/`[bucket]/[...path]/route.ts:
- Around line 21-29: The storage route handler should wrap the environment
lookup, signature verification, and bucket fetch in try/catch handling, routing
failures through the existing handleApiError utility so rejected operations
return the standard error shape. Preserve the current INVALID_SIGNATURE and
OBJECT_NOT_FOUND responses for expected conditions.
In `@components/TopNav.tsx`:
- Around line 42-47: Update handleLogout in TopNav to guard against concurrent
submissions by tracking a loggingOut state, returning early when logout is
already pending, and setting the guard before the sign-out fetch and clearing it
after completion. Bind the logout button’s disabled state to loggingOut so it
remains disabled while the request is pending.
In `@lib/cloudflare/client.ts`:
- Around line 401-404: Update the allowlist check in QueryBuilderLike.from to
use an own-property check via Object.hasOwn instead of the in operator, so
inherited keys such as constructor and toString are rejected with the existing
Unknown D1 table error.
🪄 Autofix
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 Plus
Run ID: 0bc08b86-e53c-4217-8a2c-bd6aab254050
⛔ Files ignored due to path filters (4)
app/apple-icon.pngis excluded by!**/*.pngapp/icon.pngis excluded by!**/*.pngpackage-lock.jsonis excluded by!**/package-lock.jsonpublic/icon-192.pngis excluded by!**/*.png
📒 Files selected for processing (65)
.env.example.gitignoreREADME.mdapp/api/assets/photo/route.tsapp/api/auth/[...all]/route.tsapp/api/auth/login/route.tsapp/api/auth/signup/route.tsapp/api/entries/[id]/diff/route.tsapp/api/entries/[id]/export/pdf/route.tsapp/api/entries/[id]/export/pptx/route.tsapp/api/entries/[id]/lock_intent/route.tsapp/api/entries/[id]/memos/auto/route.tsapp/api/entries/[id]/memos/route.tsapp/api/entries/[id]/photos/[photoId]/diff/route.tsapp/api/entries/[id]/photos/[photoId]/lock_intent/route.tsapp/api/entries/[id]/photos/[photoId]/route.tsapp/api/entries/[id]/photos/[photoId]/translate/route.tsapp/api/entries/[id]/photos/route.tsapp/api/entries/[id]/rewrite/route.tsapp/api/entries/[id]/route.tsapp/api/entries/[id]/translate/route.tsapp/api/entries/multi/route.tsapp/api/entries/route.tsapp/api/exports/[token]/download/route.tsapp/api/internal/supabase-keepalive/route.tsapp/api/me/route.tsapp/api/memos/[id]/route.tsapp/api/storage/[bucket]/[...path]/route.tsapp/apple-icon.tsxapp/entries/new/page.tsxapp/icon-192.tsxapp/icon.tsxapp/login/page.tsxapp/manifest.tscloudflare/migrations/0001_initial.sqlcloudflare/migrations/0002_updated_at_triggers.sqlcloudflare/migrations/0003_photo_limit_trigger.sqlcloudflare/migrations/0004_photo_state_trigger.sqlcomponents/TopNav.tsxlib/api/fetcher.tslib/auth/better-auth.tslib/auth/legacy-supabase.tslib/auth/session.tslib/cloudflare/authed.tslib/cloudflare/client.tslib/cloudflare/context.tslib/cloudflare/storage-signature.tslib/env.tslib/image/sanitize.tslib/pdf/generator.tslib/supabase/client.tslib/workflows/export.tslib/workflows/rewrite.tsnext-env.d.tsnext.config.mjsopen-next.config.tspackage.jsonscripts/migrate-supabase-to-cloudflare.mjstests/export-content.test.tstests/learning-notes.test.tstests/legacy-auth-migration.test.tstests/rls-policy.test.tstsconfig.jsonvercel.jsonwrangler.jsonc
💤 Files with no reviewable changes (7)
- vercel.json
- lib/env.ts
- lib/supabase/client.ts
- app/api/internal/supabase-keepalive/route.ts
- app/icon.tsx
- app/icon-192.tsx
- app/apple-icon.tsx
🚧 Files skipped from review as they are similar to previous changes (9)
- tests/learning-notes.test.ts
- app/api/entries/[id]/export/pdf/route.ts
- app/api/entries/[id]/memos/auto/route.ts
- app/entries/new/page.tsx
- app/api/entries/[id]/diff/route.ts
- app/api/exports/[token]/download/route.ts
- app/api/entries/[id]/export/pptx/route.ts
- tests/export-content.test.ts
- app/api/entries/[id]/photos/[photoId]/diff/route.ts
| APP_MASTER_KEY_B64= | ||
| OPENAI_API_KEY= | ||
| OPENAI_MODEL=gpt-4o-mini | ||
| BETTER_AUTH_SECRET= | ||
| BETTER_AUTH_URL= | ||
| STORAGE_SIGNING_SECRET= | ||
| PHOTO_BUCKET=photos | ||
| EXPORT_BUCKET=exports | ||
|
|
||
| # Supabase migration and temporary first-login bridge only. | ||
| NEXT_PUBLIC_SUPABASE_URL= | ||
| NEXT_PUBLIC_SUPABASE_ANON_KEY= | ||
| SUPABASE_SERVICE_ROLE_KEY= | ||
| LEGACY_SUPABASE_URL= | ||
| LEGACY_SUPABASE_ANON_KEY= |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Restore the dotenv-linter key order.
The supplied dotenv-linter report flags the new keys as unordered. Sort the keys to keep the environment-template lint check clean.
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 4-4: [UnorderedKey] The BETTER_AUTH_SECRET key should go before the OPENAI_API_KEY key
(UnorderedKey)
[warning] 5-5: [UnorderedKey] The BETTER_AUTH_URL key should go before the OPENAI_API_KEY key
(UnorderedKey)
[warning] 7-7: [UnorderedKey] The PHOTO_BUCKET key should go before the STORAGE_SIGNING_SECRET key
(UnorderedKey)
[warning] 8-8: [UnorderedKey] The EXPORT_BUCKET key should go before the OPENAI_API_KEY key
(UnorderedKey)
[warning] 12-12: [UnorderedKey] The NEXT_PUBLIC_SUPABASE_ANON_KEY key should go before the NEXT_PUBLIC_SUPABASE_URL key
(UnorderedKey)
[warning] 14-14: [UnorderedKey] The LEGACY_SUPABASE_URL key should go before the NEXT_PUBLIC_SUPABASE_ANON_KEY key
(UnorderedKey)
[warning] 15-15: [UnorderedKey] The LEGACY_SUPABASE_ANON_KEY key should go before the LEGACY_SUPABASE_URL key
(UnorderedKey)
🤖 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 @.env.example around lines 1 - 15, Reorder the environment variables in the
dotenv template according to dotenv-linter’s expected key order, including the
newly added Supabase-related keys, while preserving every key and its current
value or empty placeholder.
Source: Linters/SAST tools
| await client.from('exports').delete(); | ||
| await client.from('memos').delete(); | ||
| await client.from('entry_photos').delete(); | ||
| await client.from('entries').delete(); | ||
| await client.from('assets').delete(); | ||
| await client.from('user_profiles').delete(); | ||
|
|
||
| const env = await getAppEnv(); | ||
| await env.DB.prepare('DELETE FROM "user" WHERE id = ?').bind(user.id).run(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Check the delete results before deleting the user row.
Each .delete() call returns { data, error } instead of throwing. QueryBuilder.execute in lib/cloudflare/client.ts lines 274-283 converts every failure into an error object. This code discards all six results, then deletes the user row and returns { deleted: true }.
If any table delete fails, the response still reports success. Check each result and fail the request when a delete returns an error.
🐛 Proposed fix
- await client.from('exports').delete();
- await client.from('memos').delete();
- await client.from('entry_photos').delete();
- await client.from('entries').delete();
- await client.from('assets').delete();
- await client.from('user_profiles').delete();
+ for (const table of ['exports', 'memos', 'entry_photos', 'entries', 'assets', 'user_profiles']) {
+ const { error } = await client.from(table).delete();
+ if (error) {
+ badRequest('ACCOUNT_DELETE_FAILED', 'Unable to delete account data');
+ }
+ }
const env = await getAppEnv();
- await env.DB.prepare('DELETE FROM "user" WHERE id = ?').bind(user.id).run();
+ const removal = await env.DB.prepare('DELETE FROM "user" WHERE id = ?').bind(user.id).run();
+ if (!removal.success) {
+ badRequest('ACCOUNT_DELETE_FAILED', 'Unable to delete account');
+ }🤖 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 `@app/api/me/route.ts` around lines 153 - 161, Check the results of all six
delete operations in the account-deletion flow before removing the user row. Use
each returned result’s error field, fail the request when any deletion reports
an error, and only execute the user deletion and success response after every
table deletion succeeds.
| const handleLogout = async () => { | ||
| await fetch('/api/auth/sign-out', { | ||
| method: 'POST', | ||
| headers: { 'content-type': 'application/json' }, | ||
| credentials: 'same-origin' | ||
| }).catch(() => undefined); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate files =="
git ls-files | rg '(^|/)TopNav\.tsx$|^components/|src/lib/auth|lib/auth|app/api/auth|middleware|better-auth|package\.json' || true
echo
echo "== TopNav outline/section =="
if [ -f components/TopNav.tsx ]; then
wc -l components/TopNav.tsx
cat -n components/TopNav.tsx | sed -n '1,140p'
fi
echo
echo "== auth token store =="
if [ -f lib/auth/token-store.ts ]; then
cat -n lib/auth/token-store.ts
fi
echo
echo "== usages of handleLogout/clearAccessToken/sign-out/auth session =="
rg -n "handleLogout|clearAccessToken|sign-out|useAuth|session|HttpOnly|better-auth|accessToken" . --glob '!node_modules' --glob '!dist' --glob '!build' || trueRepository: mani1261790/PHOTO-TEXTE
Length of output: 12648
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== token fetcher =="
cat -n lib/api/fetcher.ts
echo
echo "== Better Auth route and config =="
cat -n app/api/auth/[...all]/route.ts
cat -n lib/auth/better-auth.ts
echo
echo "== lock metadata around better-auth package =="
node - <<'JS'
const fs = require('fs');
const lock = JSON.parse(fs.readFileSync('package-lock.json', 'utf8'));
const pk = lock.packages?.['node_modules/better-auth'];
const pkg = JSON.parse(fs.readFileSync('package.json', 'utf8'));
console.log(JSON.stringify({
packageManager: pkg.packageManager || null,
dependency: pkg.dependencies?.['better-auth'] || null,
locked: pk && { version: pk.version, resolved: pk.resolved },
}, null, 2));
JSRepository: mani1261790/PHOTO-TEXTE
Length of output: 4312
🌐 Web query:
Better Auth version 1.6.26 signOut session cookie HttpOnly session invalidation route /sign-out
💡 Result:
In Better Auth version 1.6.26, the signOut process is handled via the /sign-out endpoint (typically POST /api/auth/sign-out) [1][2][3]. When called, this endpoint invalidates the user's session in the database and clears the associated session cookies [1]. Key technical details regarding the signOut process: 1. Cookie Invalidation: Better Auth explicitly clears session-related cookies upon sign-out [1][4]. This includes the primary session token cookie (often named better-auth.session_token) and, if cookie caching is enabled, the session data cache cookie (better-auth.session_data) [5][6]. The library uses a helper function (deleteSessionCookie) to ensure these are expired in the browser [1][4][6]. 2. HttpOnly Cookies: Better Auth utilizes HttpOnly and secure attributes for its session cookies by default in production environments, which protects them from being accessed via client-side JavaScript [5]. Because the cookies are HttpOnly, the browser automatically handles the storage and transmission, and the /sign-out endpoint clears them by sending Set-Cookie headers with an expiration date in the past [1]. 3. Implementation: - Client-side: You can initiate the sign-out process using the client SDK: await authClient.signOut; [7][2] - Server-side/API: The underlying endpoint is a POST request to /sign-out, which requires the session cookies to be present in the request headers [1][2][3]. If you experience issues where a session appears to persist after sign-out, ensure that your application or framework is correctly forwarding the Set-Cookie headers returned by the /sign-out response to the browser [4]. Frameworks like Next.js or custom reverse proxies can sometimes interfere with cookie propagation if not configured to pass these headers through [4][5]. Recent versions, including 1.6.26, have included fixes for session cleanup and ensuring that both the session_token and session_data cookies are consistently cleared [8][4][9]. [6] confirms through automated integration testing that the sign-out process successfully clears both cookies in environments where cookie caching is active.
Citations:
- 1: https://app.unpkg.com/better-auth@1.6.25/files/dist/api/routes/sign-out.mjs
- 2: https://better-auth.com/docs/authentication/email-password
- 3: https://better-auth.rs/docs/reference/api-routes
- 4: ### Bug: signOut() does not clear session_data (cache) cookie better-auth/better-auth#8273
- 5: https://better-auth.com/docs/concepts/cookies
- 6: https://github.com/better-auth/better-auth/blob/fd6b8c13/e2e/integration/vanilla-node/e2e/cookie-cache-signout.spec.ts
- 7: https://better-auth.com/docs/basic-usage
- 8: https://github.com/better-auth/better-auth/releases/tag/v1.6.26
- 9: tracking: session cookie cache (lifecycle, cleanup, set-cookie propagation) better-auth/better-auth#9177
Broken Authentication (CWE-613): Insufficient Session Expiration
Reachability: External
Do not report logout as complete when the server call fails.
fetch ignores network failures and does not enforce the non-2xx sign-out response, so handleLogout still clears local state and navigates to /login while a failed Better Auth session may still be valid. Complete the UI transition only after /api/auth/sign-out returns success.
🧰 Tools
🪛 React Doctor (0.9.3)
[error] 43-43: This async handler awaits a mutating request and only flips state after the await, so a fast double-click or double Enter fires the request twice. Add a leading if (busy) return guard (or set a flag before the await and disable the control) to close the re-entry window.
An async onClick/onSubmit handler on a host control that awaits a mutating request and sets state only afterward stays interactive across the await, so a double-click fires the write twice. Add a leading if (busy) return guard, or set a flag before the await inside try and reset it in finally while the control is disabled.
(no-async-event-handler-without-reentry-guard)
🤖 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 `@components/TopNav.tsx` around lines 42 - 47, Update handleLogout so local
state clearing and navigation to /login occur only after the /api/auth/sign-out
request succeeds with a successful response. Remove the catch that suppresses
failures, check the fetch response status, and preserve the current logout
transition for successful sign-outs while leaving it incomplete when the request
fails or returns non-2xx.
| async function loadJapanesePdfFont(): Promise<Uint8Array> { | ||
| const env = await getAppEnv(); | ||
| const object = await env.CONTENT_BUCKET.get("system/fonts/NotoSansJP-Regular.ttf"); | ||
| if (!object) { | ||
| badRequest("PDF_FONT_MISSING", "PDF Japanese font is not installed"); | ||
| } | ||
| return object.bytes(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 3 'CONTENT_BUCKET\.get|object\.bytes|arrayBuffer\(' lib/workflows/export.ts
# Confirm the current Cloudflare R2 body API in the official reference.
curl -fsSL 'https://developers.cloudflare.com/r2/api/workers/workers-api-reference/' |
rg -n -C 2 'R2ObjectBody|arrayBuffer|bytes' || trueRepository: mani1261790/PHOTO-TEXTE
Length of output: 50379
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate R2 API type declarations or dependencies without running repository code.
git ls-files | rg -n '(^|/)(wrangler\.json.*|package\.json|package-lock\.json|pnpm-lock\.yaml|yarn\.lock|\.wrangler/types|.*d\.ts|`@cloudflare/workers-types`|workerd|cloudflare|types)' || true
# If local Cloudflare Worker type declarations are present, inspect R2ObjectBody method signatures.
python3 - <<'PY'
import subprocess, re
result = subprocess.run(["git","ls-files"], text=True, stdout=subprocess.PIPE)
files = result.stdout.splitlines()
matches = []
for f in files:
try:
data = open(f, "rt", encoding="utf-8", errors="ignore").read()
except Exception:
continue
if "R2ObjectBody" in data and ("arrayBuffer" in data or ".bytes" in data):
matches.append(f)
print("candidate type files:")
for f in matches[:50]:
print(f)
for f in matches:
try:
lines = open(f, "rt", encoding="utf-8", errors="ignore").readlines()
except Exception:
continue
for i, line in enumerate(lines, 1):
if "R2ObjectBody" in line or ".arrayBuffer" in line or ".bytes" in line:
print(f"\n--- {f}:{i} ---")
for j in range(max(1, i-4), min(len(lines), i+6)):
print(f"{j}: {lines[j-1].rstrip()}")
PYRepository: mani1261790/PHOTO-TEXTE
Length of output: 692
Read the R2 object with arrayBuffer().
R2ObjectBody defines arrayBuffer(), not bytes(), so this call fails the Cloudflare Workers R2 API type contract.
Proposed fix
- return object.bytes();
+ return new Uint8Array(await object.arrayBuffer());📝 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.
| async function loadJapanesePdfFont(): Promise<Uint8Array> { | |
| const env = await getAppEnv(); | |
| const object = await env.CONTENT_BUCKET.get("system/fonts/NotoSansJP-Regular.ttf"); | |
| if (!object) { | |
| badRequest("PDF_FONT_MISSING", "PDF Japanese font is not installed"); | |
| } | |
| return object.bytes(); | |
| async function loadJapanesePdfFont(): Promise<Uint8Array> { | |
| const env = await getAppEnv(); | |
| const object = await env.CONTENT_BUCKET.get("system/fonts/NotoSansJP-Regular.ttf"); | |
| if (!object) { | |
| badRequest("PDF_FONT_MISSING", "PDF Japanese font is not installed"); | |
| } | |
| return new Uint8Array(await object.arrayBuffer()); |
🤖 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 `@lib/workflows/export.ts` around lines 49 - 55, Update loadJapanesePdfFont to
read the R2ObjectBody using its supported arrayBuffer() method instead of
bytes(), while preserving the existing missing-font validation and return type
behavior.
| "lint": "next lint", | ||
| "test": "vitest run" | ||
| }, | ||
| "dependencies": { | ||
| "@supabase/supabase-js": "^2.49.1", | ||
| "diff": "^7.0.0", | ||
| "@opennextjs/cloudflare": "^1.20.2", | ||
| "@pdf-lib/fontkit": "^1.1.1", | ||
| "better-auth": "^1.6.26", | ||
| "diff": "8.0.3", | ||
| "jszip": "^3.10.1", | ||
| "next": "^16.1.6", | ||
| "next": "16.3.0", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
According to the official Next.js 16.3.0 documentation, confirm that next lint is removed and identify the supported replacement command.
💡 Result:
The next lint command has been removed in Next.js 16 [1][2]. There is no direct "replacement" command provided by Next.js; instead, you should use the official CLI for your chosen linter directly [1][2]. For most projects, this means invoking the ESLint CLI (e.g., eslint .) or the Biome CLI directly [1][2]. Additionally, automatic linting during the next build process has been removed, providing developers with full control over when and how linting is executed [3][4][2]. To migrate existing projects, Next.js provides a codemod: npx @next/codemod@canary next-lint-to-eslint-cli [1][2].
Citations:
- 1: https://nextjs.org/docs/app/guides/upgrading/version-16
- 2: https://nextjs.org/blog/next-16
- 3: Remove automatic linting from build vercel/next.js#83136
- 4: Remove automatic linting from build vercel/next.js#83136
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== package.json scripts and lint deps =="
python3 - <<'PY'
import json
from pathlib import Path
p=Path('package.json')
data=json.loads(p.read_text())
for section in ['scripts','devDependencies','dependencies']:
vals=data.get(section,{})
if section=='scripts':
for k,v in vals.items():
print(f"{section}.{k}={v}")
else:
print(f"## {section}")
for k in sorted(vals):
print(f"{k}={vals[k]}")
PY
echo
echo "== lint config files =="
git ls-files | grep -E '(^|/)(eslint\.|\.eslintrc(\..*)?|biome|biome\.)|package-lock\.json|pnpm-lock\.yaml|yarn\.lock$' | sort
echo
echo "== references to next lint / lint command / lint config =="
rg -n '"next lint"|next lint|eslint|biome|`@next/codemod`' package.json .eslintrc* biome.json biome.* package-lock.json 2>/dev/null || trueRepository: mani1261790/PHOTO-TEXTE
Length of output: 1420
Replace the removed next lint command.
scripts.lint still calls next lint, but Next.js 16.3.0 no longer provides that command. Update the script to run ESLint/Biome directly and add the matching config, since the repo currently has no ESLint/Biome dependencies or config.
🤖 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 17 - 26, Update the package.json scripts.lint
entry to invoke the repository’s chosen linter directly instead of next lint,
and add the corresponding linter dependency and configuration required for it to
run. Keep the existing lint script interface intact while ensuring it targets
the project source files.
| 3. 成功時にBetter Auth形式でパスワードをハッシュし、D1へ保存 | ||
| 4. 以後はCloudflareだけでログイン | ||
|
|
||
| 移行期間中はCloudflareへ `LEGACY_SUPABASE_URL` と `LEGACY_SUPABASE_ANON_KEY` を登録します。9利用者全員にCredentialが作成されたことを確認した後、この2値とSupabaseプロジェクトを削除できます。 |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the Japanese wording.
Replace 9利用者全員 with 9人の利用者全員 if the count is fixed, or 全利用者 if the procedure applies to all users.
🤖 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 `@README.md` at line 145, Update the README migration instructions by replacing
`9利用者全員` with the appropriate Japanese phrasing: use `9人の利用者全員` for a fixed
count, or `全利用者` when the procedure applies to all users.
| const supabaseUrl = process.env.NEXT_PUBLIC_SUPABASE_URL?.replace(/\/$/, ''); | ||
| const serviceKey = process.env.SUPABASE_SERVICE_ROLE_KEY; | ||
| const r2Bucket = process.env.CLOUDFLARE_R2_BUCKET ?? 'photo-texte-content'; | ||
| const cloudflareAccountId = process.env.CLOUDFLARE_ACCOUNT_ID ?? '2ea670c2a6ff28e248ef084adf095e8b'; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check whether the account id appears elsewhere and whether it is documented.
rg -n "2ea670c2a6ff28e248ef084adf095e8b|CLOUDFLARE_ACCOUNT_ID" -g '!node_modules'Repository: mani1261790/PHOTO-TEXTE
Length of output: 161
🏁 Script executed:
#!/bin/bash
set -u
echo "== repo files matching migrate script =="
git ls-files | rg 'migrate-supabase-to-cloudflare\.mjs|scripts/migrate|CONTL\.*(md|txt)|\.env|example' || true
echo
echo "== inspect candidate script if present =="
if [ -f scripts/migrate-supabase-to-cloudflare.mjs ]; then
nl -ba scripts/migrate-supabase-to-cloudflare.mjs | sed -n '1,180p'
fi
echo
echo "== search for hardcoded cloudflare account-ish strings =="
rg -n "2ea670c2a6ff28e248ef084adf095e8b|account.*(id|identifier)|cloudflare|CLOUDFLARE|SUPABASE_SERVICE_ROLE|NEXT_PUBLIC_SUPABASE" -g '!node_modules' . || trueRepository: mani1261790/PHOTO-TEXTE
Length of output: 10345
🏁 Script executed:
#!/bin/bash
set -u
echo "== git status and diff stat =="
git status --short
git diff --stat || true
echo
echo "== show exact diff around environment handling =="
git diff -- scripts/migrate-supabase-to-cloudflare.mjs | sed -n '1,220p' || trueRepository: mani1261790/PHOTO-TEXTE
Length of output: 243
🏁 Script executed:
#!/bin/bash
set -u
echo "== candidate script with line numbers =="
awk '{printf "%6d\t%s\n", NR, $0}' scripts/migrate-supabase-to-cloudflare.mjs | sed -n '1,180p'
echo
echo "== .env.example =="
awk '{printf "%6d\t%s\n", NR, $0}' .env.example | sed -n '1,180p'
echo
echo "== wrangler account_id =="
awk '{printf "%6d\t%s\n", NR, $0}' wrangler.jsonc | sed -n '1,40p'
echo
echo "== env documentation around env variables =="
awk '{printf "%6d\t%s\n", NR, $0}' README.md | sed -n '45,60p;105,140p'Repository: mani1261790/PHOTO-TEXTE
Length of output: 11021
Security Misconfiguration (CWE-1188): Insecure Default Initialization of Resource
Reachability: Internal
Remove the hardcoded Cloudflare account id default.
CLOUDFLARE_ACCOUNT_ID falls back to a literal account id when unset, then passes that value into the wrangler child process. Require the variable, like NEXT_PUBLIC_SUPABASE_URL and SUPABASE_SERVICE_ROLE_KEY, so an omitted migration environment variable fails before wrangler runs.
🛡️ Proposed fix
-const cloudflareAccountId = process.env.CLOUDFLARE_ACCOUNT_ID ?? '2ea670c2a6ff28e248ef084adf095e8b';
-if (!supabaseUrl || !serviceKey) throw new Error('Supabase migration credentials are missing');
+const cloudflareAccountId = process.env.CLOUDFLARE_ACCOUNT_ID;
+if (!supabaseUrl || !serviceKey) throw new Error('Supabase migration credentials are missing');
+if (!cloudflareAccountId) throw new Error('CLOUDFLARE_ACCOUNT_ID is 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 `@scripts/migrate-supabase-to-cloudflare.mjs` at line 22, Remove the literal
fallback from the cloudflareAccountId initialization and require
CLOUDFLARE_ACCOUNT_ID to be present, matching the validation behavior used for
NEXT_PUBLIC_SUPABASE_URL and SUPABASE_SERVICE_ROLE_KEY. Ensure the migration
exits or throws before launching the wrangler child process when the variable is
unset.
| async function fetchAuthUsers() { | ||
| const users = []; | ||
| for (let page = 1; ; page += 1) { | ||
| const body = await fetchJson(`${supabaseUrl}/auth/v1/admin/users?page=${page}&per_page=1000`, { | ||
| headers: supabaseHeaders | ||
| }); | ||
| const batch = body.users ?? []; | ||
| users.push(...batch); | ||
| if (batch.length < 1000) return users; | ||
| } | ||
| } | ||
|
|
||
| async function fetchTable(table) { | ||
| const rows = []; | ||
| for (let offset = 0; ; offset += 1000) { | ||
| const response = await fetch(`${supabaseUrl}/rest/v1/${table}?select=*`, { | ||
| headers: { | ||
| ...supabaseHeaders, | ||
| range: `${offset}-${offset + 999}` | ||
| } | ||
| }); | ||
| if (!response.ok) throw new Error(`Unable to export ${table} (${response.status})`); | ||
| const batch = await response.json(); | ||
| rows.push(...batch); | ||
| if (batch.length < 1000) return rows; | ||
| } | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Supabase Admin API listUsers per_page maximum limit
💡 Result:
The Supabase Admin API listUsers method allows you to paginate results using the per_page parameter [1][2]. While the default value for per_page is 50 [3][4], the maximum limit allowed for per_page is 1000 [5][1][2]. When using the Supabase client libraries (such as supabase-js), you can pass this parameter within the options object [1][2][6]: // Example using JavaScript/TypeScript const { data: { users }, error } = await supabase.auth.admin.listUsers({ page: 1, perPage: 1000 // Sets the per_page parameter to the maximum limit }); If you are calling the Auth Admin API directly via HTTP, use the per_page query parameter [7]: GET /auth/v1/admin/users?per_page=1000 Authorization: Bearer <SERVICE_ROLE_KEY> apikey: <SUPABASE_ANON_KEY>
Citations:
- 1: https://supabase.com/docs/reference/javascript/auth-admin-listusers
- 2: https://supabase.com/docs/reference/python/auth-admin-listusers
- 3: https://github.com/supabase/auth/blob/master/openapi.yaml
- 4: https://github.com/supabase/gotrue/blob/master/openapi.yaml
- 5: https://raw.githubusercontent.com/api-evangelist/supabase/refs/heads/main/openapi/supabase-admin-api-openapi.yml
- 6: https://supabase.com/docs/reference/swift/auth-admin-listusers
- 7:
auth.admin.listUsers()only lists up to 50 users supabase/auth-js#538
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the migration script and relevant symbols without executing repo code.
if [ -f scripts/migrate-supabase-to-cloudflare.mjs ]; then
echo "== file size =="
wc -l scripts/migrate-supabase-to-cloudflare.mjs
echo "== outline =="
ast-grep outline scripts/migrate-supabase-to-cloudflare.mjs || true
echo "== relevant lines 1-220 =="
sed -n '1,220p' scripts/migrate-supabase-to-cloudflare.mjs | nl -ba
else
echo "scripts/migrate-supabase-to-cloudflare.mjs not found"
fd -a 'migrate-supabase-to-cloudflare\.mjs$' .
fi
echo "== search for migration and table order logic =="
rg -n "fetchAuthUsers|fetchTable|insertSql|tableColumns|order=|per_page|range:" . --glob '!node_modules' --glob '!dist' --glob '!build' || trueRepository: mani1261790/PHOTO-TEXTE
Length of output: 1605
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant lines 1-120 =="
awk '{printf "%6d %s\n", NR, $0}' scripts/migrate-supabase-to-cloudflare.mjs | sed -n '1,120p'
echo "== relevant lines 121-260 =="
awk '{printf "%6d %s\n", NR, $0}' scripts/migrate-supabase-to-cloudflare.mjs | sed -n '121,260p'
echo "== exact matches =="
rg -n "fetchAuthUsers|fetchTable|insertSql|tableColumns|order=|per_page|range:" scripts/migrate-supabase-to-cloudflare.mjs || trueRepository: mani1261790/PHOTO-TEXTE
Length of output: 11722
🌐 Web query:
Supabase PostgREST range header order default duplicate rows documentation
💡 Result:
PostgREST does not have a default sort order when you use the Range header for pagination [1][2][3]. PostgreSQL does not guarantee any specific order for rows returned from a query unless an explicit ORDER BY clause is provided [1][3]. If you omit the order parameter while using pagination (e.g.,.range in Supabase or Range headers in PostgREST), the order of the results is effectively undefined [1][4]. This leads to two significant issues: 1. Unpredictable Results: Since the database does not guarantee an order, the result set can be different every time you execute the same request [1][4]. 2. Duplicate or Missing Rows: When you paginate through a dataset without a stable, unique ordering, rows may shift between pages. A row might appear on the first page, and then appear again on the second page (or be skipped entirely) because the database's internal ordering changed between the two requests [1][2][4]. To ensure reliable, consistent, and duplicate-free pagination, you must always include an explicit order clause on a unique column (such as a primary key or a unique timestamp) [5][4][3]. For example, in Supabase, you should chain an.order call before your.range call: // Example of stable pagination const { data, error } = await supabase.from('your_table').select('*').order('id', { ascending: true }) // Stable ordering by a unique column.range(0, 9); If you do not specify an order, the system may exhibit behavior that appears to be ordered (such as falling back to the physical disk order, often referred to as ctid), but this is not reliable and should never be relied upon for production applications [4][6].
Citations:
- 1:
range()documentation appears to be incorrect supabase/supabase#15222 - 2: On data request when using both .order and .range, no matter the offset+limit combination i get the same element as last element supabase/supabase#16409
- 3: inconsistent page ordering PostgREST/postgrest#1955
- 4: https://github.com/erp-mafia/accounted/blob/main/lib/supabase/fetch-all.ts
- 5:
range()withorder()return inconsistent results supabase/supabase-js#445 - 6: https://github.com/michelfaure/rembrandt-samples/blob/main/postgrest-row-cap/02-eslint-rule.mjs
🌐 Web query:
Supabase admin api listUsers returns truncated short batch empty source count
💡 Result:
The Supabase admin API listUsers() method returns a limited, paginated set of users by default—typically 50—rather than the entire user list [1][2][3]. If you are encountering truncated results, empty data, or incorrect pagination counts, it is likely due to the following factors: 1. Default Pagination: listUsers() defaults to returning a single page of results (50 users) [1][2][3]. To retrieve more users, you must explicitly use the page and perPage parameters [4][3]. 2. Pagination Metadata Issues: Older versions of the Supabase JavaScript library contained bugs where the nextPage and lastPage metadata were incorrectly parsed if they exceeded a single digit (e.g., page 10 was read as page 1) [5][6]. If your application relies on these values to fetch subsequent pages, ensure you are using an up-to-date version of supabase-js, as these issues were resolved in later patches [5][6]. 3. Correct Implementation: You can control the batch size and page number by passing an options object to the method [4]: const { data: { users, nextPage, total }, error } = await supabase.auth.admin.listUsers({ page: 1, perPage: 1000 // Adjust as needed, usually up to 1000 }); If you need to retrieve all users and are seeing unexpected behavior, verify that you are not relying on truncated pagination links and consider fetching in a loop using the explicit page parameter rather than relying solely on the returned nextPage helper if you are on an older version [1][5]. Always ensure this code runs in a secure, server-side environment using your service_role key [4].
Citations:
- 1:
auth.admin.listUsers()only lists up to 50 users supabase/auth-js#538 - 2: Clarify listUsers() example supabase/supabase#11463
- 3: feat: support pagination options for
listUsers()method supabase/auth-js#537 - 4: https://supabase.com/docs/reference/javascript/auth-admin-listusers
- 5: fix(auth): read full page number from listUsers Link header supabase/supabase-js#2465
- 6: feat: support more than one digit of pagination metadata in
listUsers()supabase/auth-js#793
Use stable pagination for the migration dataset.
fetchTable pages with only range, so PostgREST row order is not guaranteed between requests. That can produce duplicate or skipped rows; INSERT OR IGNORE then hides the duplicates while skipped rows are lost. Add &order=id.asc to every table query before the range and stop on an empty batch.
INSERT OR IGNORE does not replace the need to compare the migrated database/user counts against the source count before migration.
🤖 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 `@scripts/migrate-supabase-to-cloudflare.mjs` around lines 54 - 80, Update
fetchTable to request a stable ascending id order on every table query before
applying the range, and stop pagination when a batch is empty rather than only
when it is short. Also add pre-migration count comparisons between the source
Supabase users/table data and the destination database, failing or reporting a
mismatch before inserts proceed; do not rely on INSERT OR IGNORE to validate
completeness.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@wrangler.jsonc`:
- Around line 62-66: Add the production BETTER_AUTH_URL to the top-level vars in
wrangler.jsonc, using the deployed production URL and preserving the existing
env.dev value. Ensure lib/auth/better-auth.ts receives this variable unchanged
for baseURL, secure cookies, and host allowlist behavior.
🪄 Autofix
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 Plus
Run ID: ffad4fca-4c58-42e5-b164-d7614541c3ce
📒 Files selected for processing (5)
README.mdcomponents/TopNav.tsxpackage.jsonscripts/migrate-supabase-to-cloudflare.mjswrangler.jsonc
🚧 Files skipped from review as they are similar to previous changes (3)
- components/TopNav.tsx
- scripts/migrate-supabase-to-cloudflare.mjs
- README.md
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/globals.css (1)
184-184: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAlign
.brand smallwith the minimum font-size rule.The French override sets
--minimum-font-sizeto12px, while.brand smallstill uses hard-coded11pxand is not scoped inside the media-query exception. Usefont-size: max(11px, var(--minimum-font-size));or add an explicit comment that this element is intentionally exempt.🤖 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 `@app/globals.css` at line 184, Update the `.brand small` rule to use `font-size: max(11px, var(--minimum-font-size));`, ensuring it respects the French minimum font-size override while retaining the 11px floor.
🧹 Nitpick comments (1)
components/LogoMark.tsx (1)
11-11: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace deprecated
prioritywithpreload.This repository uses Next.js 16.3.0, where Image
priorityis deprecated in favor ofpreload.Proposed fix
- priority + preload🤖 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 `@components/LogoMark.tsx` at line 11, In the Image props within LogoMark, replace the deprecated priority prop with preload while preserving the existing eager-loading behavior.
🤖 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 @.github/workflows/deploy-cloudflare-dev.yml:
- Around line 48-49: Add connection and transfer timeouts to both curl health
checks assigning login_status and auth_status, using curl’s --connect-timeout
and --max-time options with bounded values so unresponsive endpoints fail
promptly while preserving the existing HTTP status capture.
- Around line 43-44: Update the “Deploy dev Worker” workflow step to run the
shared D1 migration command before npm run cf:deploy:dev, targeting the dev D1
binding named photo-texte from wrangler.jsonc. Preserve the existing deployment
command and ordering so migrations complete before the Worker is deployed.
- Around line 20-22: Remove the job-level CLOUDFLARE_API_TOKEN from the workflow
and define it only in the Deploy dev Worker step’s environment, leaving the
account ID appropriately scoped. Update actions/checkout@v4 with
persist-credentials: false so the GitHub token is not retained for later steps.
In `@app/globals.css`:
- Line 1: Update the Google Fonts `@import` declaration in the global stylesheet
to use the configured quoted import-string notation instead of url(...), while
preserving the existing font URL and parameters.
- Around line 354-357: Update the focus-visible outline in the .topnav-links
a:focus-visible and .topnav-links button:focus-visible rules to use an opaque
theme teal color instead of the current 25%-opacity color, while preserving the
existing outline width and offset.
In `@lib/ai/client.ts`:
- Around line 66-80: Add a bounded AbortController timeout around the shared
OpenAI fetch in createOpenAIResponse, passing its signal to fetch so translate,
rewrite, and generateLearningNotes cannot hang indefinitely. Clear the timeout
in a finally block after the request completes or fails.
---
Outside diff comments:
In `@app/globals.css`:
- Line 184: Update the `.brand small` rule to use `font-size: max(11px,
var(--minimum-font-size));`, ensuring it respects the French minimum font-size
override while retaining the 11px floor.
---
Nitpick comments:
In `@components/LogoMark.tsx`:
- Line 11: In the Image props within LogoMark, replace the deprecated priority
prop with preload while preserving the existing eager-loading behavior.
🪄 Autofix
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 Plus
Run ID: 6c611575-849c-41fa-afa1-74d35c8d5f2d
⛔ Files ignored due to path filters (4)
app/apple-icon.pngis excluded by!**/*.pngapp/icon.pngis excluded by!**/*.pngpackage-lock.jsonis excluded by!**/package-lock.jsonpublic/icon-192.pngis excluded by!**/*.png
📒 Files selected for processing (7)
.github/workflows/deploy-cloudflare-dev.ymlREADME.mdapp/globals.csscomponents/LogoMark.tsxlib/ai/client.tspackage.jsonwrangler.jsonc
🚧 Files skipped from review as they are similar to previous changes (2)
- wrangler.jsonc
- README.md
| - name: Deploy dev Worker | ||
| run: npm run cf:deploy:dev |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import json
scripts = json.load(open("package.json"))["scripts"]
for name in ("cf:deploy:dev", "db:migrate:dev", "db:migrate:shared"):
print(f"{name}: {scripts.get(name, '<missing>')}")
PY
rg -n -C 3 '"d1_databases"|"migrations_dir"|"dev"' wrangler.jsoncRepository: mani1261790/PHOTO-TEXTE
Length of output: 1148
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- workflow excerpt ---\n'
sed -n '1,90p' .github/workflows/deploy-cloudflare-dev.yml
printf '\n--- package scripts excerpt ---\n'
python3 - <<'PY'
import json
with open("package.json") as f:
scripts = json.load(f)["scripts"]
for name in sorted(scripts):
if "dev" in name.lower() or "cf:" in name or "db" in name.lower() or "migrate" in name.lower():
print(f"{name}: {scripts[name]}")
PY
printf '\n--- cloudflare/migrations directory ---\n'
if [ -d cloudflare/migrations ]; then
find cloudflare/migrations -maxdepth 2 -type f -print | sort
else
echo '<missing>'
fi
printf '\n--- deploy scripts references ---\n'
rg -n "cf:deploy:dev|opennextjs-cloudflare|wrangler.*d1|migrations apply|DB --remote|env.dev|env=\"\"" .Repository: mani1261790/PHOTO-TEXTE
Length of output: 252
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- workflow excerpt ---'
sed -n '1,90p' .github/workflows/deploy-cloudflare-dev.yml
printf '%s\n' ''
printf '%s\n' '--- package scripts excerpt ---'
python3 - <<'PY'
import json
with open("package.json") as f:
scripts = json.load(f)["scripts"]
for name in sorted(scripts):
if "dev" in name.lower() or "cf:" in name or "db" in name.lower() or "migrate" in name.lower():
print(f"{name}: {scripts[name]}")
PY
printf '%s\n' ''
printf '%s\n' '--- cloudflare/migrations directory ---'
if [ -d cloudflare/migrations ]; then
find cloudflare/migrations -maxdepth 2 -type f -print | sort
else
echo '<missing>'
fi
printf '%s\n' ''
printf '%s\n' '--- deploy scripts references ---'
rg -n "cf:deploy:dev|opennextjs-cloudflare|wrangler.*d1|migrations apply|DB --remote|env.dev|env=\"\"" .Repository: mani1261790/PHOTO-TEXTE
Length of output: 3352
Run D1 migrations before cf:deploy:dev.
npm run cf:deploy:dev only builds and deploys the dev Worker, and there is no separate cf:migrate:dev script. Run the shared dev D1 migration command before deployment so new Worker code does not execute against an older photo-texte schema. Use the dev D1 binding from wrangler.jsonc when applying migrations.
🤖 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 @.github/workflows/deploy-cloudflare-dev.yml around lines 43 - 44, Update the
“Deploy dev Worker” workflow step to run the shared D1 migration command before
npm run cf:deploy:dev, targeting the dev D1 binding named photo-texte from
wrangler.jsonc. Preserve the existing deployment command and ordering so
migrations complete before the Worker is deployed.
mani1261790
changed the title
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 (3)
app/entries/new/page.tsx (1)
69-69: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winPreserve compliant PNG and WebP files.
Line 69 sends every accepted image through
canvas.toBlob(..., "image/jpeg"). This converts compliant PNG and WebP files to JPEG. It removes PNG transparency and drops animated WebP frames before upload.Return the original file when it already meets the dimension and size limits.
Proposed fix
const scale = Math.min( 1, MAX_IMAGE_DIMENSION / Math.max(image.naturalWidth, image.naturalHeight), ); + if (scale === 1 && file.size <= MAX_UPLOAD_BYTES) { + return file; + } const width = Math.max(1, Math.round(image.naturalWidth * scale));🤖 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 `@app/entries/new/page.tsx` at line 69, Update the image-processing flow around the file type check and canvas conversion so files that already satisfy the dimension and size limits are returned unchanged, preserving compliant PNG and WebP formats. Only invoke the existing JPEG conversion for images that require resizing or size reduction.components/EntryWizard.tsx (2)
533-539: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winKeep annotation state stable during export.
exportFilesaves dirty annotations withsilent=true. This path does not setannotationSavingId, soCorrectionAnnotationEditorcan remain editable while the export save is in flight. If the user changes an annotation during that request, Line 463 clears its dirty state from the stale save. The export can then use older annotations and the new edit can appear saved without persistence.Disable annotation editing during export, or use a revision check that clears dirty state only when the saved snapshot still matches the current annotations.
🤖 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 `@components/EntryWizard.tsx` around lines 533 - 539, Update exportFile’s dirty-annotation save flow around saveAnnotationsForPhoto so annotation editing is blocked for the entire in-flight export save, or add a revision/snapshot guard that preserves dirty state when annotations change during the request. Ensure stale saves cannot clear newer edits, and the export uses the latest stable annotation values.
487-510: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winSerialize self-note saves.
saveSelfNotecan run again before an earlier request resolves. Each call uses its capturedmemosvalue. If no self-note exists, overlapping calls can both issuePOSTrequests. If an earlier request completes after a newer edit, it clearsmemoPendingSavefor the newer draft. The debounce then cancels the newer save, and the UI can show an unsaved note as saved.Queue note mutations, or track a revision per draft. Clear
memoPendingSaveonly when the completed request matches the latest revision.🤖 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 `@components/EntryWizard.tsx` around lines 487 - 510, Serialize mutations in saveSelfNote so overlapping saves cannot use stale memos or issue duplicate POST requests; chain each save behind the previous request or otherwise track the latest draft revision. Ensure memoPendingSave is cleared and memoSavedAt is updated only when the completed save corresponds to the latest revision, leaving newer edits pending.
🤖 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 `@components/EntryWizard.tsx`:
- Around line 225-231: Update the initialization around
setGeneratedFinalByPhotoId in EntryWizard so it uses the API’s separate
generated-final-text field instead of photo.final_fr. Ensure the corresponding
API persistence returns and exposes that generated field, allowing
activeGeneratedFinal and the restore action to retain the original generated
text after manual edits and reloads.
---
Outside diff comments:
In `@app/entries/new/page.tsx`:
- Line 69: Update the image-processing flow around the file type check and
canvas conversion so files that already satisfy the dimension and size limits
are returned unchanged, preserving compliant PNG and WebP formats. Only invoke
the existing JPEG conversion for images that require resizing or size reduction.
In `@components/EntryWizard.tsx`:
- Around line 533-539: Update exportFile’s dirty-annotation save flow around
saveAnnotationsForPhoto so annotation editing is blocked for the entire
in-flight export save, or add a revision/snapshot guard that preserves dirty
state when annotations change during the request. Ensure stale saves cannot
clear newer edits, and the export uses the latest stable annotation values.
- Around line 487-510: Serialize mutations in saveSelfNote so overlapping saves
cannot use stale memos or issue duplicate POST requests; chain each save behind
the previous request or otherwise track the latest draft revision. Ensure
memoPendingSave is cleared and memoSavedAt is updated only when the completed
save corresponds to the latest revision, leaving newer edits pending.
🪄 Autofix
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 Plus
Run ID: bdda9146-7c55-47ec-80b8-9c3f4b1234c2
📒 Files selected for processing (6)
.github/workflows/deploy-cloudflare-dev.ymlapp/entries/new/page.tsxapp/globals.csscomponents/EntryWizard.tsxlib/ai/client.tswrangler.jsonc
🚧 Files skipped from review as they are similar to previous changes (3)
- wrangler.jsonc
- lib/ai/client.ts
- .github/workflows/deploy-cloudflare-dev.yml
| setGeneratedFinalByPhotoId((current) => { | ||
| const next = { ...current }; | ||
| orderedPhotos.forEach((photo) => { | ||
| if (next[photo.id] === undefined) next[photo.id] = photo.final_fr ?? ""; | ||
| }); | ||
| return next; | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Persist a separate generated-text baseline.
Lines 225-231 initialize generatedFinalByPhotoId from photo.final_fr. saveFinalText persists manual edits in final_fr. After a reload, activeGeneratedFinal contains the saved manual text, not the generated text. The restore action then cannot restore the generated version.
Store the generated final text separately in the API and initialize this state from that field.
🤖 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 `@components/EntryWizard.tsx` around lines 225 - 231, Update the initialization
around setGeneratedFinalByPhotoId in EntryWizard so it uses the API’s separate
generated-final-text field instead of photo.final_fr. Ensure the corresponding
API persistence returns and exposes that generated field, allowing
activeGeneratedFinal and the restore action to retain the original generated
text after manual edits and reloads.
1 participant
Summary
Deployment model
photo-textephoto-texte-devphoto-texteand R2photo-texte-contentValidation
npm test -- --run(19 tests)npx tsc --noEmitnpm run buildnpm run cf:build:devSummary by CodeRabbit
New Features
Improvements
Authentication