Sign up
View stargazersSign up

refactor(mobile): showcase capture readiness is a tested pure model#25816

merged2
main
effect/split-mobile-showcase-controller
opened1d agobyJJulius Marminge
4 files changed
View on GitHubTwo-way sync enabledPR last imported 1d ago

Problem

The mobile showcase capture coordinator decided route scene, theme, and capture readiness inline with no tests, and kept its ready marker in sync through effects: one effect mirrored threads and projects into a ref every render, and the readiness effect cleared the marker with setState after commit, so a stale showcase-ready-* marker stayed mounted for one commit after readiness was lost.

Change

  • New showcaseCaptureModel.ts (pure): showcaseSceneOnScreen (with the private sceneFromPathname), isShowcaseThemeApplied, showcaseReadinessKey, plus tests. These are the same rules as before, moved out of the component.
  • ShowcaseCaptureCoordinator.tsx builds a readiness key from the model. The marker hides during the same render that the key becomes null (readiness lost), using React's adjust-state-during-render pattern. A changed non-null key (a ReviewSheet redraw or a theme change while ready) keeps the marker and restarts the 500ms settle, as on main. The readiness effect only owns the settle timer and animation frames.
  • Agent activity staging reads current threads and projects through useEffectEvent instead of a ref that an effect kept in sync.
  • showcaseRenderSignal.ts exposes one { signal, reports } snapshot, replaced on each report, so a redraw in the same scene and theme still restarts the settle delay. Before, this happened through the signal object's identity in the effect deps.

First slice of the showcase controller unit. Moving the whole coordinator into a module-level installer (removing the rest of its effects) comes in a later PR.

What this improves

  • Sync removed: the entities-ref mirror effect, and the setReadyScene(null) set-state-in-effect (along with its lint suppression).
  • Readiness rules are now covered by unit tests (showcaseCaptureModel.test.ts).
  • No bug-fix claim. Because the stale marker frame is cleared during render, the marker now disappears one commit sooner when readiness is lost. Only the showcase capture build (EXPO_PUBLIC_SHOWCASE=1) can see this.

Behavior changes

  • The ready marker hides in the same render that readiness is lost, instead of one commit later. Nothing else changes: same settle delay, frames, logging, and navigation.

Effects

ShowcaseCaptureCoordinator.tsx: 11 -> 10. showcaseCaptureModel.ts, showcaseRenderSignal.ts: 0 -> 0.

Verification

  • vp test run src/features/showcase (apps/mobile): 6 passed
  • vp run check:changed: format, lint, knip 1/2 and 2/2, typecheck (2 packages), test all pass

Model: Claude Opus 5.5, Claude Code harness.

🤖 Generated with Claude Code

Checks

24 successful · 15 skipped
All checks passed
6a2a9fb

Added 1 commit

  • refactor(mobile): showcase capture readiness is a tested pure model9777275
MmacroscopeappBot1d ago
Synced from GitHub

Approvability

Verdict: Approved at 6a2a9fb

Macroscope's review found this PR approvable — This refactors opt-in showcase capture readiness into a tested pure model and improves handling of redraws and stale readiness markers. The affected behavior is confined to the EXPO_PUBLIC_SHOWCASE capture harness, with no normal customer-path or product-default changes.

You can add or adjust custom eligibility rules. Learn more.

CcoderabbitaiBot1d ago
Synced from GitHub

Review in Change Stack →

No actionable comments were generated in the recent review.

External image Recent review info
External image Run configuration
External image Commits

Reviewing files that changed from the base of the PR and between 977727547156aefe8e22954b09706b37ad418c74 and 6a2a9fb97c43dbef9c7aebee6730df15508652f3.

External image Files selected for processing: 3
  • apps/mobile/src/features/showcase/ShowcaseCaptureCoordinator.tsx
  • apps/mobile/src/features/showcase/showcaseCaptureModel.ts
  • apps/mobile/src/features/showcase/showcaseRenderSignal.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.

Walkthrough

The showcase capture model now provides shared scene selection, theme-application, and readiness checks. The coordinator uses a readiness key that includes render-report state. Render snapshots now expose a signal and report count.

Changes: Showcase capture readiness

Layer / File(s)Summary
Scene and readiness rules
apps/mobile/src/features/showcase/showcaseCaptureModel.ts, apps/mobile/src/features/showcase/showcaseCaptureModel.test.ts		Shared helpers map routes to scenes, check theme application, and generate readiness keys. Tests cover scene detection, theme readiness, readiness preconditions, and render state.
Render snapshots and coordinator readiness
apps/mobile/src/features/showcase/showcaseRenderSignal.ts, apps/mobile/src/features/showcase/ShowcaseCaptureCoordinator.tsx		Render snapshots expose the signal and report count. The coordinator reads current fixture entities and uses shared helpers and render state to manage a keyed readiness marker.

Priority: External image Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Refactor

Sequence Diagram(s)

mermaid
1sequenceDiagram
2 participant showcaseRenderSignal
3 participant ShowcaseCaptureCoordinator
4 participant showcaseCaptureModel
5 showcaseRenderSignal->>ShowcaseCaptureCoordinator: Provide render signal and report count
6 ShowcaseCaptureCoordinator->>showcaseCaptureModel: Calculate readiness key from capture state
7 showcaseCaptureModel-->>ShowcaseCaptureCoordinator: Return readiness key or null
8 showcaseCaptureCoordinator->>showcaseCaptureCoordinator: Update readiness marker
Merge Risk: External image Minimal · up to 6a2a9

The showcase capture changes are ready to merge after normal checks. No new capture failure was established.

Pre-merge checks: Passed 3 · Failed 1

Failed Failed checks: 1 warning

Check nameStatusExplanationResolution
Description checkWarningThe description clearly covers the problem, implementation, behavior changes, effects, and verification results. It does not include the required Scope and approval section or explain why the change q…Add a Scope and approval section. Link the triaged issue or maintainer approval with the approval comment, or explain why this focused refactor qualifies for the repository's small, obvious-change exemption.
Passed Passed checks: 3 passed
Check nameStatusExplanation
Title checkPassedThe title clearly identifies the mobile showcase readiness refactor and states that the readiness model is tested.
Linked Issues checkPassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes checkPassedCheck skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description clearly covers the problem, implementation, behavior changes, effects, and verification results. It does not include the required Scope and approval section or explain why the change qualifies for an exemption from prior issue or maintainer approval.

Resolution

Add a Scope and approval section. Link the triaged issue or maintainer approval with the approval comment, or explain why this focused refactor qualifies for the repository's small, obvious-change exemption.

Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

Added 1 commit

  • fix(mobile): showcase redraws keep the ready marker while they re-settle6a2a9fb