Skip to content
This repository was archived by the owner on Oct 1, 2026. It is now read-only.

Show file size on timeline frames - #46

Merged
elkampu merged 8 commits into
KalyteraSystems:mainfrom
iibaranov-IG:feat/timeline-frame-sizes
Sep 23, 2026
Merged

elkampu merged 8 commits into
KalyteraSystems:mainfrom
iibaranov-IG:feat/timeline-frame-sizes

Conversation

@iibaranov-IG

Copy link
Copy Markdown
Contributor

Summary

  • show whole KiB below 1 MiB and one-decimal MiB at or above it on server-rendered timeline cards
  • apply the identical display contract to cards added by Load more
  • reuse the existing FrameInfo.SizeBytes response field without changing the API
  • cover 2 KB and 1.5 MB examples plus the dynamic card path

Verification

  • dotnet test IPCamLapse.sln -c Release: 104 tests pass (57 IPCamLapse + 47 OpenCamInterop)
  • dotnet format IPCamLapse.sln --verify-no-changes --no-restore: pass
  • git diff --check: pass
  • signed commit: 0e84927

Closes #7.

@elkampu elkampu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The server and Load more paths do not yet implement one identical display contract.

The server uses C# F0/F1 with the current culture (lines 533-535), while JavaScript uses Math.round/toFixed (lines 421-423). For example, 2,560 bytes is a midpoint: .NET F0 uses midpoint-to-even and renders 2 KB, while Math.round renders 3 KB. A locale using a decimal comma can also diverge from JavaScript's decimal point. Please make rounding and culture explicitly identical in both paths and add a midpoint regression that exercises the dynamically created card rather than only searching the script text.

@iibaranov-IG

Copy link
Copy Markdown
Contributor Author

Agreed — nice midpoint catch. Two parts of one UI should not start a philosophical dispute over whether 2.5 rounds to 2 or 3. I’ll make the rounding rule and culture invariant explicit on both server and client paths, then add a midpoint regression that exercises a dynamically created card.

@elkampu elkampu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Exact head 0f28411 still does not exercise one shared formatting contract and fails CI on both platforms. The new test searches the rendered HTML for MidpointRounding.AwayFromZero, but Razor source is not emitted into the response, so that assertion necessarily fails. It also continues to search JavaScript source rather than execute a dynamically created card. In addition, the server FormatFrameSize MB branch still uses culture-sensitive F1, while JavaScript toFixed(1) always uses a decimal point. Please make KB and MB culture/rounding behavior explicit and identical, assert the rendered midpoint output, and execute the dynamic formatter/card path in a real JS/DOM test.

@iibaranov-IG

Copy link
Copy Markdown
Contributor Author

Готово: форматирование MB теперь явно использует инвариантную культуру, а midpoint в KB совпадает с JavaScript (2,560 bytes → 3 KB). Устаревшие проверки Razor-исходника убраны; вместо них новый тест запускает фактические formatFrameSize и addFrame, создаёт динамическую карточку и проверяет оба граничных результата. Head: 760bf88. CI зелёный на Ubuntu и Windows, включая контейнер и CodeQL. Буду признателен за повторную проверку.

@iibaranov-IG

iibaranov-IG commented Sep 10, 2026 via email

Copy link
Copy Markdown
Contributor Author

@elkampu elkampu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Exact head 760bf88 now fixes the culture handling, executes the dynamic card path, and passes the full suite, but the MB midpoint contract still differs. At 1,310,720 bytes (exactly 1.25 MiB), the server's double ToString("F1", InvariantCulture) renders 1.2 MB while JavaScript toFixed(1) renders 1.3 MB. The current tests use 1.5 MiB, so they cannot detect this divergence. Please choose one explicit MB midpoint rule for both paths and add the 1.25 MiB regression to the server-rendered and executed dynamic-card checks.

@iibaranov-IG

Copy link
Copy Markdown
Contributor Author

Addressed the latest review: server and dynamic-card formatting now use the same explicit half-up midpoint rule, including a 1.25 MiB → 1.3 MB regression. The JavaScript path executes the helper in the DOM test. CI is green on Ubuntu and Windows.

@elkampu elkampu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Verified exact head b74887f against current main in a clean merge simulation. The 1.25 MiB server/dynamic-card regression now agrees at 1.3 MB; locked restore and all 134 Release solution tests pass. The full diff and current checks are green.

@elkampu elkampu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Revalidated the up-to-date exact head 1730c3c after the normal branch update: locked restore, all 134 Release solution tests, and format verification pass locally. The 1.25 MiB regression agrees between server and dynamic card. Current Windows, Ubuntu, CodeQL, and C# analysis checks pass; container build is still running.

@elkampu
elkampu merged commit 10bbc6e into KalyteraSystems:main Sep 23, 2026
5 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Show file size on each timeline frame

2 participants