ci(experiment): don't check in generated TS files - #255
Conversation
80d5cfa to
f0f3691
Compare
|
for posterity: I need to make sure that the @quent packages are able to re-export the generated types correctly when built, and also add some "pre-dev" scripts to make sure types are generated so IDEs have types and typecheck works seamlessly locally. |
# Description Enables the ts-rs formatting feature for simulator server bindings. Regenerates the bindings without trailing whitespace that can accidentally be removed when committing and ensures repeated builds produce stable output. Written by Codex. # Related issues #255 Authors: - Johan Peltenburg (https://github.com/johanpel) Approvers: - Matthijs Brobbel (https://github.com/mbrobbel) URL: #421
Remove the 52 generated TypeScript definitions from examples/simulator/server/ts-bindings/ from git tracking. They are regenerated on each CI run by building quent-simulator-server and uploaded as an artifact. Two new jobs in ui.yml: - generate-bindings: builds quent-simulator-server and uploads the generated ts-bindings/ as a CI artifact; all UI jobs that need the bindings download it before running. - diff-bindings: on pull requests, diffs the PR bindings against the base branch and posts the result to the step summary. The full bindings are also shown after the diff. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Removes bare `corepack enable pnpm` + `pnpm` calls that pulled in pnpm@11.9.0 via corepack, conflicting with the pinned pnpm@11.6.0. Also fixes the prettier config and write paths for `pnpm --dir ui exec` (which sets CWD to ui/). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
#393 merged generated ts bindings that weren't formatted using the changes of #421, this fixes them. cc. @johallar @cmatzenbach could we please consider #255 to avoid these issues altogether? Authors: - Johan Peltenburg (https://github.com/johanpel) Approvers: - Matthijs Brobbel (https://github.com/mbrobbel) - Dhruv Vats (https://github.com/dhruv9vats) URL: #424
…ed-checkin Signed-off-by: Johan Peltenburg <johan.peltenburg+code@gmail.com> # Conflicts: # examples/simulator/server/ts-bindings/CategoricalDecl.ts # examples/simulator/server/ts-bindings/CategoricalSeries.ts # examples/simulator/server/ts-bindings/CategoricalTimelineRequest.ts # examples/simulator/server/ts-bindings/DataFlowTimelineBinned.ts # examples/simulator/server/ts-bindings/DimensionKeyDecl.ts # examples/simulator/server/ts-bindings/MeasureDecl.ts
Signed-off-by: Johan Peltenburg <johan.peltenburg+code@gmail.com>
Signed-off-by: Johan Peltenburg <johan.peltenburg+code@gmail.com>
036f8df to
d583f39
Compare
|
Added some commits to make the bindings compile automatically as a part of First bindings take some time to compile, but without Rust changes it's fast now: |
Signed-off-by: Johan Peltenburg <johan.peltenburg+code@gmail.com>
Signed-off-by: Johan Peltenburg <johan.peltenburg+code@gmail.com>
📝 WalkthroughWalkthroughThe PR introduces a reusable Rust TypeScript-binding generator, moves UI consumers to shared generated bindings, adds binding-aware build scripts and Cargo integration, and extends CI to generate, compare, scan, and report binding changes. ChangesTypeScript binding generation pipeline
Estimated code review effort: 3 (Moderate) | ~30 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
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 @.github/workflows/license-check.yml:
- Line 26: Update the license-check workflow to generate the simulator
TypeScript bindings before collecting files, then reuse one shared file list
containing both tracked sources and generated bindings for license validation
and the reported count. Replace the index-only git ls-files input in the
relevant steps while preserving the existing extensions and output behavior.
In `@examples/simulator/server/build.rs`:
- Around line 20-55: Add tests for the sync_bindings function covering copying
generated files, avoiding writes when destination content is unchanged, and
removing stale .ts files while preserving current outputs. Use temporary
directories and assert both file contents and stale-file deletion behavior.
🪄 Autofix (Beta)
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: Path: .coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: e8adb2a8-ca9d-4031-92e8-3bde0f445a95
⛔ Files ignored due to path filters (68)
examples/simulator/server/ts-bindings/BinnedSpanSec.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/BulkTimelineRequest.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/BulkTimelinesResponse.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/BulkTimelinesResponseEntry.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/CapacityDecl.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/CapacityKind.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/CategoricalDecl.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/CategoricalSeries.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/CategoricalTimelineRequest.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/DataFlowTimelineBinned.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/DimensionKeyDecl.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/DynamicAttribute.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/DynamicList.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/DynamicStruct.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/DynamicValue.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/Edge.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/Engine.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/EngineImplementationAttributes.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/EntityFilter.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/EntityListEntry.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/EntityListFilter.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/EntityListRequest.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/EntityListResponse.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/EntityRef.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/EntityScope.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/EntitySortKey.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/FiniteStateMachine.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/FsmStateTypeDecl.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/FsmTransition.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/FsmTransitionDecl.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/FsmTypeDecl.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/FsmUsage.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/MeasureDecl.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/Operator.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/OperatorFilter.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/OperatorStatistics.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/PageParams.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/Plan.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/PlanTree.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/Port.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/PortStatistics.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/PrefixSystem.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/QuantitySpec.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/Query.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/QueryBundle.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/QueryEntities.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/QueryFilter.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/QueryGroup.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/Resource.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/ResourceGroup.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/ResourceGroupNode.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/ResourceGroupTimelineRequest.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/ResourceGroupTypeDecl.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/ResourceTimeline.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/ResourceTimelineBinned.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/ResourceTimelineBinnedByState.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/ResourceTimelineRequest.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/ResourceTree.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/ResourceTypeDecl.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/SingleTimelineRequest.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/SingleTimelineResponse.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/Sort.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/SortDir.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/SpanSec.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/TimeWindow.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/TimelineConfig.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/TimelineRequest.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/Worker.tsis excluded by!examples/simulator/server/ts-bindings/**
📒 Files selected for processing (13)
.github/workflows/license-check.yml.github/workflows/rust.yml.github/workflows/ui.yml.gitignoreREADME.mddomains/query_engine/server/build.rsexamples/simulator/server/build.rsui/README.mdui/package.jsonui/packages/@quent/client/package.jsonui/packages/@quent/components/package.jsonui/packages/@quent/hooks/package.jsonui/packages/@quent/utils/package.json
This reverts commit 5cb5c3e.
|
Tested this in commit 5cb5c3e |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
examples/simulator/ui-bindings/src/lib.rs (1)
22-41: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winAdd coverage for the binding generator.
Test generation into a temporary directory, including expected output creation and stale-file removal. This is the reusable contract that all UI build paths depend on.
As per coding guidelines, “New Rust components must include accompanying tests.”
🤖 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 `@examples/simulator/ui-bindings/src/lib.rs` around lines 22 - 41, Add tests for the generate function that use a temporary directory, verify the expected bindings are created, and confirm stale files are removed when generation runs. Keep the test focused on the public generate contract and ensure temporary resources are cleaned up automatically.Source: Coding guidelines
🧹 Nitpick comments (1)
ui/packages/@quent/client/package.json (1)
12-15: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winSerialize binding generation for recursive package commands.
Each package independently runs the same root generator. Recursive pnpm commands execute across workspace projects with concurrency, causing redundant Cargo invocations and potentially concurrent writes to
ui/generated/ts-bindings. Centralize generation for recursive flows or add serialization/idempotence.
ui/packages/@quent/client/package.json#L12-L15: adjustprebuild/pretypecheck.ui/packages/@quent/components/package.json#L15-L18: adjustprebuild/pretypecheck.ui/packages/@quent/hooks/package.json#L13-L16: adjustprebuild/pretypecheck.ui/packages/@quent/utils/package.json#L12-L15: adjustprebuild/pretypecheck.(pnpm.io)
🤖 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 `@ui/packages/`@quent/client/package.json around lines 12 - 15, Serialize binding generation for recursive workspace commands by updating the prebuild and pretypecheck scripts in ui/packages/@quent/client/package.json (12-15), ui/packages/@quent/components/package.json (15-18), ui/packages/@quent/hooks/package.json (13-16), and ui/packages/@quent/utils/package.json (12-15). Centralize generation or otherwise make these shared bindings commands serialized and idempotent, while preserving the existing build and typecheck flows.
🤖 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.
Outside diff comments:
In `@examples/simulator/ui-bindings/src/lib.rs`:
- Around line 22-41: Add tests for the generate function that use a temporary
directory, verify the expected bindings are created, and confirm stale files are
removed when generation runs. Keep the test focused on the public generate
contract and ensure temporary resources are cleaned up automatically.
---
Nitpick comments:
In `@ui/packages/`@quent/client/package.json:
- Around line 12-15: Serialize binding generation for recursive workspace
commands by updating the prebuild and pretypecheck scripts in
ui/packages/@quent/client/package.json (12-15),
ui/packages/@quent/components/package.json (15-18),
ui/packages/@quent/hooks/package.json (13-16), and
ui/packages/@quent/utils/package.json (12-15). Centralize generation or
otherwise make these shared bindings commands serialized and idempotent, while
preserving the existing build and typecheck flows.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: 13d31af5-ae61-45b5-912f-1b83fe06066f
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock,!Cargo.lock
📒 Files selected for processing (22)
.coderabbit.yaml.github/workflows/ui.yml.gitignoreCargo.tomlREADME.mddomains/query_engine/server/Cargo.tomldomains/query_engine/server/build.rsexamples/simulator/server/Cargo.tomlexamples/simulator/ui-bindings/Cargo.tomlexamples/simulator/ui-bindings/src/lib.rsexamples/simulator/ui-bindings/src/main.rsui/README.mdui/eslint.config.jsui/package.jsonui/packages/@quent/client/package.jsonui/packages/@quent/components/package.jsonui/packages/@quent/hooks/package.jsonui/packages/@quent/utils/package.jsonui/packages/@quent/utils/src/types/index.tsui/packages/@quent/utils/tsconfig.jsonui/tsconfig.jsonui/vite.config.ts
💤 Files with no reviewable changes (1)
- examples/simulator/server/Cargo.toml
|
/merge |

Description
Stop checking generated TypeScript bindings into the repository.
Bindings are generated into
ui/generated/ts-bindingsthrough the dedicatedquent-simulator-ui-bindingspackage. This keeps each workflow self-contained:pnpm build,pnpm typecheck, and development scripts generate bindings automatically.uifeature generate bindings before building and embedding the UI.uifeature perform no binding or frontend work.UI CI jobs now generate their own inputs instead of sharing a generated-bindings artifact.
Clean-state standalone Cargo and pnpm builds were verified with no generated bindings present. Consecutive embedded Cargo builds were also verified to avoid recompilation.
Written by Codex.
Example
The best way I could come up with surfacing whether TS bindings were changed was to make this check not skip (it will show skipped if there were no changes):
And then if you go to the UI summary tab, you can inspect them:
Any review comments based on those changes will still have to go to the Rust source of these bindings of course (unless we bump packages and the generators change).