fix(terminal): gate per-frame repaint on dirty flag

[Vadim1987]

terminalView redrew the whole grid every frame (dirty gate was commented out, invalidate_terminal was a no-op). Restore the terminal-level dirty gate, implement invalidate_terminal, and mark the terminal dirty on write/reset so scrolling still repaints. Verified on desktop LÖVE: output, errors, and scroll-on-fill work; device test pending for battery/frame-time.

[Copilot]

Pull request overview

This PR restores a terminal-level “dirty” gate so the terminal grid isn’t repainted every frame, and wires up invalidation so terminal output/scrolling triggers repaint when needed.

Changes:

• Re-enabled terminal canvas redraw only when terminal.dirty is set (or when rendering snapshot/overlay mode).
• Marked the terminal dirty on write, reset, and invalidate_terminal() so output and scroll-on-fill still repaint.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/view/canvas/terminalView.lua	Reintroduces the dirty/overlay gate around full-grid repaint of the terminal canvas.
src/model/canvasModel.lua	Implements terminal invalidation by setting terminal.dirty on write/reset/invalidate.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[dsent]

Found a relevant commit when checking for dirty state was disabled and as I understand, we switched to repainting every frame: 86a7947

The exact context of this commit is not obviously clear.

[dsent]

@aldum just a gentle reminder about this one. Can you confirm the commit above was just a temporary/debug thing and not something supposed to work around some huge problem, yet not fixed?

[aldum]

@aldum just a gentle reminder about this one. Can you confirm the commit above was just a temporary/debug thing and not something supposed to work around some huge problem, yet not fixed?

I might have phrased that wrong. The point is, I vaguely remember doing this for a reason, and would like strong guarantees that undoing won't bork something. So let's Chesterton's fence this and recover the reason before removing, or build a strong test suite.

[Vadim1987]

Good point — let’s fence it properly. I recovered the reason: you turned the gate off in 86a7947 (“fix: terminal overlay drawing”), mid overlay-debugging — so the gate’s real interaction is with the overlay path, not normal output.

For the strong guarantee, I enumerated every terminal writer vs dirty (with D1 applied):

• write / push / reset / invalidate_terminal — all mark dirty (write & reset is what fixes scroll-on-fill, since the vendored roll_up doesn’t);
• update (scroll) — covered, since the preceding write/reset already marked dirty;
• redirect (io.write + magicPrint) — go through write, so covered.

The only path that doesn’t mark dirty is overlay exit (clear_snapshot just nils View.snapshot) — which is exactly your overlay code, and the staleness Copilot flagged on this PR. So D1 is provably safe for all non-overlay rendering; the overlay-exit repaint is the one open piece, and it’s yours.

If you want, I can close that last gap by marking the terminal dirty on clear_snapshot (one line, forces a single repaint on overlay exit) — but since you’re reworking overlay, I’d rather leave it to you unless you’d prefer I add it.

[aldum]

Good point — let’s fence it properly.

I think the LLM misunderstood what I meant by "Chesterton's fence" :D

clear_snapshot shouldn't be doing anything to terminal state, it's job is, as the name indicates, to clear the snapshot in View. It might make sense to dirty the terminal when clear snapshot happens, because it's related to a state change, but definitely not in this method.

[Vadim1987]

yeah, fair — clear_snapshot is View’s, it shouldn’t touch the terminal. wrong spot on my end. if we dirty the terminal on overlay exit it goes in the controller where it leaves the overlay, next to the clear_snapshot call, not in the method itself. so I’ll keep D1 to just the gate + dirty-on-write/reset for scroll, and leave the overlay-exit dirty to you. happy to add it at the call site if you want, just not in clear_snapshot.

[dsent]

Okay guys, I think AI can't address the raised issue properly (however, I think that its guess about the reason for the initial commit is reasonable, pun not intended).

What would be the shape of a good enough test suite for this, what do you think? @aldum

[dsent]

It's quite hard to test for visual regressions automatically against an unclear surface (after all, we don't know what exactly prompted the change).

I propose the following: manually test all our lesson-plan-required games, then watch for possible visual glitches and if we see any, check with this commit reverted.

If there's some visual problem, we have this as the prime suspect.

[aldum]

I have some vague ideas about adding golden tests to harmony and fuzzy-matching and diffing screenshots. Plans for the future.
Short of that, we should at least assemble a checklist for manual testers to go through. Good point on concentrating on lesson-relevant stuff, reduces blast radius of tests.