Skip to content

fix: guard against 0 / unfriendly font-scale editing (#49)#51

Merged
szrudi merged 4 commits into
mainfrom
fix/font-size-zero
Jun 27, 2026
Merged

fix: guard against 0 / unfriendly font-scale editing (#49)#51
szrudi merged 4 commits into
mainfrom
fix/font-size-zero

Conversation

@szrudi

@szrudi szrudi commented Jun 27, 2026

Copy link
Copy Markdown
Collaborator

Closes #49.

Problem

Seen live during a labelling session on hector. Two halves of the same bug:

  1. Server crash. /api/preview (and the print path) 500'd with ValueError: font size must be greater than 0. labelle sizes the font as round(line_height * fontScale/100) and PIL's ImageFont.truetype rejects a size of 0, so a fontScale of 0 took down the whole render.

  2. The input produced that 0, and was miserable to edit. The Scale field was a plain <input type="number" value={number}>. Backspacing the 90 default left the field empty, and Number("") === 0 got committed — i.e. the UI manufactured the exact value that crashes the server. Worse, the leftover digit meant typing a replacement read as a leading-zero (6006). (Reported verbatim: cleared 90, got 0, tried to type 60, ended up with 06.)

Fix

Server (label_builder.py): add _font_size_ratio() — coerces fontScale to a positive ratio and falls back to the default (DEFAULT_FONT_SCALE = 90, matching the client) for any non-positive or non-numeric value. Defends preview and print against malformed label files / direct API calls, not just this one input.

Client: extract NumberField, a numeric input that keeps the visible text as a free-form draft so the field can be emptied and retyped, commits a clamped number to the store only when the draft parses, and resyncs the draft to the committed value on blur — never mid-type, so clamping can't snap a half-entered number out from under the cursor. Reused for both the Scale and Frame inputs in TextWidgetEditor.

Tests

  • Server: TestFontScaleGuard — zero/negative/non-numeric/missing fall back to default; valid values pass through; preview_label with fontScale: 0 returns valid PNG bytes instead of raising.
  • Client: NumberField.test.tsx — clearing shows empty (not 0) and commits nothing; clear-and-retype has no leading zero; clamps below min / above max; settles to the clamped value on blur; reflects external value changes when not being edited.
  • Full suites green: client 87 passed, server 297 passed.

Version

PATCH bump 1.8.2 → 1.8.3.

🤖 Generated with Claude Code

szrudi and others added 2 commits June 27, 2026 11:00
Two halves of the same bug seen in the field:

Server: a non-positive or non-numeric fontScale 500'd /api/preview (and
print) — labelle sizes the font as round(line_height * fontScale/100) and
PIL rejects a size of 0. Add `_font_size_ratio()` to fall back to the
default for any unusable value, so a malformed label file or direct API
call can't crash the render.

Client: a plain `<input type="number">` made hand-editing the scale
hostile — clearing the field yielded Number("") === 0 (the very value that
crashed the server), and the leftover digit turned the next keystroke into
e.g. "06". Extract a `NumberField` that keeps the visible text as a draft
so the field can be emptied and retyped, commits a clamped number only when
the draft parses, and resyncs on blur (never mid-type). Reused for both the
Scale and Frame inputs.

Bump to 1.8.3 (PATCH).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The eye (preview) and x (remove row) buttons sat 8px apart in a narrow
cell, so previewing a row risked deleting it by accident. Bump the gap
from gap-2 to gap-4.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@szrudi

szrudi commented Jun 27, 2026

Copy link
Copy Markdown
Collaborator Author

Tacked on one small unrelated UI tweak (af0b631): widened the gap between a batch row's preview (eye) and delete (x) buttons from gap-2gap-4. They were close enough that previewing a row risked deleting it by accident. No version re-bump — folds into the unreleased 1.8.3.

szrudi added a commit that referenced this pull request Jun 27, 2026
#51 drops its version bump, so this PR carries the single 1.8.3 bump that
releases both fixes together. Merge #51 first (no release), then this.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Keeping package.json at main's 1.8.2 so merging this PR alone triggers no
release (the tag job skips an existing tag). The font + input-UX fixes here
ship together with the variable-rename fix under #52's single 1.8.3 bump.
Merge order: this PR first, then #52.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@szrudi

szrudi commented Jun 27, 2026

Copy link
Copy Markdown
Collaborator Author

Release batching (read before merging): dropped this PR's version bump (back to 1.8.2 = current main, commit 12bf760) so merging it alone triggers no release. Its fixes ship together with #52 under a single 1.8.3 release.

➡️ Merge this PR first, then #52. #52 carries the only version bump (1.8.3); because its merge commit sits on top of this one, the single release build contains both PRs' changes — avoiding two back-to-back releases.

An independent review pass found NaN/Infinity slip past the `<= 0` check
(Flask's JSON parser accepts the NaN/Infinity literals) and then blow up in
PIL's int conversion — the same 500 this guard exists to prevent. Use
math.isfinite. Documents the residual tiny-positive-rounds-to-0 case
(unreachable from the UI, tape-dependent) rather than silently clamping
otherwise-valid values.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@szrudi

szrudi commented Jun 27, 2026

Copy link
Copy Markdown
Collaborator Author

Self-review follow-up (9162d40): an independent adversarial pass caught that NaN/Infinity slip past the <= 0 guard — Flask's JSON parser accepts the NaN/Infinity literals, so {"fontScale": NaN} reproduced the same 500. Fixed with math.isfinite + tests. Also documented a residual: a tiny positive fontScale (e.g. 1) can round to 0 px on a small tape and still raise — unreachable from the UI (min 10), tape-dependent, so left un-normalized rather than silently clamping otherwise-valid values.

szrudi added a commit that referenced this pull request Jun 27, 2026
Keeping package.json at main's 1.8.2 so merging this PR (first) triggers no
release. The scan-select fix ships together with #51 and #52 under #52's
single 1.8.3 bump, which merges last.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@szrudi

szrudi commented Jun 27, 2026

Copy link
Copy Markdown
Collaborator Author

Final release plan (supersedes my earlier comment): no version bump here; merge it before #52. Full order is #53#51#52, where only #52 bumps (1.8.3) and triggers the single release containing all three.

@szrudi szrudi merged commit 5120c7f into main Jun 27, 2026
5 checks passed
@szrudi szrudi deleted the fix/font-size-zero branch June 27, 2026 11:26
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.

/api/preview 500s with 'font size must be greater than 0' when fontScale reaches 0

1 participant