Stream handling / Stream idle mode v2; a ton of network related changes (ping; scene indicator -> stream indicator, stream resizing (even on pause)) by lf94 · Pull Request #5312 · KittyCAD/modeling-app

lf94 · 2025-02-07T22:04:56Z

The PR has been called "stream idle" for awhile but it's actually a major rework of the network stream code.

There are no new additions other than current ping; everything is a change to the existing features:

The model state indicator, in the bottom right, no longer signals that the scene is done executing. It now tells the user if the stream is paused, playing or loading.
There is now a ping value inside the Network popup. > 50ms of lag is considered a weak connection, as it should be, because it's horribly noticeable at that point. That value is calculated from 60fps * 3
Stream idle migrates the old boolean value to a number | undefined type, in case there were any old users using it (doubtful, but I care about that sorta thing).
Stream idle now idles when the user gives no input for N amount of minutes.
Stream comes back online on any input from the user.
Window resizing now properly resizes the stream as well and it will unpause the stream.
Stream idle is a toggle with a slider because it would otherwise be hard for users to select 1 minute instead of 0 minutes. It's much easier to toggle it Off if the user wants 0, or have them slide all the way to the left to select 1 minute.
Stream.tsx is now EngineStream.tsx.
useSetupEngineManager is now gone.
A shit ton of the logic has now been migrated to a beautiful engineStreamMachine, which lives with the other top level actors.
Now that model-state-indicator-* is gone, tons of tests now use scene.settled instead of waitForExecutionDone which used that indicator.

It appears going from Projects view to Modeling scene is still a little buggy, I'm working on it as you all review.

vercel · 2025-02-07T22:05:00Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
modeling-app	✅ Ready (Inspect)	Visit Preview	Apr 7, 2025 9:47am

qa-wolf · 2025-02-07T22:05:01Z

QA Wolf here! As you write new code it's important that your test coverage is keeping up.
Click here to request test coverage for this PR!

codecov · 2025-02-07T22:38:27Z

Codecov Report

Attention: Patch coverage is 26.66667% with 11 lines in your changes missing coverage. Please review.

Project coverage is 85.09%. Comparing base (38446b5) to head (92a8610).
Report is 6 commits behind head on main.

Files with missing lines	Patch %	Lines
rust/kcl-lib/src/settings/types/mod.rs	26.66%	11 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5312      +/-   ##
==========================================
- Coverage   85.34%   85.09%   -0.26%     
==========================================
  Files         110      110              
  Lines       44248    44684     +436     
==========================================
+ Hits        37765    38024     +259     
- Misses       6483     6660     +177

Flag	Coverage Δ
rust	`85.09% <26.66%> (-0.26%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

pierremtb · 2025-02-21T16:06:11Z

@lf94 I'm hitting Update branch on this

lf94 · 2025-02-21T18:16:07Z

Heh I think I may have already rebased locally. Thanks though!

src/hooks/useEngineStreamContext.ts

src/lang/std/engineConnection.ts

codspeed-hq · 2025-03-17T19:33:08Z

CodSpeed Walltime Performance Report

Merging #5312 will degrade performances by 14.59%

_{Comparing lf94/stream-idle-revamp (92a8610) with main (be05dd7)}

⚠️

Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

Summary

⚡ 2 improvements
❌ 1 regressions
✅ 89 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

	Benchmark	`BASE`	`HEAD`	Change
⚡	`execute_color-cube`	1.6 s	1.4 s	+11%
❌	`execute_french-press`	3.8 s	4.4 s	-14.59%
⚡	`execute_multi-axis-robot`	12.8 s	11.2 s	+13.64%

franknoirot · 2025-03-21T17:56:19Z

Am I wrong or could like half the bugs and issues assigned to you be closed with this PR @lee-at-zoo-corp ?

jessfraz · 2025-03-21T17:57:52Z

i was just about to ask whats blocking us from merging :) we should probabluy make some playwright tests if we dont have some tho

Local MacOS and CI MacOS don't agree

pierremtb/stream-idle-revamp-appearance-fixes

* pierremtb/stream-idle-revamp-more-fixes * Fix lint

pierremtb

Two little things I noticed but nothing that can't be addressed in a subsequent PR. We need this big branch in before the week starts. Way to go!

pierremtb · 2025-04-07T10:59:32Z

e2e/playwright/testing-samples-loading.spec.ts

+        await page.waitForTimeout(1000)
        await commandOption.click()
+        await page.waitForTimeout(1000)
        await commandSampleOption(sampleTwo.title).click()
+        await page.waitForTimeout(1000)
        await commandMethodArgButton.click()
+        await page.waitForTimeout(1000)
        await commandMethodOption.click()
+        await page.waitForTimeout(1000)
        await expect(commandMethodArgButton).toContainText('overwrite')
        await expect(newFileWarning).not.toBeVisible()
        await expect(overwriteWarning).toBeVisible()
        await confirmButton.click()
+        await page.waitForTimeout(1000)


I'd prefer if these timeouts could be cleaned up but def not a blocker to merging

pierremtb · 2025-04-07T11:02:59Z

src/components/FileMachineProvider.tsx

-  // Only create the native file menus on desktop
-  useEffect(() => {
-    if (isDesktop()) {
-      window.electron.createModelingPageMenu().catch(reportRejection)
-    }
-  }, [])
-


Did this get moved somewhere else? We can always add back later, but wanted to leave a note to make sure

Yep, otherwise the native menu tests wouldve failed I believe

…ated changes (ping; scene indicator -> stream indicator, stream resizing (even on pause)) (#5312) * Add back stream idle mode * Shut up codespell * Correct serialization; only expose at user level * cargo fmt * tsc lint fmt * Move engineStreamMachine as a global actor; tons of more work * Fix up everything after bumping kittycad/lib * Remove camera sync * Use pause/play iconology * Add back better ping indicator * wip * Fix streamIdleMode checkbox being wonky * yarn fmt * Massive extinction event for waitForExecutionDone; try to stop projects view switching from crashing * Clear diagnostics when unmounting code editor! * wip * Rework initial root projects dir + deflake many projects tests * More e2e fixes * Deflake revolve some revolve tests * Fix the rest of the mfing tests * yarn fmt * yarn lint * yarn tsc * Fix tsc after rebase * wip * less flaky point and click * wip * Fixup after rebase * Fix more tests * Fix 2 more * Fix up named-views tests * yarn fmt lint tsc * Fix up new changes * Get rid of 1 cyclic dependency * Fix another cyclic mfer! * fmt * fmt tsc * Fix zoom to fit being frigged * a new list of circular deps * Remove NetworkHealthIndicator test that was shit * Fix the bad reload repeat issue kevin started on * Fix zoom to fit at the right moments... * Fix cache count numbers in editor test * Remove a test race - poll window info. * Qualify fail function * Try something * Use scene.connectionEstablished * Hopefully fix snapshots at least * Add app console.log * Fix native menu tests more * tsc lint * Fix camera failure * Try again * Test attempt number 15345203, action! * Add back old window detection heuristic * Remove firstWindow to complete the work of 2342d04 * Tweak some tests for MacOS * Tweak "set appearance" test for MacOS Revert this if it messes up any other platform's color checks! * Are you serious? This was all that needed formatting? * More color tweaks Local MacOS and CI MacOS don't agree * Fixes on apperance e2e test for stream idle branch (#6168) pierremtb/stream-idle-revamp-appearance-fixes * Another apperance fix * Skip one native menu test to make stream idle green (#6169) * pierremtb/stream-idle-revamp-more-fixes * Fix lint * Update snapshot for test_generate_settings_docs --------- Co-authored-by: lee-at-zoo-corp <lee@zoo.dev> Co-authored-by: Frank Noirot <frankjohnson1993@gmail.com> Co-authored-by: Pierre Jacquier <pierrejacquier39@gmail.com> Co-authored-by: Pierre Jacquier <pierre@zoo.dev>

* origin/main: Quick app rename typo fix in settings.md (#6198) Add point-and-click Insert from local project files (#6129) Install and start Vector on macOS CI runners (#6147) Implement polar std function in KCL (#6180) Bump typescript from 5.8.2 to 5.8.3 in /packages/codemirror-lsp-client in the patch group (#6188) Bump @types/node from 22.13.13 to 22.14.0 in /packages/codemirror-lsp-client in the minor group (#6189) Bump the major group in /packages/codemirror-lang-kcl with 2 updates (#6194) Bump taiki-e/install-action from 2.49.30 to 2.49.45 in the patch group (#6185) Bump the patch group with 6 updates (#6186) Bump the patch group in /rust/kcl-language-server with 3 updates (#6183) Bump the patch group in /packages/codemirror-lang-kcl with 2 updates (#6193) Remove unnecessary timeouts waiting for command bar (#6199) Stream handling / Stream idle mode v2; a ton of network related changes (ping; scene indicator -> stream indicator, stream resizing (even on pause)) (#5312) More propagation of numeric types (#6177) Apply type-directed coercions to arguments in calls of user functions (#6179) Erase comment start positions from snapshot tests (#6178) Implement coercion of numeric types for ascription and arithmetic (off by default) (#6175) Reduce the number of reps in the add_lots test (#6174) take things off the batch in a more safe way (#6171)

Fixes #6472. Just a missing setting moved in #5312, which wasn't caught because testing that the orbit maneuver is actually performing a "trackball-like" movement is nonexistent. I don't know how to test that reliably, but typing this object provides the red squiggles to reveal the missing property.

* Fix trackball camera by weaving through EngineStream Fixes #6472. Just a missing setting moved in #5312, which wasn't caught because testing that the orbit maneuver is actually performing a "trackball-like" movement is nonexistent. I don't know how to test that reliably, but typing this object provides the red squiggles to reveal the missing property. * Update src/components/EngineStream.tsx --------- Co-authored-by: Kurt Hutten <k.hutten@protonmail.ch>

lf94 requested review from Irev-Dev, franknoirot, nadr0 and pierremtb February 7, 2025 22:04

vercel bot deployed to Preview February 7, 2025 22:30 View deployment

lf94 force-pushed the lf94/stream-idle-revamp branch from 41e7503 to 2ef660e Compare February 10, 2025 21:18

vercel bot deployed to Preview February 10, 2025 21:31 View deployment

lf94 force-pushed the lf94/stream-idle-revamp branch from 2ef660e to 324b409 Compare February 11, 2025 16:44

vercel bot deployed to Preview February 11, 2025 17:10 View deployment

vercel bot deployed to Preview February 21, 2025 16:22 View deployment

lf94 force-pushed the lf94/stream-idle-revamp branch from b5c3f0c to b00aaf2 Compare February 25, 2025 19:13

vercel bot deployed to Preview February 25, 2025 19:29 View deployment

lf94 force-pushed the lf94/stream-idle-revamp branch from b00aaf2 to eb582a6 Compare February 26, 2025 20:53

vercel bot had a problem deploying to Preview February 26, 2025 20:53 Failure

jgomez720 mentioned this pull request Mar 6, 2025

Not rendering model after idle #5664

Closed

vercel bot had a problem deploying to Preview March 17, 2025 17:32 Failure

jessfraz reviewed Mar 17, 2025

View reviewed changes

src/hooks/useEngineStreamContext.ts Outdated Show resolved Hide resolved

jessfraz reviewed Mar 17, 2025

View reviewed changes

src/lang/std/engineConnection.ts Outdated Show resolved Hide resolved

vercel bot deployed to Preview March 17, 2025 18:55 View deployment

lee-at-zoo-corp force-pushed the lf94/stream-idle-revamp branch from 7a61452 to 91a3a27 Compare March 17, 2025 21:11

vercel bot had a problem deploying to Preview March 17, 2025 21:25 Failure

vercel bot deployed to Preview March 20, 2025 20:52 View deployment

vercel bot deployed to Preview April 5, 2025 01:47 View deployment

More color tweaks

2c35d19

Local MacOS and CI MacOS don't agree

vercel bot deployed to Preview April 5, 2025 02:58 View deployment

Merge branch 'main' into lf94/stream-idle-revamp

5e01dc3

vercel bot deployed to Preview April 5, 2025 15:45 View deployment

Merge branch 'main' into lf94/stream-idle-revamp

27bd553

vercel bot deployed to Preview April 5, 2025 16:00 View deployment

Fixes on apperance e2e test for stream idle branch (#6168)

306eca8

pierremtb/stream-idle-revamp-appearance-fixes

vercel bot deployed to Preview April 5, 2025 17:29 View deployment

Another apperance fix

1ef2cea

vercel bot deployed to Preview April 5, 2025 19:01 View deployment

Skip one native menu test to make stream idle green (#6169)

ba874ed

* pierremtb/stream-idle-revamp-more-fixes * Fix lint

vercel bot deployed to Preview April 5, 2025 21:55 View deployment

Merge branch 'main' into lf94/stream-idle-revamp

3078662

vercel bot deployed to Preview April 6, 2025 12:45 View deployment

Update snapshot for test_generate_settings_docs

92a8610

vercel bot deployed to Preview April 7, 2025 09:47 View deployment

pierremtb approved these changes Apr 7, 2025

View reviewed changes

pierremtb merged commit df6f571 into main Apr 7, 2025
57 of 59 checks passed

pierremtb deleted the lf94/stream-idle-revamp branch April 7, 2025 11:08

franknoirot mentioned this pull request Apr 11, 2025

Don't bail command palette if there's no engineConnection #6286

Merged

franknoirot mentioned this pull request May 8, 2025

Fix trackball camera by weaving through EngineStream #6760

Merged

Conversation

lf94 commented Feb 7, 2025 • edited by jessfraz Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vercel bot commented Feb 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qa-wolf bot commented Feb 7, 2025

Uh oh!

codecov bot commented Feb 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

pierremtb commented Feb 21, 2025

Uh oh!

lf94 commented Feb 21, 2025

Uh oh!

Uh oh!

Uh oh!

codspeed-hq bot commented Mar 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CodSpeed Walltime Performance Report

Merging #5312 will degrade performances by 14.59%

Summary

Benchmarks breakdown

Uh oh!

franknoirot commented Mar 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jessfraz commented Mar 21, 2025

Uh oh!

pierremtb left a comment

Choose a reason for hiding this comment

Uh oh!

pierremtb Apr 7, 2025

Choose a reason for hiding this comment

Uh oh!

pierremtb Apr 7, 2025

Choose a reason for hiding this comment

Uh oh!

lee-at-zoo-corp Apr 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

lf94 commented Feb 7, 2025 •

edited by jessfraz

Loading

vercel bot commented Feb 7, 2025 •

edited

Loading

codecov bot commented Feb 7, 2025 •

edited

Loading

codspeed-hq bot commented Mar 17, 2025 •

edited

Loading

franknoirot commented Mar 21, 2025 •

edited

Loading