GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Improve UX of rapidly changing transaction IDs and commit progress by Karakatiza666 · Pull Request #7742 · feldera/feldera

Conversation

@Karakatiza666

@Karakatiza666 Karakatiza666 commented Oct 30, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor
  • The commit bar shows only for commits of 6 s or more.
  • The transaction ID shows only while a transaction runs.
  • The indicator keeps a constant height when the bar or the ID hides.
Screencast.From.2026-10-30.18-25-32.webm

Checklist

  • Unit tests added/updated
  • Integration tests added/updated
  • Documentation updated
  • Changelog updated

Breaking Changes?

Mark if you think the answer is yes for any of these components:

  • OpenAPI / REST HTTP API / feldera-types / manager (What is a breaking change?)
  • Feldera SQL (Syntax, Semantics)
  • feldera-sqllib (incl. dependencies fxp, etc.) (What is a breaking change?)
  • Python SDK (What is a breaking change?)
  • fda (CLI arguments)
  • Adapters (including configuration)
  • Storage Format / Checkpoints
  • Others (specify)

Describe Incompatible Changes

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@Karakatiza666
[web-console] Improve UX of rapidly changing transaction IDs and comm…
1b45c1f
…it progress

- The commit bar shows only for commits of 6 s or more, with a slide transition.
- The transaction ID shows only while a transaction runs.
- The indicator keeps a constant height when the bar or the ID hides.

Signed-off-by: Karakatiza666 <[email protected]>
@Karakatiza666
Karakatiza666 requested a review from mihaibudiu October 30, 2026 15:57
mythical-fred-oss[bot]
mythical-fred-oss Bot reviewed Oct 30, 2026
View reviewed changes
Comment thread ...ackages/web-console/src/lib/components/pipelines/editor/performance/CommitProgressRow.svelte Outdated
not, without either row stretching to fill a line it has to itself. -->
<div
class="items-top flex max-w-150 basis-150 flex-wrap gap-x-4 gap-y-2 {_class}"
class="items-top flex max-w-150 basis-150 flex-wrap gap-x-4 gap-y-2 h-16 {_class}"

@mythical-fred-oss mythical-fred-oss Bot Oct 30, 2026

Copy link
Copy Markdown
Contributor

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.

bug (medium): the fixed h-16 breaks the layout below sm. There, the label column is w-full and the bar column wraps under it. I rendered a long commit in a 390px viewport: the row's clientHeight was 64px and its scrollHeight was 112px, so the content overflows into whatever sits below the row. Use sm:h-16, or a min-h-16, to keep the constant height on desktop only. The old mobile handling (useIsScreenSm) was removed, but its import is still there.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
mythical-fred-oss[bot]
mythical-fred-oss Bot reviewed Oct 30, 2026
View reviewed changes
Comment thread js-packages/web-console/scripts/playwright-snapshots.ts Outdated
async function push() {
const workDir = mkdtempSync(path.join(tmpdir(), 'playwright-snapshots-'))
await $`git clone -q --depth 1 [email protected]:${REPO}.git ${workDir}`
await $`git -C ${workDir} rm -r -q --ignore-unmatch .`

@mythical-fred-oss mythical-fred-oss Bot Oct 30, 2026

Copy link
Copy Markdown
Contributor

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.

pitfall (high): push replaces the whole tree of the snapshots repo with the local copy and pushes it to main. Suppose two PRs update screenshots at the same time. Each pins a commit that lacks the other PR's new screenshots, so whichever PR merges second breaks the other's screenshot tests on main. A fix could rebase onto the snapshots repo's current main and copy only the changed files (the manifest already lists them via listChangedFiles). At least, document this in the README.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
mythical-fred-oss[bot]
mythical-fred-oss Bot reviewed Oct 30, 2026
View reviewed changes
Comment thread js-packages/web-console/package.json
"test-prepare": "bun scripts/playwright-snapshots.ts pull",
"test-snapshots-push": "bun scripts/playwright-snapshots.ts push",
"test-update-snapshots": "bun run deps && bun run test-prepare && vitest --run --project client --update && DISPLAY= bun playwright test --update-snapshots",
"test-unit": "bun run deps && bun run test-prepare && vitest --run --project client --project server",

@mythical-fred-oss mythical-fred-oss Bot Oct 30, 2026

Copy link
Copy Markdown
Contributor

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.

pitfall (medium): test-unit (and test-e2e) now has to download from codeload.github.com. If that download fails, the run fails. The old git clone … || true did not fail. A fresh CI checkout downloads on every run. Also, the commit message only covers the transaction UX, but this PR also changes the snapshot workflow (pinned commit, pull/push script, viewport, ui: false). Please split that into its own commit or PR, and explain why in the commit message.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
mythical-fred-oss[bot]
mythical-fred-oss Bot reviewed Oct 30, 2026
View reviewed changes
Comment thread ...nsole/src/lib/components/pipelines/editor/performance/CommitProgressIndicator.svelte.spec.ts
/** Waits until the bars have moved to the latest poll. */
const settleBars = () => sleep(2 * PERIOD_MS + 100)
// The browser is slow while the other test files run. Thus, the screenshot can take longer.
const screenshotOptions = { timeout: 15_000 }

@mythical-fred-oss mythical-fred-oss Bot Oct 30, 2026

Copy link
Copy Markdown
Contributor

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.

flaky (medium): the screenshot tests rely on wall-clock time: settleBars() sleeps, and the comment says the browser is slow under load. I ran bun run test-unit with the default worker count. shows an idle transaction row timed out (locator.screenshot: Timeout 5000ms exceeded), and three unrelated screenshot tests failed too. With VITEST_MAX_WORKERS=4, as CI uses, all tests passed. The sleep(600) waits in the height test and the real-timer ID count test are similar. A scheduler prop (ManualScheduler) could drive the animations deterministically instead of sleeping.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@mythical-fred-oss

mythical-fred-oss Bot commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

Review summary. The rules for the counter, the long-commit detector and the bar animator are clean, and their unit tests are good.

What I ran

Check Result
pre-commit run --files <changed> ✅ passed
bun run test-unit, default workers ❌ 4 screenshot timeouts (1 in this PR's spec, 3 unrelated)
vitest --project client --project server, VITEST_MAX_WORKERS=4 (as in CI) ✅ 97 files, 1007 tests
PR spec files plus compositions/, run 2 times ✅ 50/50 both times
Narrow viewport (390px), long commit ❌ the row overflows its h-16 (see inline comment)

Gates

  • Flaky tests (medium): the screenshot tests depend on wall-clock sleeps and on the machine's load (see inline comment).
  • Commit quality (medium): the commit message does not mention the snapshot-tooling rework, which belongs in its own commit. The subject is about 80 characters long.

Missing (medium): CounterAnimator has no unit tests. Only a real-timer component test covers it. Here is a suggested spec. I ran it, and all 7 tests pass:

+++ b/js-packages/web-console/src/lib/compositions/counterAnimation.spec.ts
+import { describe, expect, it } from 'vitest'
+import { CounterAnimator, MAX_COUNTED_STEP } from './counterAnimation'
+import { ManualScheduler } from './manualAnimationScheduler'
+
+const PERIOD = 2000
+const setup = () => {
+  const scheduler = new ManualScheduler()
+  const shown: number[] = []
+  const counter = new CounterAnimator(PERIOD, (v) => shown.push(v), { scheduler })
+  return { scheduler, shown, counter, last: () => shown.at(-1) }
+}
+
+describe('CounterAnimator', () => {
+  it('counts up to the new value in one period while the activity runs', () => {
+    const { counter, scheduler, last } = setup()
+    counter.update({ value: 7, active: true })
+    counter.update({ value: 11, active: true })
+    scheduler.advance(PERIOD / 2)
+    expect(last()).toBe(9)
+    scheduler.advance(PERIOD / 2)
+    expect(last()).toBe(11)
+    expect(scheduler.pendingCount).toBe(0)
+  })
+  it('jumps after an idle poll, on a decrease, and on a step above MAX_COUNTED_STEP', () => {
+    const { counter, last } = setup()
+    counter.update({ value: 7, active: false })
+    counter.update({ value: 9, active: true })
+    expect(last()).toBe(9)
+    counter.update({ value: 3, active: true })
+    expect(last()).toBe(3)
+    counter.update({ value: 4 + MAX_COUNTED_STEP, active: true })
+    expect(last()).toBe(4 + MAX_COUNTED_STEP)
+  })
+  it('continues from the shown value when a new poll interrupts a count', () => {
+    const { counter, scheduler, last } = setup()
+    counter.update({ value: 0, active: true })
+    counter.update({ value: 4, active: true })
+    scheduler.advance(PERIOD / 2)
+    expect(last()).toBe(2)
+    counter.update({ value: 6, active: true })
+    scheduler.advance(PERIOD)
+    expect(last()).toBe(6)
+  })
+  it('cancels a running count on dispose', () => {
+    const { counter, scheduler } = setup()
+    counter.update({ value: 0, active: true })
+    counter.update({ value: 4, active: true })
+    counter.dispose()
+    expect(scheduler.pendingCount).toBe(0)
+  })
+})

Missing (low-medium): no test renders the indicator below the sm breakpoint. That test would have caught the h-16 overflow.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

mihaibudiu
mihaibudiu approved these changes Oct 30, 2026
View reviewed changes
Comment thread js-packages/web-console/src/lib/functions/commitProgress.spec.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread js-packages/web-console/src/lib/compositions/counterAnimation.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread js-packages/web-console/src/lib/compositions/longCommit.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread js-packages/web-console/src/lib/compositions/manualAnimationScheduler.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread js-packages/web-console/src/lib/compositions/progressBarAnimation.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread js-packages/web-console/src/lib/compositions/progressBarAnimation.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread js-packages/web-console/src/lib/compositions/progressBarAnimation.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread js-packages/web-console/src/lib/compositions/progressBarAnimation.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread js-packages/web-console/src/lib/compositions/progressBarAnimation.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread js-packages/web-console/src/lib/compositions/progressBarAnimation.ts
scheduler?: AnimationScheduler
}

type Phase = 'follow' | 'finish' | 'restart'

@mihaibudiu mihaibudiu Oct 30, 2026

Copy link
Copy Markdown
Contributor

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.

you don't like enums? they should be more efficient

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@Karakatiza666 Karakatiza666 Nov 2, 2026

Copy link
Copy Markdown
Contributor Author

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.

Typescript introduced enums to JS.. and they are considered an anti-pattern.

https://dev.to/ivanzm123/dont-use-enums-in-typescript-they-are-very-dangerous-57bh

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
lalithsuresh
lalithsuresh requested changes Oct 30, 2026
View reviewed changes

@lalithsuresh lalithsuresh left a comment •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

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.

@Karakatiza666

  • why do we have a spinning wheel by the transaction ID at all times?
  • why does the ID number come and go?
  • what are there two transaction bars?
  • Are we also showing the same logic at the top of the UI where it normally says "Running" ?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@Karakatiza666

Karakatiza666 commented Oct 30, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author
  • The spinning wheel is a part of the design, I can ask Anna to iterate on it
  • The ID number is displayed for an in-progress transaction. When there is none in progress, the number disappears
  • One is transaction commit progress, other is bootstrap progress
  • No. The design includes progress bars only in performance tab. At the top of the UI only the commit status is displayed
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Karakatiza666

Karakatiza666 commented Oct 31, 2026

Copy link
Copy Markdown
Contributor Author

@lalithsuresh let me know what you'd like to change / don't like about the current design

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@lalithsuresh

lalithsuresh commented Oct 31, 2026

Copy link
Copy Markdown
Contributor

If the ID only shows up for active transactions, and there are modes where transactions happen all the time, I think the wheel isn't helpful -- let's remove.

I'm less sure about the ID showing up and disappearing but let's keep that behavior for now.

Karakatiza666 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Karakatiza666

Karakatiza666 commented Nov 2, 2026

Copy link
Copy Markdown
Contributor Author

@lalithsuresh I removed the spinner:

Screencast.From.2026-11-03.00-55-27.webm
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Karakatiza666
[web-console] Address review comments on the transaction progress UX
4bcfe7d
- Remove the spinner next to the transaction ID. Transactions can run all
  the time, so the spinner told nothing.
- Color "Transaction Started" in the page header blue, as "Started" in the
  performance tab.
- Keep the row height constant only from `sm` up. Below `sm`, the label and
  the bar take separate lines, and a fixed height made them overflow.
- Replace the transaction ID counter with `useInterpolatedValue`, a Svelte
  `Tween` that jumps when a key changes.
- Drive the animations in tests with Vitest fake timers instead of sleeps
  and a custom scheduler. Under load, the sleeps made the tests flaky.
- Push only the changed screenshots on top of the latest commit of the
  snapshots repository. A full-tree push deleted the screenshots that
  another branch had pushed. Retry a failed download.
- Reword the docs of the progress bar animation, and update the screenshots.

Signed-off-by: Karakatiza666 <[email protected]>
@Karakatiza666
Karakatiza666 force-pushed the fix-tx-progress-bar branch from 5fc851e to 4bcfe7d Compare November 2, 2026 22:04
@Karakatiza666
Karakatiza666 requested a review from lalithsuresh November 3, 2026 12:03
lalithsuresh
lalithsuresh approved these changes Nov 3, 2026
View reviewed changes
@Karakatiza666
Karakatiza666 added this pull request to the merge queue Nov 3, 2026
Merged via the queue into main with commit b992cdd Nov 3, 2026
1 check passed

Uh oh!

There was an error while loading. Please reload this page.

@Karakatiza666
Karakatiza666 deleted the fix-tx-progress-bar branch November 3, 2026 13:21
This was referenced Nov 9, 2026
Show progress of only long transactions #7405
Closed
web console unhelpful animation for transaction commit #6386
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mythical-fred-oss mythical-fred-oss[bot] mythical-fred-oss[bot] left review comments
@lalithsuresh lalithsuresh lalithsuresh approved these changes
@mihaibudiu mihaibudiu mihaibudiu approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@Karakatiza666 @lalithsuresh @mihaibudiu
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.