docs: add frontend refine workflow#18
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughAdds Task 17 front-end refine docs and implements matching UI/styling/components/tests: design-system references, Tailwind token/themes, brand/icon components, AppShell and route refactors (browse, reader), responsive-image importer/sanitizer, numerous Vitest contract tests, and expanded Playwright E2E suites. ChangesTask 17 Front-End Refine
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
docs/workflows/task-17-front-end-refine.md (1)
45-49: ⚡ Quick winAdd
docs/INDEX.mdto required context for workflow orientation.Please include
docs/INDEX.mdin this section so implementers start from the canonical docs entrypoint before diving into task-specific references.Proposed doc patch
## Required Context +- [Docs index](../INDEX.md) - [Technology stack](../references/technology-stack.md) - [UI and routing architecture](../architecture/ui-and-routing.md) - [Coding standards](../references/coding-standards/INDEX.md) - [Tailwind migration archive](archive/task-16a-tailwind-migration.md)As per coding guidelines, "All project detail lives under
docs/directory. Start at docs/INDEX.md for orientation."🤖 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 `@docs/workflows/task-17-front-end-refine.md` around lines 45 - 49, Add the canonical docs entry point to the top of the reference list in the workflow doc by inserting a link to docs/INDEX.md as the first item in the existing list of references (above "Technology stack", "UI and routing architecture", etc.) so implementers are directed to the project-level orientation before task-specific materials.docs/workflows/task-17-front-end-refine/05-reader-surface.md (1)
28-32: ⚡ Quick winConsider varying sentence structure for better readability.
Three consecutive sentences begin with "Sample". While the content is clear, varying the opening words improves readability.
✍️ Proposed refactor for sentence variety
-- Sample `PaneHeader` maps to the reader title/header. -- Sample `MemoryDetail` visual details map to reader chrome only. -- Sample `detail__prose` maps to Tailwind typography classes and +- The sample `PaneHeader` maps to the reader title/header. +- Visual details from `MemoryDetail` map to reader chrome only. +- The sample `detail__prose` maps to Tailwind typography classes and `reader-styles.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 `@docs/workflows/task-17-front-end-refine/05-reader-surface.md` around lines 28 - 32, The three consecutive lines all start with "Sample", which reduces readability; rephrase them to vary sentence openings while preserving meaning—e.g., change the first to "The `PaneHeader` maps to the reader title/header", the second to "Visual details in `MemoryDetail` correspond only to reader chrome", the third to "The `detail__prose` maps to Tailwind typography classes and `reader-styles.ts`", and finish with a reworded fourth line clarifying that highlight quote styling is shared with browse/highlights where applicable; update mentions of `PaneHeader`, `MemoryDetail`, `detail__prose`, and `reader-styles.ts` to ensure clarity and parallel structure.
🤖 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
`@docs/workflows/task-17-front-end-refine/04-memory-browse-and-highlight-surfaces.md`:
- Around line 8-24: The Ownership section is missing the test referenced later;
update the primary files list in
docs/workflows/task-17-front-end-refine/04-memory-browse-and-highlight-surfaces.md
to include tests/components/highlights-route-state.test.ts alongside the
existing entries (e.g., src/components/memories/MemoryBrowse.tsx,
src/routes/highlights/index.tsx, src/components/memories/browse-data.ts,
src/components/memories/browse-fixtures.ts, tests/memories/browse-data.test.ts,
e2e/browse-shell.spec.ts) so the verification command’s referenced test is
listed as owned.
In
`@docs/workflows/task-17-front-end-refine/06-visual-verification-and-handoff.md`:
- Around line 44-55: The docs list three non-existent test files
(tests/components/brand-assets.test.ts, tests/components/trauma-icons.test.ts,
tests/components/app-shell.test.ts) in the verification command; update the
workflow docs by removing these filenames from the mise exec -- bun run test
command (or alternatively add those test files if intended) so the command only
references existing tests (e.g., keep existing files like
tests/components/highlights-route-state.test.ts) and ensure the E2E and unit
command blocks reflect actual repository filenames.
- Around line 78-83: The workflow step runs a find command that references an
uncommitted directory ("refined_sample/screenshots"), which will break
execution; either commit that directory (git add refined_sample/screenshots/) so
the find command can run as-is, or change the documentation around the find
invocation to mark it optional and conditional (update the find line and
surrounding text to note "if available" and fallback to step 4/acceptance
criteria), or remove the step entirely if screenshots aren't needed—locate the
find command referencing refined_sample/screenshots and apply one of these fixes
to keep the workflow runnable.
---
Nitpick comments:
In `@docs/workflows/task-17-front-end-refine.md`:
- Around line 45-49: Add the canonical docs entry point to the top of the
reference list in the workflow doc by inserting a link to docs/INDEX.md as the
first item in the existing list of references (above "Technology stack", "UI and
routing architecture", etc.) so implementers are directed to the project-level
orientation before task-specific materials.
In `@docs/workflows/task-17-front-end-refine/05-reader-surface.md`:
- Around line 28-32: The three consecutive lines all start with "Sample", which
reduces readability; rephrase them to vary sentence openings while preserving
meaning—e.g., change the first to "The `PaneHeader` maps to the reader
title/header", the second to "Visual details in `MemoryDetail` correspond only
to reader chrome", the third to "The `detail__prose` maps to Tailwind typography
classes and `reader-styles.ts`", and finish with a reworded fourth line
clarifying that highlight quote styling is shared with browse/highlights where
applicable; update mentions of `PaneHeader`, `MemoryDetail`, `detail__prose`,
and `reader-styles.ts` to ensure clarity and parallel structure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 500fdc3c-e530-424d-a360-42c6cc1cd811
📒 Files selected for processing (8)
docs/workflows/README.mddocs/workflows/task-17-front-end-refine.mddocs/workflows/task-17-front-end-refine/01-design-system-tokens.mddocs/workflows/task-17-front-end-refine/02-brand-assets-and-icons.mddocs/workflows/task-17-front-end-refine/03-shell-navigation-and-theme.mddocs/workflows/task-17-front-end-refine/04-memory-browse-and-highlight-surfaces.mddocs/workflows/task-17-front-end-refine/05-reader-surface.mddocs/workflows/task-17-front-end-refine/06-visual-verification-and-handoff.md
![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: 94d82e9862
ℹ️ 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".
|
|
||
| ## Current Status | ||
|
|
||
| Ready for implementation. This workflow should run on a `refine/*` branch. |
There was a problem hiding this comment.
This marks Task 17 as ready even though the required refined_sample/ source is not present in the repository; I checked the clean tree with find . -maxdepth 2 -type d -name refined_sample and found no copy. A worker starting from this commit will immediately hit the next section's instruction to stop and request the sample, so the workflow is not actually implementable from main unless the sample assets are committed or the status is changed to blocked/external-input-required.
Useful? React with 👍 / 👎.
| - `/memories?view=grid` | ||
| - `/memories?q=reader` | ||
| - `/highlights` | ||
| - `/memories/memory-foundation` in fixture mode |
There was a problem hiding this comment.
This required visual route does not actually exercise the reader surface under the documented fixture mode: Playwright only sets TRAUMA_BROWSE_FIXTURES=1, which loadBrowseMemories uses for the browse list, while loadReaderMemory still opens SQLite and returns not_found when memory-foundation is absent. As a result the final visual pass can inspect only the reader error state instead of the /memories/:id article layout that Task 17 is meant to refine; use the reader E2E fixture ID/setup or document the fixture creation step here.
Useful? React with 👍 / 👎.
Agent-Logs-Url: https://github.com/hauntedfail/Trauma/sessions/3fb14ec9-ce7b-4b6f-b7fa-ce319a90e288 Co-authored-by: nonnil <85566220+nonnil@users.noreply.github.com>
Agent-Logs-Url: https://github.com/hauntedfail/Trauma/sessions/3fb14ec9-ce7b-4b6f-b7fa-ce319a90e288 Co-authored-by: nonnil <85566220+nonnil@users.noreply.github.com>
feat: refine frontend from sample
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0855136f61
ℹ️ 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".
| if (sourceResponsiveMarkup !== null) { | ||
| return `\n${protectGeneratedMarkdown(sourceResponsiveMarkup)}\n`; |
There was a problem hiding this comment.
When a page has a responsive <picture> but fewer than 80 readable characters, this generated HTML is now included in extracted.markdown; readableMarkdownLength() only strips Markdown links/images and punctuation, so tag names, attributes, and URLs count toward the importer’s minimum body check in src/server/importer/index.ts. That lets image-only or nearly empty pages be saved as successfully extracted articles instead of falling back to link-only/too-short handling.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7bfb9ff. readableMarkdownLength() now strips preserved <picture>, <source>, <img>, and remaining HTML tags before applying the minimum readable body check. Added regression coverage so responsive image markup alone counts as 0 readable characters.
| { kind: "disabled", icon: "categories", label: "Categories" }, | ||
| { kind: "disabled", icon: "tags", label: "Tags" }, |
There was a problem hiding this comment.
With the right rail hidden at max-[1040px] and the filter shortcuts also hidden there, making the phone Categories and Tags entries disabled removes the only category/tag filter UI for tablet and phone users. This regresses the previously working mobile filter drawer: users on these widths can no longer apply or clear category/tag filters except by manually editing the URL.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
e2e/cross-device-responsive.spec.ts (1)
7-7: ⚡ Quick winClarify viewport naming or kind classification.
The viewport named "tablet split" is classified as
kind: "phone", which is inconsistent and may confuse future maintainers. Consider either renaming to "phone landscape" or reclassifying the kind to "tablet" if it's meant to represent a split-screen tablet scenario.🤖 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 `@e2e/cross-device-responsive.spec.ts` at line 7, The viewport entry with name "tablet split" currently has kind: "phone", which is inconsistent; update the test fixture by either renaming the entry name from "tablet split" to "phone landscape" to match kind: "phone", or change kind: "phone" to kind: "tablet" if it should represent a tablet split-screen; locate the object literal with name "tablet split" and modify the name or kind accordingly so naming and classification are consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/memories/captured-at.ts`:
- Around line 23-31: Currently the code only checks day is 1..31 and monthIndex
bounds (using day, monthIndex, shortMonthNames) which allows impossible dates
like Feb 31; update the validation to be month/year-aware before returning the
`${day} ${shortMonthNames[monthIndex]}` string—either compute the month's max
days (e.g., use a small helper daysInMonth(year, monthIndex) or create a
Date(year, monthIndex, day) and verify it round-trips) and only return the
formatted string if day <= maxDay (and year is present/validated); otherwise
fall through to the existing fallback path.
---
Nitpick comments:
In `@e2e/cross-device-responsive.spec.ts`:
- Line 7: The viewport entry with name "tablet split" currently has kind:
"phone", which is inconsistent; update the test fixture by either renaming the
entry name from "tablet split" to "phone landscape" to match kind: "phone", or
change kind: "phone" to kind: "tablet" if it should represent a tablet
split-screen; locate the object literal with name "tablet split" and modify the
name or kind accordingly so naming and classification are consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 697eb457-1073-46ac-84ad-e1939252e78a
⛔ Files ignored due to path filters (3)
public/assets/trauma-mark.pngis excluded by!**/*.pngpublic/assets/trauma-mark.svgis excluded by!**/*.svgpublic/favicon.icois excluded by!**/*.ico
📒 Files selected for processing (68)
docs/INDEX.mddocs/references/coding-standards/solidstart-ui.mddocs/references/design-system/DESIGN.mddocs/references/design-system/INDEX.mddocs/references/design-system/assets-and-icons.mddocs/references/design-system/components-and-surfaces.mddocs/references/design-system/interaction-and-accessibility.mddocs/references/design-system/layout-and-shell.mddocs/references/design-system/reader-and-content.mddocs/references/design-system/tokens-and-themes.mddocs/references/design-system/verification.mddocs/references/technology-stack.mddocs/workflows/README.mddocs/workflows/task-17-front-end-refine.mddocs/workflows/task-17-front-end-refine/01-design-system-tokens.mddocs/workflows/task-17-front-end-refine/03-shell-navigation-and-theme.mddocs/workflows/task-17-front-end-refine/04-memory-browse-and-highlight-surfaces.mddocs/workflows/task-17-front-end-refine/05-reader-surface.mddocs/workflows/task-17-front-end-refine/06-visual-verification-and-handoff.mddocs/workflows/task-17-front-end-refine/07-shell-spacing-right-rail-row-interaction.mddocs/workflows/task-17-front-end-refine/08-mobile-cross-device-responsive.mddocs/workflows/task-17-front-end-refine/08a-responsive-strategy-and-rules.mddocs/workflows/task-17-front-end-refine/08b-responsive-contract-tests.mddocs/workflows/task-17-front-end-refine/08c-container-ownership.mddocs/workflows/task-17-front-end-refine/08d-component-responsive-implementation.mddocs/workflows/task-17-front-end-refine/08e-cross-device-e2e.mddocs/workflows/task-17-front-end-refine/08f-design-system-responsive-docs.mddocs/workflows/task-17-front-end-refine/08g-safe-area-layout-tokens.mddocs/workflows/task-17-front-end-refine/08h-responsive-image-markup.mddocs/workflows/task-17-front-end-refine/08i-capability-preference-media-queries.mddocs/workflows/task-17-front-end-refine/08j-cross-device-shell-chrome-cleanup.mde2e/bootstrap.spec.tse2e/browse-shell.spec.tse2e/cross-device-responsive.spec.tse2e/reader.spec.tsplaywright.config.tssrc/app.tsxsrc/components/brand/TraumaMark.tsxsrc/components/highlights/HighlightExcerpt.tsxsrc/components/icons/TraumaIcons.tsxsrc/components/icons/index.tssrc/components/memories/AddMemoryForm.tsxsrc/components/memories/MemoryBrowse.tsxsrc/components/memories/browse-fixtures.tssrc/components/memories/captured-at.tssrc/components/reader/MemoryReader.tsxsrc/components/reader/reader-styles.tssrc/components/shell/AppShell.tsxsrc/components/shell/right-rail-context.tsxsrc/components/shell/theme.tssrc/components/ui/WaxSealButton.tsxsrc/routes/[...404].tsxsrc/routes/highlights/index.tsxsrc/routes/memories/[id].tsxsrc/server/importer/extractor.tssrc/server/reader/markdown-renderer.tssrc/styles/tailwind.csstests/components/app-shell.test.tstests/components/brand-assets.test.tstests/components/captured-at.test.tstests/components/mobile-responsive-contract.test.tstests/components/reader-style-contract.test.tstests/components/shell-theme.test.tstests/components/trauma-icons.test.tstests/memories/browse-data.test.tstests/scripts/frontend-refine-tokens.test.tstests/server/importer/extractor.test.tstests/server/reader/markdown-renderer.test.ts
✅ Files skipped from review due to trivial changes (22)
- docs/INDEX.md
- docs/references/coding-standards/solidstart-ui.md
- tests/components/captured-at.test.ts
- src/routes/memories/[id].tsx
- src/app.tsx
- tests/components/shell-theme.test.ts
- e2e/bootstrap.spec.ts
- tests/components/trauma-icons.test.ts
- docs/references/design-system/verification.md
- docs/workflows/task-17-front-end-refine/08c-container-ownership.md
- docs/workflows/task-17-front-end-refine/08-mobile-cross-device-responsive.md
- docs/workflows/task-17-front-end-refine/08g-safe-area-layout-tokens.md
- docs/references/design-system/INDEX.md
- docs/references/technology-stack.md
- src/components/ui/WaxSealButton.tsx
- docs/workflows/README.md
- src/routes/[...404].tsx
- docs/workflows/task-17-front-end-refine/03-shell-navigation-and-theme.md
- docs/workflows/task-17-front-end-refine/08d-component-responsive-implementation.md
- docs/workflows/task-17-front-end-refine/08j-cross-device-shell-chrome-cleanup.md
- docs/workflows/task-17-front-end-refine/01-design-system-tokens.md
- docs/workflows/task-17-front-end-refine/06-visual-verification-and-handoff.md
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/workflows/task-17-front-end-refine.md
Summary
Verification
Notes
Summary by CodeRabbit
Documentation
New Features
Tests