-
Notifications
You must be signed in to change notification settings - Fork 14
feat: demo add synth events #332
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
base: feat-demo-final-deploy-fixes
Are you sure you want to change the base?
Conversation
|
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx affected -t lint typecheck test --parallel -... |
❌ Failed | 15s | View ↗ |
☁️ Nx Cloud last updated this comment at 2025-11-11 14:45:40 UTC
🔍 Preview Deployment: Website✅ Deployment successful! 🔗 Preview URL: https://pr-332.pgflow.pages.dev 📝 Details:
_Last updated: _ |
🔍 Preview Deployment: Demo✅ Deployment successful! 🔗 Preview URL: https://pr-332-pgflow-demo.jumski.workers.dev 📝 Details:
_Last updated: _ |
dd7cb02 to
63d7289
Compare
| stateVersion; | ||
| return run?.step(stepSlug).status || 'created'; | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical bug: Optional chaining is incomplete. When run is null, run?.step(stepSlug) returns undefined, then accessing .status on undefined will throw a TypeError.
// Current (broken):
return run?.step(stepSlug).status || 'created';
// Fixed:
return run?.step(stepSlug)?.status || 'created';This affects all property getters in the step() function (status, output, error, started_at, completed_at, failed_at) and will crash whenever these properties are accessed before a run is started or after it's cleared.
| stateVersion; | |
| return run?.step(stepSlug).status || 'created'; | |
| }, | |
| stateVersion; | |
| return run?.step(stepSlug)?.status || 'created'; | |
| }, |
Spotted by Graphite Agent
Is this helpful? React 👍 or 👎 to let us know.
| // Ensure reactivity to step status changes by accessing flowState.run | ||
| void flowState.run; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace 'void flowState.run' with a proper
Spotted by Graphite Agent (based on CI logs)
Is this helpful? React 👍 or 👎 to let us know.
| let lastEventCount = $state(0); | ||
| $effect(() => { | ||
| const currentEventCount = flowState.events.length; | ||
| const currentEventCount = flowState.timeline.length; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure the timeline implementation uses SvelteDate instead of the built-in Date class for reactivity. If the timeline is created in pgflow-state.svelte, make sure all Date instances are replaced with SvelteDate there.
Spotted by Graphite Agent (based on CI logs)
Is this helpful? React 👍 or 👎 to let us know.
63d7289 to
7d9e3a0
Compare
4069c1b to
00a42ab
Compare
| // Use reactive step() method from runState | ||
| return flowState.step(stepSlug).status === 'started'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment here is redundant since the code already clearly shows we're using the step() method. Consider removing the comment to make the code cleaner.
Spotted by Graphite Agent (based on CI logs)
Is this helpful? React 👍 or 👎 to let us know.
7d9e3a0 to
84196b4
Compare
00a42ab to
5e1c8fa
Compare
84196b4 to
7d29fe5
Compare
5e1c8fa to
259e682
Compare
7d29fe5 to
96d0289
Compare

No description provided.