Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @leynos, you have reached your 24-hour rate limit for Sourcery. Please try again later
|
Warning Rate limit exceeded@leynos has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 50 seconds before requesting another review. ⌛ 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. 📒 Files selected for processing (6)
Summary by CodeRabbit
WalkthroughUpdate CI workflow permissions, refine .gitignore patterns, revise AGENTS.md wording/tooling guidance, replace runtime validation with TypeScript interfaces in the PWA API client, and adjust App rendering for loading/error/list states without altering overall query configuration. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as App.tsx
participant API as listUsers
participant Net as customFetch
participant Server as /api/users
UI->>API: useQuery(queryFn: listUsers)
API->>Net: customFetch<User[]>("/api/users")
Net->>Server: GET /api/users
Server-->>Net: 200 User[]
Net-->>API: Promise<User[]>
API-->>UI: User[]
UI->>UI: Render list / show error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~18 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
leynos
force-pushed
the
codex/set-up-repository-structure-pmtg14
branch
from
28a20c3 to
9b6396b
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (5)
.github/workflows/ci.yml(0 hunks).gitignore(1 hunks)AGENTS.md(5 hunks)frontend-pwa/src/api/client.ts(1 hunks)frontend-pwa/src/app/App.tsx(2 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/ci.yml
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (AGENTS.md)
**/*.{ts,tsx}: ESM-only: source/build outputs are ES Modules; configure Vite accordingly; no CommonJS
Place JSDoc comments above declarations/decorators; keep docs close to code
Prefer immutability: const, readonly, Readonly; avoid mutating inputs
Extract helpers for long functions; keep trivial functions on one line when readable
Group related parameters into typed objects/builders; avoid long positional lists
Extract predicate functions or use lookup tables for multi-branch conditionals; enforce exhaustive switch with never guard
Every module begins with /** @file … */ describing purpose and usage
Validate I/O with runtime schemas (zod/valibot) at boundaries; keep types and schemas in sync
Use branded types for identifiers/tokens
Accept AbortSignal for cancellable async operations; wire through TanStack Query fetchers
Centralise time and RNG adapters; avoid direct Date.now()/Math.random() in business logic
Use discriminated unions for recoverable errors; reserve Error subclasses for exceptional paths with cause
Use Vitest with jsdom/happy-dom; keep tests deterministic and parallel-safe
Use factories/builders for fixtures; prefer parameterised tests with test.each/it.each
Use vi.mock for module boundaries; inject adapters for env/time/storage/fetch; avoid monkey-patching globals
Use vi.useFakeTimers() for time-based logic; restore after each test
Keep snapshots deterministic (sorted keys, fixed seeds) and scoped to stable UI fragments
Use Biome for format+lint; disallow any, non-null !; forbid @ts-ignore in favour of @ts-expect-error with reason
Enforce import hygiene: sorted/grouped imports; no unused or extraneous deps
Use dynamic import() for code-splitting; enable Vite prefetch and asset hashing
Configure TanStack Query with appropriate stale-time, cache-time; avoid refetchOnWindowFocus unless necessary
Avoid await in loops; batch with Promise.allSettled; use async iterables/streams for large data
Ship CSP, avoid eval/new Function; use Trusted Types and sa...
Files:
frontend-pwa/src/app/App.tsxfrontend-pwa/src/api/client.ts
**/*.{tsx,ts}
📄 CodeRabbit Inference Engine (AGENTS.md)
**/*.{tsx,ts}: Map domain errors to user messages at UI boundary; do not leak raw stacks to DOM/analytics
Enable React StrictMode in dev; memoise expensive components; derive data via selectors
Prefer semantic HTML; use daisyUI components only when they don’t harm semantics; audit focus and contrast
Files:
frontend-pwa/src/app/App.tsxfrontend-pwa/src/api/client.ts
**/*.md
📄 CodeRabbit Inference Engine (AGENTS.md)
**/*.md: Documentation must use en-GB-oxendict spelling and grammar (except LICENSE name)
Validate Markdown with make markdownlint
Run make fmt after doc changes to format Markdown and fix tables
Validate Mermaid diagrams by running make nixie
Wrap Markdown paragraphs and bullets at 80 columns; do not wrap tables/headings; code blocks at 120 columns; use dashes for bullets; use GFM footnotes
Files:
AGENTS.md
⚙️ CodeRabbit Configuration File
**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")
- Use en-GB-oxendict (-ize / -our) spelling and grammar
- Headings must not be wrapped.
- Documents must start with a level 1 heading
- Headings must correctly increase or decrease by no more than one level at a time
- Use GitHub-flavoured Markdown style for footnotes and endnotes.
- Numbered footnotes must be numbered by order of appearance in the document.
Files:
AGENTS.md
🧠 Learnings (1)
📚 Learning: 2025-07-18T18:35:14.639Z
Learnt from: CR
PR: leynos/ddlint#0
File: AGENTS.md:0-0
Timestamp: 2025-07-18T18:35:14.639Z
Learning: Recognizing Refactoring Needs: Regularly assess the codebase for potential refactoring opportunities. Consider refactoring when you observe: Long Methods/Functions, Duplicated Code, Complex Conditionals, Large Code Blocks for Single Values, Primitive Obsession / Data Clumps, Excessive Parameters, Feature Envy, Shotgun Surgery.
Applied to files:
AGENTS.md
🧬 Code Graph Analysis (1)
frontend-pwa/src/api/client.ts (1)
frontend-pwa/src/api/fetcher.ts (1)
customFetch(7-40)
🪛 Biome (2.1.2)
frontend-pwa/src/app/App.tsx
[error] 16-16: The elements with this role can be changed to the following elements:
For examples and more information, see WAI-ARIA Roles
(lint/a11y/useSemanticElements)
🪛 LanguageTool
AGENTS.md
[style] ~82-~82: Would you like to use the Oxford spelling “Recognizing”? The spelling ‘Recognising’ is also correct.
Context: ... Refactoring Heuristics & Workflow - Recognising Refactoring Needs: Regularly assess t...
(OXFORD_SPELLING_Z_NOT_S)
[uncategorized] ~436-~436: Loose punctuation mark.
Context: ...ecklist (Before Commit) - bun run fmt, bun run lint, bun test all clean; n...
(UNLIKELY_OPENING_PUNCTUATION)
🔍 Remote MCP (Context7)
Relevant facts for reviewing the PR (changes pinning oven-sh/setup-bun to v2 and bun-version: 1.1.32):
- The setup-bun action supports a bun-version input (default: latest) — you can pin a specific version (e.g., "1.1.32") to get deterministic CI installs.,
- The action also supports bun-version-file to read the version from a project file (e.g., .bun-version or package.json) — useful if you prefer a declared version instead of hard-coding it in the workflow.
- Outputs provided by the action (bun-version, bun-revision, bun-path, cache-hit, bun-download-url) can be used for downstream diagnostics or caching logic in the workflow.
- Additional inputs include bun-download-url and registries (for custom download sources or private registries) which may be relevant if CI needs custom registries or alternate Bun builds.
Citations:
- oven-sh/setup-bun README (inputs, usage, outputs, version-file usage) — retrieved via Context7: resolve-library-id and get-library-docs.,
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (3)
AGENTS.md (3)
61-61: Affirm the quality gate wordingThe added reference to
make testaligns with the documented workflow.
82-101: Affirm refactoring heuristics (en‑GB spelling preserved)The “Recognising Refactoring Needs” section matches the house style and guidance.
332-334: Affirm Bun lockfile policyCommitting
bun.lockand ignoringbun.lockbmatches the reproducibility objective in this PR.
frontend-pwa/src/api/client.ts
Outdated
| export interface User { | ||
| id: string; | ||
| display_name: string; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Brand identifiers to prevent accidental mixing
Introduce a branded type for User.id to avoid mixing with arbitrary strings in the UI and elsewhere.
-export interface User {
- id: string;
- display_name: string;
-}
+export type UserId = string & { readonly brand: 'UserId' };
+export interface User {
+ id: UserId;
+ display_name: string;
+}Note: When parsing at runtime (see next comment), cast id to UserId post‑validation.
📝 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.
| export interface User { | |
| id: string; | |
| display_name: string; | |
| } | |
| -export interface User { | |
| - id: string; | |
| - display_name: string; | |
| -} | |
| +export type UserId = string & { readonly brand: 'UserId' }; | |
| + | |
| +export interface User { | |
| + id: UserId; | |
| + display_name: string; | |
| +} |
🤖 Prompt for AI Agents
In frontend-pwa/src/api/client.ts around lines 6 to 9, the User.id is currently
a plain string which can be accidentally mixed with other string types;
introduce a branded type (e.g. UserId = string & { __brand: 'UserId' }) and
change the User.id type to that branded UserId to provide type safety across the
UI and prevent accidental mixing; when parsing/validating runtime data, cast the
validated id string to UserId (as noted) rather than using raw strings.
frontend-pwa/src/app/App.tsx
Outdated
| {(data ?? []).map(u => ( | ||
| <li key={u.id}> | ||
| <span>{u.display_name}</span> | ||
| </li> | ||
| )} | ||
| ))} | ||
| </ul> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Restore an accessible empty state
Render an explicit empty state to aid users and tests when the list is empty.
- {(data ?? []).map(u => (
- <li key={u.id}>
- <span>{u.display_name}</span>
- </li>
- ))}
+ {data && data.length > 0 ? (
+ data.map(u => (
+ <li key={u.id}>
+ <span>{u.display_name}</span>
+ </li>
+ ))
+ ) : (
+ <li className="disabled">
+ <span>No users found.</span>
+ </li>
+ )}📝 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.
| {(data ?? []).map(u => ( | |
| <li key={u.id}> | |
| <span>{u.display_name}</span> | |
| </li> | |
| )} | |
| ))} | |
| </ul> | |
| {data && data.length > 0 ? ( | |
| data.map(u => ( | |
| <li key={u.id}> | |
| <span>{u.display_name}</span> | |
| </li> | |
| )) | |
| ) : ( | |
| <li className="disabled"> | |
| <span>No users found.</span> | |
| </li> | |
| )} | |
| </ul> |
🤖 Prompt for AI Agents
In frontend-pwa/src/app/App.tsx around lines 38 to 43, the list rendering only
maps items and omits an explicit accessible empty state; modify the JSX so that
when (data ?? []).length === 0 you render a clear empty-state element (e.g., a
list item or div inside the ul) with a readable message like "No users found"
and accessibility attributes (role="status" and aria-live="polite" or an
aria-label) so keyboard/screen-reader users and tests can detect the empty
state.
* Tidy config and improve user list handling * Fix tokens build script for style-dictionary v4 * Add index.html and fix token import
1 participant
Summary
Testing
make fmtmake lintmake testcd frontend-pwa && bun run audit(fails: requires an existing lockfile)cd frontend-pwa && bun run audit:snyk(fails: missingnode_modulesfolder)https://chatgpt.com/codex/tasks/task_e_689c4491c2248322afe54448c92187a7