feat: improve mocking and add react devtools - #65
Merged
Conversation
…mulated states Replaces the aspirational mock README with a real implementation: - layer(): in-memory FirestoreService backed by a SubscriptionRef, so streamDoc/streamQuery are live — writes and state toggles re-emit through already-subscribed streams like onSnapshot - fixture()/rawFixture(): seed hard-coded models encoded through the real schema pipeline so reads exercise actual decoding - MockController service (provided by the same layer): toggle collections between data/empty/loading/error at runtime, seed docs, simulate latency, reset — the control surface for a future devtools panel - In-process query evaluation (where/orderBy/limits/cursors/and/or) with Firestore type ordering - Write fidelity: ServerTimestamp materialized via Clock, Delete/ ArrayUnion/ArrayRemove sentinels honored, not-found on update, recursive delete The existing MockFirestoreService stub remains exported for backwards compatibility. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NFRv4LXEZkuAb5EzX5mhV6
- @effect-firebase/devtools: new package with MockDevtoolsPanel, a React
panel that lists collections with doc counts and toggles each between
data/empty/loading/error, picks the simulated error code, controls
latency, and resets to fixtures — subscribed live to the mock store
- firestoreMockPlugin(controller): plugin factory for
<TanStackDevtools plugins={[...]}>; the plugin interface is declared
structurally so TanStack Devtools is not a dependency
- mock: add make() returning { layer, controller } so code outside the
Effect runtime (devtools, Storybook, tests) can drive the same store
the app's layer provides; layer() now builds on make() with
fresh-per-provide semantics preserved
- mock: add controller.latency getter for the panel's latency display
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NFRv4LXEZkuAb5EzX5mhV6
…nto claude/react-effect-atoms-mock-4lzbf8
Based on the atom-based example app from claude/vigilant-pascal-qgjsm4 (merged): the Firestore layer is swappable at the registry boundary via firestoreLayerAtom, so mock mode is one initialValues entry. - example/app/src/lib/mock.ts: mock backend handle with schema-encoded Post/Author fixtures - app.tsx: VITE_MOCK_BACKEND=1 seeds firestoreLayerAtom with the mock layer instead of the emulator client; a single TanStack Devtools shell now hosts the router panel plus the Firestore Mock panel, with onStateChange refreshing latestPostsAtom so streams that ended on a simulated error re-subscribe - mock: implement withTransaction/withBatch (pass-through — no concurrency or staging semantics to simulate) - devtools: pin explicit heights on panel elements so shell CSS resets that stretch divs cannot distort the layout - pnpm example:mock script; REACT.md section 7 documents the workflow Verified end-to-end in a real browser: fixtures render through atoms, empty/loading/error toggle live from the panel, data recovers after a terminal stream error via refresh, and form writes flow through the repository into the live stream (doc counts update in the panel). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NFRv4LXEZkuAb5EzX5mhV6
…toms-mock-4lzbf8 # Conflicts: # example/app/package.json # pnpm-lock.yaml
Contributor
|
Warning Review limit reached
Next review available in: 53 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe pull request adds a reactive in-memory Firestore backend, runtime state controls, a TanStack Devtools package, example application wiring, fixture utilities, query evaluation, and timestamp conversion fixes. ChangesMock backend foundation
Devtools package
Example application integration
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
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 |
fwal
changed the title
…toms-mock-4lzbf8 # Conflicts: # example/app/package.json # packages/mock/README.md # pnpm-lock.yaml
- react 19.2.8 in devtools devDeps (two React copies broke jsdom tests) - useState initializer instead of side-effecting useMemo in app.tsx (rejected by eslint-plugin-react-hooks 7 / React Compiler) - apply prettier 3.9.6 formatting to new files Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NFRv4LXEZkuAb5EzX5mhV6
github-actions
Bot
added
📖 docs
…Arbitrary Generate N schema-valid documents from a model using effect's bundled fast-check (effect/testing/FastCheck — no new dependency). Deterministic per seed so dev pages don't churn across reloads; document IDs are sequential rather than sampled to keep paths valid and collision-free. Complements hand-written fixture() docs: generated data satisfies the schema but reads as noise, so use it for volume (lists, pagination, layout stress) and hand-written docs for demo content. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NFRv4LXEZkuAb5EzX5mhV6
Schema.toArbitrary on models previously produced DateTimes far outside what a Firestore Timestamp can store (years beyond 9999). The timestamp codecs now decode to a DateTimeUtc annotated with a toArbitrary hook that stays within 0001-01-01..9999-12-31, so generatedFixture and property tests produce storable dates. Also fixes Timestamp.fromMillis/fromDate for pre-1970 instants: the seconds/nanos split used a signed remainder, double-counting the fractional second for negative epoch millis. Nanoseconds are now always non-negative, matching Firestore, so the roundtrip holds. (Found by the new range test — the generated arbitraries caught it.) Mock README documents per-field tuning: built-in checks guide generation; toArbitrary annotations replace it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NFRv4LXEZkuAb5EzX5mhV6
fwal
marked this pull request as ready for review
August 7, 2026 12:01
Greptile SummaryThe PR adds a reactive in-memory Firestore backend, React development tools for controlling simulated collection states, and a mock-enabled example workflow. It also corrects pre-epoch timestamp conversion.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| packages/mock/src/lib/firestore/layer.ts | Implements the reactive Firestore mock layer, controller operations, live subscriptions, simulated states, latency, and reset behavior. |
| packages/mock/src/lib/firestore/query-filter.ts | Evaluates mock query constraints and now excludes documents missing ordered fields before sorting, cursors, and limits. |
| packages/mock/src/lib/firestore/value.ts | Implements Firestore value comparison, sentinel materialization, and set, merge, and update transformations. |
| packages/devtools/src/lib/panel.tsx | Provides collection-state, error, latency, clear, and reset controls with post-operation state-change notifications. |
| packages/devtools/src/lib/plugin.tsx | Exposes the mock panel through a structural TanStack Devtools plugin interface. |
| example/app/src/app/app.tsx | Selects the mock backend through an environment flag and mounts router and Firestore panels in one Devtools shell. |
| packages/effect-firebase/src/lib/firestore/schema/timestamp.ts | Normalizes the seconds/nanoseconds split so pre-1970 millisecond values round-trip correctly. |
Sequence Diagram
sequenceDiagram
participant Panel as Mock Devtools Panel
participant Controller as MockController
participant Store as Reactive Mock Store
participant Consumer as Application Consumer
Panel->>Controller: Set collection state / reset / clear
Controller->>Store: Apply state transition
Store-->>Controller: Updated snapshot
Controller-->>Panel: Effect completes
Panel->>Consumer: onStateChange(collection, effective state)
Consumer->>Store: Subscribe with fresh identity
Store-->>Consumer: Data, empty, loading, or error behavior
Reviews (8): Last reviewed commit: "fix(mock): address review findings on th..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
- devtools panel: clear and reset now invoke onStateChange (with the wildcard key and data state) so consumers terminally failed on a simulated error get refreshed; all notifications now fire only after the controller effect has applied, so refreshes re-subscribe against the new state instead of racing it - mock query evaluation: documents missing a field named by an orderBy constraint are excluded from results, matching Firestore Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NFRv4LXEZkuAb5EzX5mhV6
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 20
🤖 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 `@example/app/package.json`:
- Line 19: Keep the example app on its declared Tailwind v3 stack by removing
the `@tailwindcss/vite` dependency and its associated Tailwind v4 integration,
including the tailwindcss('example/app/src/styles.css') usage and `@import`
'tailwindcss' style setup; do not migrate the existing component dependencies to
v4.
In `@example/app/src/routes/__root.tsx`:
- Line 2: Update the import in the root route module to use the named App export
from app/app.tsx instead of a default import, preserving the existing App usage.
In `@packages/devtools/README.md`:
- Around line 19-24: Update the README example around make and fixture to define
posts and authors before they are passed in, or explicitly state that they must
already be declared; ensure the example is runnable or clearly documents the
prerequisite.
In `@packages/devtools/src/lib/panel.tsx`:
- Around line 247-250: Update applyLatency to normalize its numeric input before
use: convert non-finite or negative values to zero, then pass the normalized
value to both setLatencyMs and controller.setLatency. Preserve the existing
latency update flow for valid non-negative finite values.
- Around line 241-245: Update setState in packages/devtools/src/lib/panel.tsx
(lines 241-245) to await successful completion of controller.setState before
invoking onStateChange. In packages/devtools/src/lib/panel.spec.tsx (lines
101-124), make notification completion observable and assert the controller
state is applied before notification.
In `@packages/mock/src/lib/firestore/controller.ts`:
- Around line 46-50: Update
packages/mock/src/lib/firestore/controller.ts#L46-L50 so the seed method is
generic in R, accepts Fixture<R>, and returns an effect requiring R while
preserving its existing error and success types. Update
packages/mock/src/lib/firestore/layer.ts#L41-L45 to make LayerOptions generic
over fixture services and retain those services in its fixtures type, ensuring
both fixture entry points support models with encoding services.
In `@packages/mock/src/lib/firestore/fixture.spec.ts`:
- Around line 105-135: Extend the repository test loop for posts returned by
repo.query to assert that each decoded post.id matches the expected
generated-NNNN identifier derived from its position, confirming IDs are
recovered from document paths after generatedFixture strips the stored idField.
In `@packages/mock/src/lib/firestore/fixture.ts`:
- Around line 52-67: Detect duplicate document paths before assigning entries in
the fixture-building loop, and fail with an error instead of allowing a later
document to overwrite an earlier one. Apply the same duplicate-path validation
in generatedFixture when options.id produces repeated values, while preserving
the existing missing-ID validation and error behavior.
- Around line 121-131: Document the `options.id` callback contract near the ID
generation logic in the fixture builder, explicitly stating that its `index`
argument is zero-based, while preserving the existing custom and default ID
generation behavior.
- Around line 114-119: Validate options.count in the fixture generation flow
before passing it as numRuns to FastCheck.sample. Ensure zero and negative
counts are handled explicitly according to the fixture contract, avoiding empty
results or unhandled FastCheck failures, while preserving positive-count
sampling and the existing digits calculation.
In `@packages/mock/src/lib/firestore/layer.spec.ts`:
- Around line 389-433: Add controller-suite coverage for
MockController.clearState, setDoc, and removeDoc. Verify clearState removes the
collection-specific state so queries fall back to MockState.All, and verify
setDoc creates a directly addressable document that removeDoc subsequently
removes, using the existing FirestoreService, MockController, Option assertions,
and fixture setup.
In `@packages/mock/src/lib/firestore/layer.ts`:
- Around line 179-192: Move ID generation and collision checking from the stale
SubscriptionRef snapshot into the write callback in add. Use the callback’s
current docs to regenerate IDs until the selected docPath is unused, then
applySet and return the generated id/path while preserving collection-path
validation.
- Around line 61-68: Update the Random.nextIntBetween call in generateId to pass
the halfOpen option, ensuring the selected index is strictly below
ID_ALPHABET.length and always resolves to a valid alphabet character.
- Around line 499-519: Clarify the isolation guarantee in the `layer`
documentation: state that fresh stores are guaranteed per provide only when the
layer instance is not reused in a shared memoization scope, or explicitly
mention the `{ local: true }` requirement for reused layers. Update the nearby
`layer` doc comment without changing `make`’s shared-store behavior.
- Around line 469-485: Update the seedOnce flow so concurrent callers await the
same in-progress fixture-building effect instead of returning immediately after
Ref.getAndSet(seeded, true) reports an existing seed. Memoize or otherwise share
the seeding operation around seedOnce, while preserving the existing snapshot
and SubscriptionRef updates, so every provider observes the completed seeded
store.
In `@packages/mock/src/lib/firestore/query-filter.spec.ts`:
- Around line 118-131: Add a `supports and filters` test alongside the existing
`Where` and `Or` cases, using `Query.And` with published status and views
greater than 25, and assert that `ids(applyConstraints(...))` returns only
`['2']`.
In `@packages/mock/src/lib/firestore/query-filter.ts`:
- Around line 211-216: Update the result-limiting logic in the query filter so
limitToLast takes precedence whenever both limit and limitToLast are provided,
preventing sequential slicing. Preserve the existing limit behavior when
limitToLast is absent and the existing limitToLast slicing otherwise.
In `@packages/mock/src/lib/firestore/value.spec.ts`:
- Around line 26-31: Extend the “orders timestamps by instant” test with a
pre-1970 timestamp created via FirestoreSchema.Timestamp.fromMillis using a
negative millisecond value, then assert compare orders it correctly against a
later timestamp and that the timestamp round-trips to the original millisecond
value. Cover the negative-seconds/non-negative-nanoseconds split introduced by
Timestamp.fromMillis without changing the existing positive-order assertions.
In `@packages/mock/src/lib/firestore/value.ts`:
- Around line 26-36: Update rank to assign undefined its own distinct rank
before the generic fallback, then make compare’s final fallback deterministic
for values that are neither records nor otherwise comparable. Ensure unrelated
rank-8 values, including sentinels, no longer compare as equal, while preserving
existing ordering and equality for supported Firestore types.
In `@REACT.md`:
- Around line 368-371: Update the Runtime setup documentation and the referenced
application example to use one consistent initialization pattern, preferably the
app’s existing useState initializer; remove the outdated useMemo requirement and
example, or explicitly document the reason for retaining different patterns.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 96ded7cb-592e-4d0e-87a2-627e3bcdbfa5
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (35)
REACT.mdexample/app/package.jsonexample/app/src/app/app.tsxexample/app/src/lib/mock.tsexample/app/src/routes/__root.tsxexample/app/tsconfig.app.jsonpackage.jsonpackages/devtools/README.mdpackages/devtools/eslint.config.mjspackages/devtools/package.jsonpackages/devtools/src/index.tspackages/devtools/src/lib/panel.spec.tsxpackages/devtools/src/lib/panel.tsxpackages/devtools/src/lib/plugin.tsxpackages/devtools/tsconfig.jsonpackages/devtools/tsconfig.lib.jsonpackages/devtools/tsconfig.spec.jsonpackages/devtools/vite.config.tspackages/effect-firebase/src/lib/firestore/model/datetime.tspackages/effect-firebase/src/lib/firestore/schema/timestamp.tspackages/mock/README.mdpackages/mock/src/index.tspackages/mock/src/lib/firestore/controller.tspackages/mock/src/lib/firestore/fixture.spec.tspackages/mock/src/lib/firestore/fixture.tspackages/mock/src/lib/firestore/layer.spec.tspackages/mock/src/lib/firestore/layer.tspackages/mock/src/lib/firestore/query-filter.spec.tspackages/mock/src/lib/firestore/query-filter.tspackages/mock/src/lib/firestore/state.tspackages/mock/src/lib/firestore/store.tspackages/mock/src/lib/firestore/value.spec.tspackages/mock/src/lib/firestore/value.tspackages/mock/vite.config.tstsconfig.json
Bugs: - generateId: Random.nextIntBetween includes the upper bound by default in effect v4, so ids could contain the string 'undefined'; pass halfOpen so indexes stay within the alphabet - add: collision-check ids against the docs the write actually sees instead of a pre-read snapshot, so a concurrent add can't be replaced - make: seed fixtures through Effect.cached so concurrent providers await the same seeding run instead of racing past a boolean ref - compare: undefined gets its own rank and opaque values (sentinels) are equal only on identity, so arrayUnion dedup can't absorb unrelated members - query evaluation: limitToLast takes precedence over limit (Firestore rejects the combination; sequential slicing produced impossible sets) - panel: clamp latency input to non-negative finite values - fixtures: fail on duplicate document ids, validate count, document the zero-based id callback Docs: layer() memoization caveat, encoding-services limitation on fixture entry points, runnable devtools README example, REACT.md runtime setup aligned with the useState-initializer pattern. Tests: And filters, limit+limitToLast precedence, pre-1970 timestamp ordering/roundtrip, opaque-value equality, clearState wildcard fallback, setDoc/removeDoc, generated-id decoding, count validation, duplicate-id rejection. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NFRv4LXEZkuAb5EzX5mhV6
Owner
Author
|
Addressed the CodeRabbit review in 26829ef — 16 of 20 findings fixed (the standouts were real bugs: Skipped with reasons:
Generated by Claude Code |
Toggling 'loading' in the devtools panel appeared to do nothing: the stream goes silent (correct — a stream cannot retract its last emission), and effect-atom results retain their previous value across registry.refresh AND component remounts (the new subscriber attaches in the same commit as the old one detaches, so the atom node never disposes). The stale posts stayed on screen. Fix: give reads a fresh atom identity per toggle. latestPostsAtom is now an Atom.family keyed by mockEpochAtom, which the devtools onStateChange bumps — a new epoch is a new atom that re-subscribes from Initial against the toggled state: spinner for loading, failure for error, data on recovery. Outside mock mode the epoch is always 0, so the family behaves like the previous single shared atom. Docs updated (devtools README, REACT.md §7): the previous guidance to refresh atoms only worked when the fresh subscription emits; the epoch identity pattern covers loading too. Verified in the browser: loading now shows the spinner, and empty/error/data still toggle live. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NFRv4LXEZkuAb5EzX5mhV6
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@packages/mock/src/lib/firestore/fixture.ts`:
- Around line 66-74: In packages/mock/src/lib/firestore/fixture.ts at lines
66-74, add a shared document-ID path-segment validator and invoke it before
constructing path for fixture IDs, rejecting any ID containing '/'. Apply the
same validator to IDs returned by options.id at lines 152-160 before path
construction, while preserving duplicate-ID handling; add coverage for both
invalid fixture IDs and invalid generated IDs.
In `@packages/mock/src/lib/firestore/value.ts`:
- Around line 114-116: Update the rank-9 opaque-value branch in compare so
distinct values receive a deterministic antisymmetric ordering rather than
always returning 1; retain a 0 result only when a and b are identical, using
stable identity ordering or rejecting unsupported opaque values before storage.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: bd512366-2bad-4bfb-aadd-acf01cf6d301
📒 Files selected for processing (16)
REACT.mdexample/app/src/app/app.tsxexample/app/src/lib/atoms.tsexample/app/src/routes/firestore.tsxpackages/devtools/README.mdpackages/devtools/src/lib/panel.spec.tsxpackages/devtools/src/lib/panel.tsxpackages/mock/src/lib/firestore/controller.tspackages/mock/src/lib/firestore/fixture.spec.tspackages/mock/src/lib/firestore/fixture.tspackages/mock/src/lib/firestore/layer.spec.tspackages/mock/src/lib/firestore/layer.tspackages/mock/src/lib/firestore/query-filter.spec.tspackages/mock/src/lib/firestore/query-filter.tspackages/mock/src/lib/firestore/value.spec.tspackages/mock/src/lib/firestore/value.ts
- value.ts: distinct opaque values (sentinels, bigints) previously compared as 1 in both argument orders, breaking the comparator contract and making sorts engine-dependent. Objects now order by a stable first-seen identity (WeakMap), primitives by string form; 0 remains reserved for equal values. - fixture.ts: document IDs are validated as single path segments in both fixture() and generatedFixture() — an ID containing '/' would silently place the document outside the intended collection. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NFRv4LXEZkuAb5EzX5mhV6
reset restores the backend's configured initial states, which may not be 'data' — the callback previously reported wildcard Data unconditionally. clear and reset now read the states back from the controller after the operation and report the resolved wildcard state. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NFRv4LXEZkuAb5EzX5mhV6
Contributor
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 (2)
packages/mock/src/lib/firestore/fixture.ts (1)
43-48: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winValidate
collectionPathbefore fixture construction.
fixture()validates the ID but notcollectionPath. For example,collectionPath: 'posts/a'producesposts/a/id, while the mock rejectsposts/aas a collection path. The seeded document is then inaccessible through the declared fixture collection.
packages/mock/src/lib/firestore/fixture.ts#L43-L48: validateoptions.collectionPathwith the shared collection-path validator before building document paths.packages/mock/src/lib/firestore/fixture.spec.ts#L45-L53: add coverage that an even-segment or empty-segment collection path fails during fixture construction.🤖 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 `@packages/mock/src/lib/firestore/fixture.ts` around lines 43 - 48, Update fixture() in packages/mock/src/lib/firestore/fixture.ts#L43-L48 to validate options.collectionPath with the shared collection-path validator before constructing document paths, while preserving ID validation. Add coverage in packages/mock/src/lib/firestore/fixture.spec.ts#L45-L53 confirming fixture construction rejects even-segment and empty-segment collection paths.packages/mock/README.md (1)
130-140: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winImport every identifier used by this example.
The snippet uses
Effect.runPromiseandAtom.runtime, but it imports onlymake. Add theEffectimport and the import forAtomfrom its owning package.🤖 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 `@packages/mock/README.md` around lines 130 - 140, Update the README example imports to include every referenced identifier: add Effect from its package for Effect.runPromise and import Atom from its owning package for Atom.runtime, while retaining the existing make import.
♻️ Duplicate comments (1)
packages/mock/src/lib/firestore/layer.ts (1)
187-190: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPrevent
addfrom overwriting an existing generated document.Line 190 replaces an existing document when
generateIdreturns an occupied ID.updateEffectmakes the replacement atomic, but it does not enforce uniqueness.Generate and check the ID inside the
writemutation against its currentdocsvalue. Retry until the path is unused before applyingapplySet.🤖 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 `@packages/mock/src/lib/firestore/layer.ts` around lines 187 - 190, Update the add flow around generateId and write so ID generation and occupancy checks occur inside the write mutation using its current docs value. Retry generating IDs until the resulting docPath is unused, then apply applySet only to that new path, preserving atomic uniqueness and preventing overwrites.
🤖 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 `@packages/mock/src/lib/firestore/value.ts`:
- Around line 114-120: Replace symbol-string-based ordering in compareOpaque
with stable per-symbol identity IDs stored in a Map<symbol, number>, preserving
antisymmetric non-zero ordering for distinct symbols; alternatively reject
symbols as unsupported. Update
packages/mock/src/lib/firestore/value.ts#L114-L120 accordingly, and add
same-description distinct-symbol coverage with a non-zero antisymmetric
comparison assertion in packages/mock/src/lib/firestore/value.spec.ts#L49-L57.
---
Outside diff comments:
In `@packages/mock/README.md`:
- Around line 130-140: Update the README example imports to include every
referenced identifier: add Effect from its package for Effect.runPromise and
import Atom from its owning package for Atom.runtime, while retaining the
existing make import.
In `@packages/mock/src/lib/firestore/fixture.ts`:
- Around line 43-48: Update fixture() in
packages/mock/src/lib/firestore/fixture.ts#L43-L48 to validate
options.collectionPath with the shared collection-path validator before
constructing document paths, while preserving ID validation. Add coverage in
packages/mock/src/lib/firestore/fixture.spec.ts#L45-L53 confirming fixture
construction rejects even-segment and empty-segment collection paths.
---
Duplicate comments:
In `@packages/mock/src/lib/firestore/layer.ts`:
- Around line 187-190: Update the add flow around generateId and write so ID
generation and occupancy checks occur inside the write mutation using its
current docs value. Retry generating IDs until the resulting docPath is unused,
then apply applySet only to that new path, preserving atomic uniqueness and
preventing overwrites.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e262b355-300c-4178-874b-abaed8edd744
📒 Files selected for processing (11)
packages/devtools/src/lib/panel.spec.tsxpackages/devtools/src/lib/panel.tsxpackages/devtools/src/lib/plugin.tsxpackages/effect-firebase/src/lib/firestore/schema/timestamp.tspackages/mock/README.mdpackages/mock/src/lib/firestore/fixture.spec.tspackages/mock/src/lib/firestore/fixture.tspackages/mock/src/lib/firestore/layer.tspackages/mock/src/lib/firestore/store.tspackages/mock/src/lib/firestore/value.spec.tspackages/mock/src/lib/firestore/value.ts
- fixture(): validate collectionPath before building document paths — an even-segment path would seed documents unreachable from the declared collection - value.ts: distinct symbols share a string form; order them by first-seen identity so they never compare as equal - README: import Effect and Atom in the make() example - layer.ts add(): document that generating an ID without an occupancy check matches the real SDK (collision odds ~62^-20) — deliberately kept simple Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NFRv4LXEZkuAb5EzX5mhV6
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Develop pages against a static mock backend: seed hard-coded models through your real schemas, then toggle collections between data / empty / loading / error live from a devtools panel — no emulator required.
@effect-firebase/mock— implements the in-memory backend the README already described. ReactiveSubscriptionRefstore, so live streams re-emit on writes and state toggles.fixture()encodes models through their schema,generatedFixture()derives volume data viaSchema.toArbitrary(deterministic per seed, tunable withtoArbitraryannotations),MockControllerdrives states/latency/seeding at runtime, andmake()returns{ layer, controller }for use outside the Effect runtime. Queries are evaluated in-process; server timestamps and field sentinels are honored. The existing stub stays exported.@effect-firebase/devtools— new package:MockDevtoolsPanel(React) plusfirestoreMockPlugin()for the TanStack Devtools shell (structural interface, no TanStack dependency).onStateChangelets consumers re-subscribe streams that ended on a simulated error (terminal, likeonSnapshot).Core — generated timestamps stay within Firestore's storable range (years 1–9999), and
Timestamp.fromMillis/fromDatenow roundtrip pre-1970 instants (nanoseconds are always non-negative, matching Firestore).Example app —
pnpm example:mockseedsfirestoreLayerAtomwith the mock layer and mounts one devtools shell with the Router and Firestore Mock panels. Documented in REACT.md §7.Testing
57 new specs (mock + devtools) and a Playwright run against
pnpm example:mockcovering live toggles, error recovery via refresh, and form writes flowing through the store.nx run-many -t lint,build,testgreen across all projects.🤖 Generated with Claude Code
https://claude.ai/code/session_01NFRv4LXEZkuAb5EzX5mhV6
Summary by CodeRabbit