Update: Use incremental step building for large event counts#46
Update: Use incremental step building for large event counts#46
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
FL4TLiN3
force-pushed
the
perf/optimize-buildsteps
branch
from
0e53728 to
708b00c
Compare
|
Deployment failed with the following error: Learn More: https://vercel.com/fl4tlin3s-projects?upgradeToPro=build-rate-limit |
| setSteps(buildStepsFromMap(stepMapRef.current)) | ||
| } | ||
| processedCountRef.current = newEvents.length | ||
| return newEvents |
There was a problem hiding this comment.
Bug: Side effects inside setState updater break React patterns
The addEvent function performs side effects (mutating stepMapRef.current via processEvent and calling setSteps) inside the setEvents updater function. React expects state updater functions to be pure. In React's Strict Mode or Concurrent Mode, updaters may be invoked multiple times for verification, causing the ref to be mutated multiple times while only one state update commits. Although the current operations are mostly idempotent, this pattern can cause the ref and state to become desynchronized under certain React scheduling scenarios, potentially leading to stale or inconsistent UI state.
Revert to the original useMemo-based implementation instead of incremental updates. The optimization caused issues with React patterns (side effects in setState updater) and historical event handling. The simpler approach is more maintainable and follows React best practices.
FL4TLiN3
force-pushed
the
perf/optimize-buildsteps
branch
from
708b00c to
a462d50
Compare
| 2. **Short-term**: Reduce code duplication in `apps/` directory | ||
| 3. **Long-term**: Consider performance optimization for high event counts | ||
|
|
||
| The codebase follows the project's coding standards (no comments, minimal blank lines, English only) and integrates cleanly with `@perstack/core` types. |
There was a problem hiding this comment.
Bug: Code review report accidentally committed to repository
The CODE_REVIEW_REPORT.md file appears to be an internal code review document that was accidentally committed to the repository. This file contains review notes, recommendations, and action items that are typically shared separately rather than being part of the codebase. The PR title mentions "incremental step building" but this file is unrelated documentation that exposes internal review findings and should likely be removed before merging.
1 participant
Summary
buildStepsto support incremental updates instead of rebuilding from scratch on every eventuseRefto maintainstepMapacross renders for O(1) event processingThis optimization reduces the time complexity from O(n) to O(1) for each
addEventcall, which improves performance for long-running sessions with high event counts.Closes #42
Test plan
Note
Adds
packages/tui/apps/start/app.tsximplementing the Start TUI App (steps rendering, browsing router, run settings) and includes a code review report document.packages/tui/apps/start/app.tsxexportingAppthat composesuseAppStatewithStep,BrowserRouter, andRunSetting.Static/Step, showsBrowserRouterwhile browsing states, andRunSettingduring editing/running.CODE_REVIEW_REPORT.mdwith findings and recommendations for@perstack/tui.Written by Cursor Bugbot for commit a462d50. This will update automatically on new commits. Configure here.