Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix so that all artifact commands are returned regardless of caching #5005

Merged
merged 2 commits into from
Jan 11, 2025

Conversation

jtran
Copy link
Collaborator

@jtran jtran commented Jan 11, 2025

Fixes #4991, a problem introduced in #4912.

If the KCL program is extended, i.e. new statements are added to the end, the execution cache detects this, and only runs new KCL statements. The problem is that artifact commands are only generated for the new KCL statements, not the full KCL program.

Luckily, we save everything in the cache. So this PR simply extends the cached artifact commands with the new commands.

Copy link

qa-wolf bot commented Jan 11, 2025

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!

Copy link

vercel bot commented Jan 11, 2025

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

Name Status Preview Updated (UTC)
modeling-app ✅ Ready (Inspect) Visit Preview Jan 11, 2025 1:00am

Copy link

codecov bot commented Jan 11, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.87%. Comparing base (2b0ba37) to head (ffda4ed).
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5005      +/-   ##
==========================================
+ Coverage   85.84%   85.87%   +0.02%     
==========================================
  Files          88       88              
  Lines       31432    31435       +3     
==========================================
+ Hits        26984    26995      +11     
+ Misses       4448     4440       -8     
Flag Coverage Δ
wasm-lib 85.87% <100.00%> (+0.02%) ⬆️

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.

@jtran jtran enabled auto-merge (squash) January 11, 2025 02:54
Copy link
Collaborator

@franknoirot franknoirot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed with user testing that this fixes the issue, great find and fix!

exec_state
.global
.artifact_commands
.extend(self.engine.take_artifact_commands());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the key part yeah? Extending the old artifact_commands with the new ones?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. This is the one line I needed.

@jtran jtran merged commit 013cb10 into main Jan 11, 2025
33 checks passed
@jtran jtran deleted the jtran/fix-missing-artifact-commands branch January 11, 2025 03:33
Copy link
Collaborator

@pierremtb pierremtb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed, fixed it for me ty!

franknoirot pushed a commit that referenced this pull request Jan 15, 2025
…5005)

* Fix so that all artifact commands are returned regardless of caching

* Add some more docs and fix up old ones
franknoirot added a commit that referenced this pull request Jan 24, 2025
* Rename `homeMachine` and accessories to `projectsMachine`

* Separate out `/home` route from `projectsMachine`

* Add logic to navigate out from deleted or renamed project

* Show a warning in the command palette for deleting a project

* Make it navigate when you create a project

* Update "New project" button to use command bar flow
Closes #2585

* More explicit warning message text

* Make projects watching code not run in web

* Tests first version: nested loops

* Tests second version: flattened

* Remove console logs

* Fix tsc

* @jtran feedback, use the type guard util

* A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu-latest)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: windows-latest)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: windows-latest)

* Fix tests that relied on one-click, no-navigation project creation

* Revert "A snapshot a day keeps the bugs away! 📷🐛 (OS: windows-latest)"

This reverts commit 7545b61.

* Revert "A snapshot a day keeps the bugs away! 📷🐛 (OS: windows-latest)"

This reverts commit 3d2e487.

* Add a mask to the state indicator to client-side scale test

* A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu-latest)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: windows-latest)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu-latest)

* Fix lint

* Fix tsc

* Add menu item to share link to file

* Forward query params while redirecting to /home or /file

* Add (broken) event logic and command triggering logic

* Fix a couple stray tests that still relied on the old way of creating projects

* De-flake another text that could be thrown off by toast-based selectors

* FMT

* Dumb test error because I was rushing

* A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu-latest)

* Ahhh more flaky toasts, they're everywhere!

* A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu-latest)

* Side quest: Only register commands once, power their disabled status while selecting commands via optional actor

* Get query-triggered command working in browser too

* A snapshot a day keeps the bugs away! 📷🐛 (OS: windows-latest)

* Tests always run on localhost, don't expect the prod origin

* rerun CI

* wip

* wip

* Everything's pretty much done but url.zoo.dev has been broken and we need to think about how to test reliably

* Merge branch 'main' into franknoirot/4088/create-file-url

* Add useCreateFileLinkQuery on Home page

* Get primary user flow working on desktop

* Rework to open browser app first, then send along to the desktop app if asked

* Styling updates to OpenInDesktopAppHandler

* Clean up unecessary file

* Merge branch 'main' into franknoirot/4088/create-file-url

* Separate creating `createFileUrl` and shortlink so it is unit testable

* Add E2E test for importing file from URL

* Add a couple component tests for OpenInDesktopAppHandler

* Fix the "existing project" user flow

* Add E2E test for "add to existing project" user flow

* Undo mistaken or unecessary changes

* Lints, fmt, tsc

* Fix unit test

* Fix broken rename and delete project commands
Something about the `optionsFromContext` config no longer works with file I/O-related commands. I suspect this has to do with our read/write loop patching

* Fix unit test, use kebab-case for url query param

* Use dev urls everywhere when configured that way

I think we were just using some constants that ended up returning bad
values for dev, it seemed to return a working shortlink when I went
through the flow.

* Clean up unneeded PROD_TOKEN

* Fix browser command flow, because we had made the projectMachine desktop-only on main

* Make the test executor a bit more patient (#5004)

* Fix so that all artifact commands are returned regardless of caching (#5005)

* Fix so that all artifact commands are returned regardless of caching

* Add some more docs and fix up old ones

* Add new lint to disallow use of confusing isNaN (#4999)

* Point-and-click Sweep (first PR) (#4989)

* Refactor 'Delete selection' as actor
Will fix #4662

* WIP logging

* WIP: working Solid3dGetExtrusionFaceInfo for loft

* Working wall deletion of loft

* Add offset plane deletion

* Add feature tree deletion of shell

* Clean up

* Revert "Clean up"

This reverts commit 214763c.

* Clean up rust changes, taking the sketch with the most paths

* Working cap selection and deletion

* Clean up

* Add test for loft and offset plane deletion via selection

* A snapshot a day keeps the bugs away! 📷🐛 (OS: windows-16-cores)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-macos-8-cores)

* Set reenter: false as it was originally

* Passing test

* Add shell deletion via feature tree test

* Revert the migration to promise actor

* A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-ubuntu-8-cores)

* Trigger CI

* A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-ubuntu-8-cores)

* Trigger CI

* A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-ubuntu-8-cores)

* Trigger CI

* A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-ubuntu-8-cores)

* Trigger CI

* Use cmd.id as solid_id after latest engine merge

* Add feature tree deletion of offset plane and fix lint

* Add feature tree deletion of loft

* Clean up

* Better comment

* Lint fix

* Remove sketch sorting

* WIP: sweep point-and-click

* Working sweep

* Add test

* Make sweep a development command

* Fix tsc error

* Clean up for review

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* Upgrade typescript-eslint from 5.62.0 to 8.19.1 and remove eslint-config-react-app (#5006)

* Fix lost lints and add new ones (#5011)

* Add eslint-plugin-jsx-a11y dependency

* Add jsx-a11y lint

* Add eslint-plugin-react-hooks dependency

* Add react hooks lints

* Ignore new react hooks lint in tests

* Add eslint-plugin-testing-library dependency

* Add testing-library lint

* Fix yarn lint to use all files recursively

* Developer workflow: added auto generated workspace file from vitest extension in vscode (#4997)

* chore: added auto generated workspace file from vitest extension in vscode

* fix: auto fmt fixes

* Change Dependabot PRs to always be made on Mondays (#5025)

* Add packages to Dependabot updates (#5024)

* Bump @lezer/generator from 1.7.1 to 1.7.2 (#5018)

Bumps [@lezer/generator](https://github.com/lezer-parser/generator) from 1.7.1 to 1.7.2.
- [Changelog](https://github.com/lezer-parser/generator/blob/main/CHANGELOG.md)
- [Commits](lezer-parser/generator@1.7.1...1.7.2)

---
updated-dependencies:
- dependency-name: "@lezer/generator"
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump handlebars from 6.2.0 to 6.3.0 in /src/wasm-lib (#5012)

Bumps [handlebars](https://github.com/sunng87/handlebars-rust) from 6.2.0 to 6.3.0.
- [Release notes](https://github.com/sunng87/handlebars-rust/releases)
- [Changelog](https://github.com/sunng87/handlebars-rust/blob/master/CHANGELOG.md)
- [Commits](sunng87/handlebars-rust@v6.2.0...v6.3.0)

---
updated-dependencies:
- dependency-name: handlebars
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump syn from 2.0.95 to 2.0.96 in /src/wasm-lib (#5015)

Bumps [syn](https://github.com/dtolnay/syn) from 2.0.95 to 2.0.96.
- [Release notes](https://github.com/dtolnay/syn/releases)
- [Commits](dtolnay/syn@2.0.95...2.0.96)

---
updated-dependencies:
- dependency-name: syn
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Fix artifact types to be more accurate (#5022)

* Fix Cargo.lock to not have changes (#5034)

* Upgrade all wasm-bindgen dependencies together (#5037)

* Disable auto-updater on non-versioned builds (#5042)

* turns on helix from edge (#5036)

* updates for new lib

Signed-off-by: Jess Frazelle <[email protected]>

* autocomplete

Signed-off-by: Jess Frazelle <[email protected]>

* bump version

Signed-off-by: Jess Frazelle <[email protected]>

* bump all the things

Signed-off-by: Jess Frazelle <[email protected]>

* new samples

Signed-off-by: Jess Frazelle <[email protected]>

* docs

Signed-off-by: Jess Frazelle <[email protected]>

---------

Signed-off-by: Jess Frazelle <[email protected]>

* ci: Add yarn test of packages/codemirror-lang-kcl (#5035)

* ci: Add yarn test of packages/codemirror-lang-kcl

* Fix CI error running tests

* Fix postcss config error

* Bump xstate from 5.17.4 to 5.19.2 (#5027)

* Hook up chamfer UI with AST-mod (#4694)

* button

* config

* hook up with ast

* cmd bar test

* button states fix and test

* little naming fix

* xState action to actor

* remove button state test updates

* fixture-based approach

* nightly

Co-authored-by: Pierre Jacquier <[email protected]>

* Update src/lib/toolbar.ts

Co-authored-by: Frank Noirot <[email protected]>

---------

Co-authored-by: Pierre Jacquier <[email protected]>
Co-authored-by: Frank Noirot <[email protected]>

* Remove Redundant Fillet Button State Test (#5009)

delete obsolete test

* Bump @types/node from 20.14.9 to 22.10.6 in /packages/codemirror-lsp-client (#5041)

* custom axis and origin example for helix (#5057)

* custom axis and origin for helix

Signed-off-by: Jess Frazelle <[email protected]>

* A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-ubuntu-8-cores)

* empty

---------

Signed-off-by: Jess Frazelle <[email protected]>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* Bump typescript from 5.7.2 to 5.7.3 (#5021)

Bumps [typescript](https://github.com/microsoft/TypeScript) from 5.7.2 to 5.7.3.
- [Release notes](https://github.com/microsoft/TypeScript/releases)
- [Changelog](https://github.com/microsoft/TypeScript/blob/main/azure-pipelines.release.yml)
- [Commits](microsoft/TypeScript@v5.7.2...v5.7.3)

---
updated-dependencies:
- dependency-name: typescript
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Refactor: break out `copyFileShareLink` into standalone function

* Add "Share file" to command palette

* Update dumb use of site URL instead of prod app URL

* fmt

* @lf94 nit

* @pierremtb spinner feedback

* Hide share link command and disable menu item for now

* Just comment out the command config for now

---------

Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: Jess Frazelle <[email protected]>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: 49lf <[email protected]>
Co-authored-by: Adam Sunderland <[email protected]>
Co-authored-by: Adam Sunderland <[email protected]>
Co-authored-by: Jonathan Tran <[email protected]>
Co-authored-by: Pierre Jacquier <[email protected]>
Co-authored-by: Kevin Nadro <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Jess Frazelle <[email protected]>
Co-authored-by: max <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Scene selections aren't working properly after Extrude or Loft
3 participants