Repository navigation
Conversation
|
Caution Review failedAn error occurred during the review process. Please try again later. WalkthroughAdded deterministic frontend mock data, Playwright visual tests for ChangesFrontend visual regression
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 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 |
6d02657 to
e16d258
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: 4
🤖 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/visual-regression.yaml:
- Around line 74-76: Update the comparison workflow step around “Compare
prototype screenshots against committed baselines” to build the exported
frontend-visual-test package with nix build .#frontend-visual-test before
running the Bun visual regression command, ensuring this occurs for
non-workflow_dispatch comparison runs.
- Around line 14-28: Add concurrency control to the workflow configuration
alongside the existing on triggers, using a group keyed by the pull request
number or ref and canceling in-progress runs. Ensure newer pushes supersede
older browser-test jobs while preserving manual, pull-request, and master push
triggers.
- Line 37: Update the actions/checkout@v4 step in the visual regression workflow
to set persist-credentials to false, ensuring subsequent bun install and
Playwright commands cannot access GITHUB_TOKEN through Git configuration.
- Line 37: Pin both GitHub Actions in .github/workflows/visual-regression.yaml
at lines 37 and 84 to immutable 40-character commit SHAs for their intended
releases, replacing the current v4 references while preserving the existing
action usage.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 64831114-fa44-4c96-bc69-d5b23e3a84a2
⛔ Files ignored due to path filters (2)
frontend/bun.lockis excluded by!**/*.lockfrontend/src/visual/prototype.spec.ts-snapshots/prototype-default-linux.pngis excluded by!**/*.png
📒 Files selected for processing (12)
.github/workflows/visual-regression.yamlROADMAP.mdflake.nixfrontend/.gitignorefrontend/bun.nixfrontend/default.nixfrontend/package.jsonfrontend/playwright.visual.config.tsfrontend/src/pages/Prototype/mockData.tsfrontend/src/visual/prototype.spec.tsfrontend/tsconfig.node.jsonfrontend/vite.config.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (5)
- GitHub Check: prototype-visual
- GitHub Check: hooks
- GitHub Check: prototype-visual
- GitHub Check: frontend
- GitHub Check: backend
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{rs,ts,tsx,md,json,yml,yaml,toml,nu}
📄 CodeRabbit inference engine (AGENTS.md)
Use ASCII for code, comments, identifiers, logs, commit subjects, PR titles, and documentation prose; reserve Unicode for user-facing text and quoted UI strings in documentation.
Files:
frontend/tsconfig.node.jsonROADMAP.mdfrontend/playwright.visual.config.tsfrontend/src/pages/Prototype/mockData.tsfrontend/src/visual/prototype.spec.tsfrontend/vite.config.tsfrontend/package.json
*
⚙️ CodeRabbit configuration file
Focus on providing constructive criticism. Whenever you see a suboptimal approach, suggest more idiomatic or robust alternative(s). Flag potential footguns. Suggest FP alternatives to mutable/imperative code. Point out architectural flaws like leaky abstractions, tight coupling, wrong level of abstraction, poor type modeling, over-abstraction, unclear domain boundaries. Code should generally be organized based on business concerns rather than technical aspects - suggest improvements if you find violations. Point out gaps in test coverage but suggest tests that are not too coupled to the implementation and actually test domain invariants and business logic
Files:
ROADMAP.mdflake.nix
**/*.md
⚙️ CodeRabbit configuration file
Focus on the contents of the docs and not on cosmetic things like markdown formatting. We use markdown files for various docs including but not limited to the north star system specification, SPEC.md, the plan for how to get there, ROADMAP.md, guidelines for AI contributors, AGENTS.md, project overview and instructions for human contributors, README.md. Think about the target audience of a document when deciding what comment to leave. For specifications and designs, suggest potential product, architectural, and UI/UX improvements. For plans, suggest changes that would make things more parallelizable and deliverable-focused. For instructions, suggest better rules and guidelines and point out missing instructions. In all cases, flag needless bloat, prefer clear concise writing, and consider the structure of the document and order of the sections
Files:
ROADMAP.md
**/*.{rs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,ts,tsx}: Prefer declarative, expression-oriented code: usemap/filter/fold/collect, pure functions, immutability by default, and method chaining over imperative loops.
Use descriptive names: avoid generic identifiers, avoid single-letter variables, and avoid abbreviations unless universally understood.
Files:
frontend/playwright.visual.config.tsfrontend/src/pages/Prototype/mockData.tsfrontend/src/visual/prototype.spec.tsfrontend/vite.config.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Useconstby default in TypeScript and avoid mutable bindings unless mutation is necessary.
Avoid boolean blindness in TypeScript; prefer discriminated unions or named functions over raw booleans.
Files:
frontend/playwright.visual.config.tsfrontend/src/pages/Prototype/mockData.tsfrontend/src/visual/prototype.spec.tsfrontend/vite.config.ts
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
In TypeScript, avoid generic utility file names such as
types.ts,interfaces.ts,utils.ts,helpers.ts,constants.ts,common.ts, andshared.ts; organize by business domain instead.
Files:
frontend/playwright.visual.config.tsfrontend/src/pages/Prototype/mockData.tsfrontend/src/visual/prototype.spec.tsfrontend/vite.config.ts
frontend/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
frontend/**/*.{ts,tsx}: In the SolidJS frontend, avoidcreateEffectunless imperative DOM work, signal side effects, or global listeners require it; prefercreateMemoor@tanstack/solid-queryfor derived/fetched state, and add an inline comment whencreateEffectis necessary.
Do not manually create shadcn-solid component files; usebunx shadcn-solid@latest add <component-name>instead.
Files:
frontend/playwright.visual.config.tsfrontend/src/pages/Prototype/mockData.tsfrontend/src/visual/prototype.spec.tsfrontend/vite.config.ts
frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (frontend/AGENTS.md)
frontend/src/**/*.{ts,tsx}: Package code by feature rather than by layer; colocate related code, and export shared types from the primary feature file instead of centralizing them in separate type-only modules.
Use Effect for all async operations with typed error channels; do not introduce rawthrow,Promise.reject, or untypedcatchblocks in production code.
In HTTP/query code, import Effect from submodules such aseffect/Effectrather than theeffectbarrel, always forward TanStack Query'ssignalinto HTTP helpers, and route HTTP calls throughsrc/lib/http.tsonly.
Do not usethrow,Promise.reject, ortry/catchin new code; use Effect failures and recovery instead, except for the explicitly allowed boundary cases in SolidJS context hooks,main.tsxstartup, and imperative DOM library callbacks.
Files:
frontend/src/pages/Prototype/mockData.tsfrontend/src/visual/prototype.spec.ts
🧠 Learnings (8)
📚 Learning: 2026-02-07T02:55:47.514Z
Learnt from: 0xgleb
Repo: data-cartel/moneymentum PR: 60
File: ROADMAP.md:33-48
Timestamp: 2026-02-07T02:55:47.514Z
Learning: ROADMAP.md should stay high-level, focusing on actionable milestones and deliverables. For detailed methodology (specs, algorithms, acceptance criteria, data processing steps), place that information in SPEC.md and reference SPEC.md from ROADMAP.md when needed. This guides readers to detailed technical specs only when required while keeping the roadmap concise.
Applied to files:
ROADMAP.md
📚 Learning: 2026-07-02T00:55:01.632Z
Learnt from: 0xgleb
Repo: dataclique/moneymentum PR: 380
File: ROADMAP.md:40-42
Timestamp: 2026-07-02T00:55:01.632Z
Learning: In this repo, GitHub issue and pull request links in documentation (e.g., ROADMAP.md and other .md files) must use the `dataclique/moneymentum` repo URL (e.g., `https://github.com/dataclique/moneymentum/issues/...` and `https://github.com/dataclique/moneymentum/pull/...`). Do not reference the old org/repo name (e.g., `data-cartel/moneymentum`).
Applied to files:
ROADMAP.md
📚 Learning: 2026-05-22T10:57:15.613Z
Learnt from: levsgit
Repo: data-cartel/moneymentum PR: 227
File: frontend/src/pages/Portfolio/components/PositionsPanel/LeverageDialog.tsx:38-38
Timestamp: 2026-05-22T10:57:15.613Z
Learning: In this repo’s JavaScript/TypeScript code, it is acceptable to name unused callback parameters with a single underscore (`_`). If a callback parameter is unused and is named exactly `_` (e.g., `(_, index) => ...`), do not report it as an unused-parameter or naming-violation issue.
Applied to files:
frontend/playwright.visual.config.tsfrontend/src/pages/Prototype/mockData.tsfrontend/src/visual/prototype.spec.tsfrontend/vite.config.ts
📚 Learning: 2026-02-18T00:02:11.967Z
Learnt from: 0xgleb
Repo: data-cartel/moneymentum PR: 68
File: frontend/default.nix:18-24
Timestamp: 2026-02-18T00:02:11.967Z
Learning: In moneymentum's frontend Nix files, favor simpler derivations over introducing rigid Nix patterns (such as runHook guards in buildPhase/installPhase) unless enhanced composability is actually needed. Prioritize readability and maintainability; only add complexity when it yields clear benefits for the build or future extensibility. If a change risks introducing unnecessary forks or brittle build logic, prefer a straightforward, stable approach.
Applied to files:
frontend/bun.nixfrontend/default.nix
📚 Learning: 2026-06-10T18:21:32.471Z
Learnt from: 0xgleb
Repo: data-cartel/moneymentum PR: 295
File: deploy.nix:141-143
Timestamp: 2026-06-10T18:21:32.471Z
Learning: When generating shell scripts via Nix `pkgs.writeShellApplication`, assume the produced script already includes `set -euo pipefail` at the top. Because `set -e` aborts on the first non-zero exit code, explicit “check exit code after command X” comments/suggestions (e.g., after `rsync`) are unnecessary for correctness in scripts produced by `pkgs.writeShellApplication`. Instead, only request additional error handling if the script is not actually created by `pkgs.writeShellApplication` (or if commands are intentionally allowed to fail and the Nix script doesn’t enforce it).
Applied to files:
frontend/bun.nixfrontend/default.nixflake.nix
📚 Learning: 2026-02-08T02:00:41.421Z
Learnt from: 0xgleb
Repo: data-cartel/moneymentum PR: 61
File: flake.nix:18-18
Timestamp: 2026-02-08T02:00:41.421Z
Learning: Guideline: When using the crane flake (github:ipetkov/crane) as a library, do not rely on inputs.nixpkgs.follows or a global nixpkgs input. Crane expects to be given a pkgs instance via crane.mkLib pkgs, and will use whatever nixpkgs you pass to it. In flake.nix files, ensure you construct your library with crane.mkLib pkgs and do not assume a shared inputs.nixpkgs. This applies to any flake.nix in the repository or similar Nix repos that integrate crane.
Applied to files:
flake.nix
📚 Learning: 2026-06-11T03:08:44.958Z
Learnt from: 0xgleb
Repo: data-cartel/moneymentum PR: 286
File: fund:0-0
Timestamp: 2026-06-11T03:08:44.958Z
Learning: In this repo’s Nix setup, reference the external Solana program `data-cartel/fund` as a Nix flake input (pinned via `flake.lock`) rather than as a git submodule. Reviews should ensure CI/dev workflows do not require `git submodule update --init --recursive` and that `submodules: recursive` is not used. Also confirm the `FUND_IDL` env var in the Nix dev shell points to the flake-generated Anchor IDL JSON output path for the pinned `fund` input (so IDL resolution is automatic after a fresh clone/CI run without extra git steps).
Applied to files:
flake.nix
📚 Learning: 2026-06-11T03:09:32.842Z
Learnt from: 0xgleb
Repo: data-cartel/moneymentum PR: 286
File: .gitmodules:0-0
Timestamp: 2026-06-11T03:09:32.842Z
Learning: In the data-cartel/moneymentum monorepo, consume the Rust/Anchor Solana `fund` program (repo `data-cartel/fund`) via a Nix flake input, not via git submodules or packaging tricks. In `flake.nix`, pin the dependency to a specific commit (e.g., `inputs.fund.url = "github:data-cartel/fund/<commit>"`), and ensure the exact pinned revision is recorded in `flake.lock`. The dev shell should export `FUND_IDL` that points to the flake’s generated Anchor IDL JSON for the current system. During code review for Nix flake changes, do not suggest switching this dependency to git submodules (`.gitmodules`), including it as npm/JS packages, adding it directly to the monorepo, or using branch tracking instead of commit pinning.
Applied to files:
flake.nix
🪛 zizmor (1.29.0)
.github/workflows/visual-regression.yaml
[warning] 37-37: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 37-37: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 84-84: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[info] 34-34: workflow or action definition without a name (anonymous-definition): this job
(anonymous-definition)
[warning] 14-28: insufficient job-level concurrency limits (concurrency-limits): workflow is missing concurrency setting
(concurrency-limits)
🔇 Additional comments (17)
frontend/src/pages/Prototype/mockData.ts (4)
5-23: LGTM!
339-346: LGTM!
769-769: LGTM!
792-794: LGTM!frontend/playwright.visual.config.ts (1)
1-63: LGTM!frontend/src/visual/prototype.spec.ts (1)
1-16: LGTM!frontend/package.json (2)
15-16: LGTM!
46-46: LGTM!frontend/tsconfig.node.json (1)
25-30: LGTM!frontend/vite.config.ts (2)
5-5: LGTM!
64-64: LGTM!frontend/.gitignore (1)
27-29: LGTM!frontend/bun.nix (1)
344-347: LGTM!Also applies to: 1280-1283, 1792-1799
flake.nix (1)
196-237: LGTM!Also applies to: 281-282, 332-332
frontend/default.nix (1)
72-97: LGTM!.github/workflows/visual-regression.yaml (1)
1-13: LGTM!Also applies to: 30-36, 39-73
ROADMAP.md (1)
173-174: LGTM!
1899c1e to
a36c1e1
Compare
a36c1e1 to
5066ee4
Compare
Pin checkout and upload-artifact to release commit SHAs, stop persisting checkout credentials, cancel superseded runs per ref, upload playwright diffs when the comparison fails, and build the exported frontend-visual-test package so a broken derivation cannot pass CI.
5066ee4 to
cc25ca2
Compare
The deterministic mock-data seeding changed the rendered chart after the previous baseline was captured, so CI compared against a stale PNG. This baseline is the workflow_dispatch refresh artifact from run 32043163300.
Motivation
Nothing exercises the rendered frontend, so layout and styling regressions reach prod without any failing check -- the dense dashboard layout the design principles demand has no guard.
Closes #439
Solution
This is part 2 of 3 in a stack made with GitButler:
Summary by CodeRabbit
Tests
Chores