video: a film says it carries its label only when a picture shows it whole by donislawdev · Pull Request #214 · donislawdev/TestingFilesGenerator

donislawdev · 2026-11-03T12:03:32Z

What changes

The square that steps across a film is drawn over the label, in the label's own ink. On a low, wide film it rides across the text. A film of 1920x64 and one second has one picture, and the square stood over the start of its label there, while the manifest said label_embedded: true with no note.

A film now says true when at least one of its pictures leaves the text of the label whole, and false with a label_omitted note when the square changes a pixel of the text in every picture:

tfg generate --format mp4 --size 200kb --set width=1920 --set height=64 --set duration=1s --out out --json

film 1920x64	before	after
`mp4`, 1 s (one picture)	`true`, no note	`false`, `label_omitted`
`webm`, 1 s	`true`, no note	`false`, `label_omitted`
`mp4`, 10 s (default, ten pictures)	`true`	`true`

The file's SHA-256 is the same before and after in all three: only what the manifest says about the pixels moved.

How the answer is reached

imagelabel.TextRect is where Draw puts the text, and imagelabel.Hides says whether a rectangle painted in the label's ink changes a pixel of it. Ink over the glyphs' own ink changes nothing, and the band around the text is not the text - the same rule Readable keeps for the padding at the foot of a low picture. A first shape that asked only whether the rectangles overlap was caught by the new guard: a square over the last column of a 7 leaves the label exactly as it was.
video: the square's rectangle has one definition (squareAt), used by both painters and by the answer, so the two cannot come apart. The square stands in ten places, so a film is asked about at most ten pictures.
A label cut off by the picture's height keeps its own note, so a film never gets both.

GIF is not affected: its first frame has no marker, so its label is always whole there.

Guards

TestAFilmSaysItCarriesItsLabelOnlyWhenAPictureShowsItWhole asks the pixels. It renders every picture of a film, compares the text rows with the same film made tall (where the square passes under the label), and holds the answer to the pixels across heights at scales 1, 2 and 4, with 1, 2 and 10 changes, and across widths where the second of two pictures clears the text. Each series has to give both answers. It also checks that TextRect agrees with the measured ink of its label.
TestAFilmWhoseSquareCoversItsLabelSaysSoInItsPlan checks the plan of both film formats through the registry.
Both are red on the old behaviour. Six mutation entries are added.
Film bytes are covered by the existing golden hashes and by the guard that holds the strip painter to the whole one.

The second commit repoints a stale mutation entry. It matched the installer job's action pins, so every Dependabot bump of those actions left it matching nothing.

🤖 Generated with Claude Code

Summary by CodeRabbit

Bug Fixes
- Image and film outputs now correctly report when a label is not fully visible, including when a moving square obscures it throughout a film.
- Such outputs are marked as having no embedded label and include a note explaining the omission. The file name and manifest still identify the file; generated pixels are unchanged.
Documentation
- Updated the changelog to clarify that low images and low, wide films can obscure their labels.

…whole The square that steps across a film is drawn over the label, in the label's own ink. On a low, wide film it rides across the text, and a film of 1920x64 and one change drew it over the start of the label in its only picture while the manifest said label_embedded true. A film now says true when at least one of its pictures leaves the text of the label whole, and false with a label_omitted note when the square changes a pixel of the text in every picture. The answer is asked of the text's pixels (imagelabel.Hides): ink over the glyphs' own ink changes nothing, and the band around the text is not the text, the rule Readable keeps for the padding at the foot of a low picture. The square's rectangle has one definition (squareAt), used by both painters and by the answer. The pixels of every film are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…test The entry that moves the installer job to Linux matched the job's first steps, action pins included, so every Dependabot bump of those actions left it matching nothing. It now matches the comment over the job and its runs-on line, which is all it changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

coderabbitai · 2026-11-03T12:04:04Z

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Advanced
Run ID: 27e0c66f-463f-4cdd-b1e6-c8199b119af3

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

Trigger review

Walkthrough

The change checks whether the moving square obscures label text across a film timeline. Film label reporting now reflects readability across pictures, and planning adds a label_omitted note when every checked picture hides part of the label. Tests, mutation entries, translations, and changelog text cover the behavior.

Changes: Film label visibility

Layer / File(s)	Summary
Label and square overlap geometry `internal/format/imagelabel/imagelabel.go`, `internal/format/video/picture.go`	New helpers return label text bounds, identify glyph ink, calculate square rectangles, and check whether any picture leaves the label unobscured.
Film label reporting and plan notes `internal/format/video/choose.go`, `internal/format/video/plan.go`, `internal/gui/text/locale/said/en.json`, `internal/gui/text/locale/said/pl.json`, `CHANGELOG.md`	`Choice.Labelled` now accounts for square overlap across the timeline. Plans add a `label_omitted` note when the square obscures text in every picture. The translations and changelog describe this behavior.
Film visibility tests and mutation coverage `internal/guard/labelclaim_test.go`, `internal/guard/mutation/entries.py`	Tests check film label claims and plan notes against picture-level visibility. Mutation entries cover the visibility checks and update the film-picture mutation target.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Possibly related PRs

donislawdev/TestingFilesGenerator#206: Introduced shared label readability reporting and film planning behavior for labels clipped by height. This PR extends film reporting to account for text obscured by the moving square.

Suggested labels: bug, ui

Merge Risk: Moderate · previous review a8b55

The film-label mutation check needs a compilable mutant before this change is ready to merge; the checked label-reporting paths revealed no other issue.

Pre-merge checks: 13 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Scope, Duplication And Docs		The PR mixes the film-label fix with an unrelated installer mutation-maintenance change. The first commit changes video label reporting, while the second commit changes the installer mutation pattern …	Remove the installer mutation-entry change from this PR and submit it as a separate maintenance PR. Keep the film-label code, tests, mutation entries, locale strings, and CHANGELOG update together.

Passed checks: 13 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title names the film label behavior that changes, but it does not state the change itself. It refers to the video film label claim, and a reader can tell the subject from the title. It is specific…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Tests For Changed Behavior		The PR changes runtime label reporting, square placement, and plan notes. It adds 172 lines to `internal/guard/labelclaim_test.go`. The tests compare rendered film pictures with tall references, cover…
No Secrets Or Debug Leftovers		PASS: The reviewed diff changes nine existing source, test, locale, changelog, and mutation files. It adds no CLAUDE.md, AGENTS.md, .claude, or .env files. Added lines contain no credentials, URLs, em…
No Hardcoded Ui Styling		The pull request does not change GUI code in XAML, Slint, Fyne, Tkinter, or WPF. Its only GUI-path changes add English and Polish localization strings in JSON. No UI styling or copy-pasted controls ar…
No Obvious Performance Problems		No clear performance problem was introduced. The new film check examines at most `squareSteps` (10) square positions, and `imagelabel.Hides` scans only the bounded square/text intersection; the square…
Desktop Robustness		PASS. The diff adds only bounded in-memory label and video geometry checks, manifest-note logic, tests, translations, and mutation metadata. It adds no working-directory asset loads, file/settings wri…
Safe File Parsing		No unsafe file parsing is introduced. The PR adds static entries to the embedded `said/en.json` and `said/pl.json` catalogues; both remain valid JSON and contain only ordinary text-template placeholde…
System Changes Are Reversible		The PR changes video label analysis, manifest notes, tests, mutation entries, and translations. It does not add or change network filters, proxies, firewalls, system time, process hooks, Windows servi…
Clear User-Facing Text		PASS: The PR adds a plan note and English/Polish translations, plus a changelog entry. The note states the picture size, the square’s effect, and that the file has no readable label while its name and…
No Resource Leaks		The pull request adds only label geometry and metadata logic. The changed production code allocates no timers, goroutines, files, network responses, processes, handles, or unbounded caches. The test-o…

Full details: Title check

Explanation

The title names the film label behavior that changes, but it does not state the change itself. It refers to the video film label claim, and a reader can tell the subject from the title. It is specific enough to match the changeset's core rule: label_embedded is reported as true only when a picture shows the label whole.

Full details: Tests For Changed Behavior

Explanation

The PR changes runtime label reporting, square placement, and plan notes. It adds 172 lines to internal/guard/labelclaim_test.go. The tests compare rendered film pictures with tall references, cover heights 1–120, multiple scales, 1/2/10 changes, boundary widths, both film formats, label_embedded, and label_omitted. The test diff contains no deleted lines, skipped tests, or weakened assertions.

Full details: No Secrets Or Debug Leftovers

Explanation

PASS: The reviewed diff changes nine existing source, test, locale, changelog, and mutation files. It adds no CLAUDE.md, AGENTS.md, .claude, or .env files. Added lines contain no credentials, URLs, email addresses, IPs, local absolute paths, or debug-output/breakpoint calls. git diff --check also reports no issues.

Full details: No Hardcoded Ui Styling

Explanation

The pull request does not change GUI code in XAML, Slint, Fyne, Tkinter, or WPF. Its only GUI-path changes add English and Polish localization strings in JSON. No UI styling or copy-pasted controls are introduced.

Full details: No Obvious Performance Problems

Explanation

No clear performance problem was introduced. The new film check examines at most squareSteps (10) square positions, and imagelabel.Hides scans only the bounded square/text intersection; the square side is capped at 128 pixels. The change adds no UI-thread I/O, per-item external calls, unbounded collection processing, or polling.

Full details: Desktop Robustness

Explanation

PASS. The diff adds only bounded in-memory label and video geometry checks, manifest-note logic, tests, translations, and mutation metadata. It adds no working-directory asset loads, file/settings writes, culture-sensitive number/date handling, background work, admin requests, destructive actions, or network calls. The test formatting uses Go strconv with explicit base 10.

Full details: Safe File Parsing

Explanation

No unsafe file parsing is introduced. The PR adds static entries to the embedded said/en.json and said/pl.json catalogues; both remain valid JSON and contain only ordinary text-template placeholders. The changed Go and Python code does not add file reads, path resolution, deserialization, archive handling, or command execution. The existing encoding/json catalogue loader and text/template interpolation are unchanged and use no executable template functions or arbitrary-type deserialization.

Full details: System Changes Are Reversible

Explanation

The PR changes video label analysis, manifest notes, tests, mutation entries, and translations. It does not add or change network filters, proxies, firewalls, system time, process hooks, Windows services, registry entries, drivers, or other system-state modifications. The runner OS text appears only inside a mutation-test entry, not as a workflow change.

Full details: Clear User-Facing Text

Explanation

PASS: The PR adds a plan note and English/Polish translations, plus a changelog entry. The note states the picture size, the square’s effect, and that the file has no readable label while its name and manifest identify it. No new controls, confirmations, vague button labels, raw exceptions, or error messages lacking required guidance were introduced. Label terminology is consistent with the existing label notes.

Full details: No Resource Leaks

Explanation

The pull request adds only label geometry and metadata logic. The changed production code allocates no timers, goroutines, files, network responses, processes, handles, or unbounded caches. The test-only image map is finite and scoped to one test run. No resource leak or accumulation over application runtime is introduced.

Full details: Scope, Duplication And Docs

Explanation

The PR mixes the film-label fix with an unrelated installer mutation-maintenance change. The first commit changes video label reporting, while the second commit changes the installer mutation pattern in internal/guard/mutation/entries.py to stop pinning workflow actions. The PR description mentions this change, but it is still unrelated scope. The label feature itself updates CHANGELOG.md and locale messages, and the changed exported methods remain under internal/, so no separate documentation or external API failure is evident.

Resolution

Remove the installer mutation-entry change from this PR and submit it as a separate maintenance PR. Keep the film-label code, tests, mutation entries, locale strings, and CHANGELOG update together.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

CodeRabbit Verify

Run CodeRabbit Verify to test the changes in this pull request.

Recent review info

Run configuration

Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Advanced
Run ID: 42590a64-68c4-4823-963f-577a69a0761c

Commits

Reviewing files that changed from the base of the PR and between f5df531 and a8b5553.

Files selected for processing: 9

CHANGELOG.md
internal/format/imagelabel/imagelabel.go
internal/format/video/choose.go
internal/format/video/picture.go
internal/format/video/plan.go
internal/guard/labelclaim_test.go
internal/guard/mutation/entries.py
internal/gui/text/locale/said/en.json
internal/gui/text/locale/said/pl.json

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Recent review details

Context from checks skipped due to timeout: 8

GitHub Check: what the run found
GitHub Check: CodeRabbit
GitHub Check: bill of materials
GitHub Check: test on macos-latest
GitHub Check: test on ubuntu-latest
GitHub Check: linters
GitHub Check: coverage gate
GitHub Check: test on windows-latest

Additional context used

Code guidelines: 1

CONTRIBUTING.md — configured

Path-based instructions: 14

Applies to text shown to the user (labels, buttons, tooltips, placeholders, dialogs, errors, status messages, empty states, translations).