Close out Sprint 10 review: verify all 3 stories, confirm ad-hoc items

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

backlog/backlog.md

@@ -447,3 +447,41 @@ backlog refinement above.
 - **Epic reorder confirmed as `scrum-master`'s own decision, not just inherited:** epic 8 stays ahead of epic 6 in this file's Order table. It is a direct, recent, explicit human-product-owner request reversing a scope call the same human product owner made only one sprint ago (Sprint 6 Review), and it is exactly what this sprint commits to — keeping the stated backlog order and the actual sprint commitment consistent.
 - **Recommended for Sprint 9:** the multi-select/align pair (10 pts, epic 6) as its own clean, undisturbed sprint slice, deliberately not sharing a sprint with another novel canvas-selection rework. "Undo and redo layout changes" (13 pts) remains available whenever it can be paired appropriately — a future planning session should weigh that pairing explicitly rather than default to bundling it in with the multi-select/align pair.
 - No changes to the two open impediments (template asset path strategy; UNC timeout/retry behavior, both in `logs/impediment_log.md`) — still non-blocking for this sprint's committed items.
+
+### Sprint 10 Review outcome (2026-10-27)
+
+`product-owner` verified all 3 Sprint 10 committed stories against acceptance criteria.
+
+- **Sprint goal met in full.** All 13/13 committed points Done: "Add a live wrap/clip indicator for
+  text elements" (3 pts), "Select multiple elements at once on the canvas" (5 pts), "Align and
+  distribute multiple elements" (5 pts). Verified against every acceptance criterion with real,
+  non-simulated evidence, recorded story-by-story in `backlog/epics/04_live_preview_and_record_navigation.md`
+  (indicator) and `backlog/epics/06_layout_efficiency_and_operator_tooling.md` (multi-select,
+  align/distribute). Full daily detail: `backlog/sprints/sprint-10.md`.
+- Independently re-ran the full test suite directly (`dotnet test`, 388/388 confirmed for
+  `EnvelopeRenderer.Desktop.Tests` — 376 pre-existing plus 12 new `AlignmentCalculatorTests` — and
+  124/124 for `EnvelopeRenderer.Cli.Tests`, unaffected by this sprint's desktop-only work) rather
+  than trusting dev-team's reported counts alone, and read the actual `CanvasElementEditor.MultiSelected`/
+  `GetWorldBounds`/`BeginMultiDrag`/`MultiDragTo`, `TemplateCanvasControl`'s `CollapseMultiSelectionIfSingular`/
+  `AlignSelection`/`DistributeSelection`, and the new `AlignmentCalculator` directly, confirming the
+  claims rather than rubber-stamping them.
+- **Two mid-sprint ad-hoc items, both correctly handled outside the formal committed scope:** (1) a
+  user-reported canvas-click smoothness regression, root-caused to an un-gated `SelectionChanged`
+  handler and fixed by only raising it on an actual selection change — confirmed the fix is
+  correctly scoped (touches only `TemplateCanvasControl.OnMouseDown`, doesn't alter any of this
+  sprint's three committed stories' own behavior) and the disclosed before/after timing evidence
+  (~50ms to 0.3ms) is genuinely conclusive, not hand-waved. (2) Two technical debt items logged
+  during Batch 2/3 verification (Address-Control interaction logic remaining untested WinForms-only,
+  now spanning three features; a pre-existing, unrelated selection-highlight width cosmetic gap) —
+  confirmed both are honestly scoped as Open/Low/non-blocking rather than either overstated or
+  swept aside, and neither blocks accepting this sprint's stories as Done.
+- **Verification-quality note worth naming:** the multi-select and align/distribute stories'
+  live-verification harnesses disclosed a real, specific tooling limit (`Control.ModifierKeys` has
+  no public setter, so modifier-click was proven via a direct call to the same private method the
+  real click handler delegates to, not a simulated key-press) rather than silently working around it
+  or overclaiming full end-to-end coverage — consistent with this team's established evidence-depth
+  disclosure practice (e.g. the Sprint 10 Batch 1 Delete-button `PerformClick()` caveat, the Sprint 9
+  rotated-clip pixel-row-count unreliability finding).
+- No new backlog items required. Epic 6 (Layout Efficiency and Operator Tooling) moves to 3 of 4
+  stories Done — only "Undo and redo layout changes" (13 pts, this backlog's highest-uncertainty
+  estimate, feasibility-checked 2026-10-19) remains in this epic.

logs/technical_debt_log.md

@@ -13,5 +13,5 @@ Append-only log of known technical debt. Maintained by `.claude/agents/qa-tech-d
 | 2026-10-19 | Address Control lines rendered on the designer canvas (`TemplateCanvasControl.DrawAddressControl`) and, since Sprint 7, on the new record-accurate preview panel (`TemplatePreviewControl`) visually overlap when a control uses a small font size with the default 1.25 line-spacing multiplier (observed with `sample-envelope-template3.xml`'s 8pt lines against the real sample CSV): each line's GDI+ `MeasureString` height is taller than the `FontSize * LineSpacingMultiplier` row height the baseline math advances by, so consecutive lines' bounding boxes visually overlap on screen even though their baselines are correctly spaced per `TEMPLATE_FORMAT.md`. Confirmed via live built-`.exe` screenshots of both surfaces during Sprint 7 batch 1 verification — present identically on the pre-existing (Sprint 6) design canvas, so this is not a Sprint 7 regression, just newly re-observed because a second surface now shares the same measurement approach. | Unintentional (side effect of using GDI+ `MeasureString`'s full line height as a stand-in for the real Debenu-rendered glyph height, which the code's own remarks already flag as "a design-time visual approximation... not a guarantee of pixel-for-point parity with the final PDF") | Low (cosmetic, canvas/preview-only — the actual PDF render path, `DebenuPdfRenderer`/`RenderEngine`, does not use this measurement at all and is unaffected; does not block reading resolved text, just makes tightly-spaced small-font address blocks visually crowded in the designer) | Open | Not fixed as part of Sprint 7 (out of scope for both the preview-panel and snap-to-grid stories). Candidate direction: derive each line's row height from the same font's ascent/descent (or a smaller line-height fraction of `MeasureString`) instead of the raw measured string height, or accept a documented minimum recommended `LineSpacingMultiplier` for small fonts. Revisit if an operator reports this as more than cosmetic. **Product-owner review confirmation (Sprint 7 Review, 2026-10-19):** Impact/status agreed as logged — Low, Open, non-blocking. Independently confirmed the root cause is confined to `TemplateCanvasControl`/`TemplatePreviewControl`'s GDI+ measurement path and that neither `DebenuPdfRenderer` nor `RenderEngine` (the real PDF render path) references this measurement at all, so no print-output correctness is at risk. No pushback on leaving this open; agree it should not block Sprint 8 planning. |
 | 2026-09-04 | `EnvelopeRenderer.Cli` only read `DEBENU_LICENSE_KEY` from the process environment. That's fine for `dotnet run --project ... --` (the CLI inherits the invoking shell's env directly), but `EnvelopeRenderer.Desktop` launches the CLI as a child process, which only inherits whatever environment variables were already present in whatever launched the desktop app itself (a double-clicked `.exe` or Start Menu shortcut typically has none) — so every desktop-launched render failed with Debenu error 999 regardless of a valid key existing on disk. Real-user-reported: the operator correctly guessed a `key.txt` dropped next to the exe should work (matching how the CLI's own test helper already resolved keys), but production code had no such fallback. This is a real Definition-of-Done verification gap from Sprint 1 Batches 4-5: the "real success run" verification used an in-process test harness with the env var set directly in that process, never the actual built `.exe` launched the way an operator would, so the gap wasn't caught before Sprint Review. | Unintentional | High (silently broke the desktop app's core success path for any non-`dotnet run` launch) | Resolved | Added `DebenuLicenseKeyResolver` (`code/src/EnvelopeRenderer.Cli/DebenuLicenseKeyResolver.cs`) to the shipped CLI: env var first, then a `key.txt` walked up from the executable's own directory — the same rule the test-only helper already used, now shared via delegation instead of duplicated. Documented in `CLI_CONTRACT.md`'s "Debenu license key" section and `code/README.md`'s desktop-app instructions. Verified by running the actual built `EnvelopeRenderer.Cli.exe` from the Desktop app's own output folder with no environment variable set at all, `key.txt` sitting next to it: exit `0`, valid 1.3 MB `%PDF-1.4` output, all 392 records. 5 new unit tests added (`DebenuLicenseKeyResolverTests.cs`); full suite 111/111 passing. |
 | 2026-10-19 | Post-Sprint-7-review, the plain editing canvas (`TemplateCanvasControl`/`CanvasElementEditor`) now deliberately rotates every element — static and dynamic/mixed alike — around its own bounding-box center (`RotationPivotCalculator.ComputeForCanvasEditing`), instead of the Sprint 6 fixed-anchor rule (`RotationPivotCalculator.Compute(isDynamic, ...)`) it previously shared uniformly with the real render and the new Sprint 7 preview panel. The editing canvas only ever draws an element's literal authored `{ColumnName}` bracket-token text (`TextElementLayout.DisplayText`), never a per-record resolved value, so the record-to-record text-width drift the fixed-anchor rule exists to prevent cannot occur there — applying it anyway just made a dynamic/mixed element's rotate-handle drag swing around a corner instead of spinning in place, a jarring interactive inconsistency the human product owner asked to have fixed after using the Sprint 7 increment. The real render (`RotatedTextAnchorCalculator`/`DebenuPdfRenderer`) and the new preview panel (`TemplatePreviewControl`/`TemplatePreviewBuilder`) are intentionally untouched and still call `RotationPivotCalculator.Compute` with the real `isDynamic` value, so the original 2026-10-09 record-drift defect fix is fully preserved where it actually matters. See the dated note added to the "Keep rotated dynamic and mixed-content fields positioned consistently across records" story in `backlog/epics/02_template_designer_gui_foundation.md` for the full reasoning and human product-owner approval record. | Deliberate (a considered, approved trade-off decoupling a purely cosmetic/interactive editing-canvas behavior from a record-accuracy rule that surface was never actually subject to — not a regression or an oversight) | Low (the editing canvas was never a record-accuracy surface for rotation to begin with, since it never draws resolved per-record text; the real render and record-accurate preview panel, where the original defect actually mattered, are unaffected and keep the exact same fixed-anchor behavior) | Logged (deliberate design decision, not a defect to resolve) | Not applicable — this row documents a considered, already-implemented, and approved design decision for future reference (e.g. if a future story ever gives the editing canvas its own resolved-per-record preview capability, this decoupling would need to be revisited). |
-| 2026-10-26 | Sprint 8 ("Rotate the whole Address Control...") extracted its *new* rigid-group-rotation and drag-handle math into framework-free, unit-tested `EnvelopeRenderer.Desktop.Core.Design` classes (`AddressControlLayout.BoxCenter`, `PointRotation`, `AddressControlRotateHandle`), but the *pre-existing* (Sprint 6) Address Control interaction logic it builds on top of — `TemplateCanvasControl.HitTestAddressControl`, `HitTestAddressResizeHandle`, and the whole-control move/resize mouse handlers — still lives directly in the WinForms `Views` project rather than a `CanvasElementEditor`-equivalent for Address Controls, so it remains untestable by `EnvelopeRenderer.Desktop.Tests` (verified only via a live built-`.exe` reflection-driven smoke, not a unit test) unlike the standalone-element equivalents (`CanvasElementEditor.HitTest`/`HandlePosition`/`RotateDragTo`, which are unit tested). This is a proportional, in-scope decision for this sprint (retroactively refactoring Sprint 6's already-shipped, already-smoke-verified move/resize/hit-test code was not part of either committed story), not an oversight, but it leaves a real, growing architectural inconsistency between the two element kinds' testability. | Unintentional (a natural side effect of adding new, better-architected code alongside older code that predates the pattern, not a deliberate call to leave the old code as-is) | Low (no known behavioral defect — the smoke-tested code paths work correctly per this sprint's live verification; purely a testability/maintainability gap, and the two newly-added rotation classes themselves are fully unit tested) | Open | Candidate direction: extract `TemplateCanvasControl`'s Address-Control-specific hit-test/drag/resize state and math into a new `AddressControlEditor` (Desktop.Core), mirroring `CanvasElementEditor`'s existing role for standalone elements, so all four interaction kinds (select, move, resize, rotate) are unit tested consistently. Not required by any currently-committed story; revisit if Address Control interaction logic grows further (e.g. multi-select epic 6 work) or a live-verified defect is found in it. **2026-10-27 update:** this row's own trigger occurred — Sprint 10 Batch 2 ("Select multiple elements at once on the canvas") added `AddressControlsInRect`/`ApplyMultiDragToAddressControls`/`ToggleAddressControlMultiSelect` directly to `TemplateCanvasControl`, following the same untested-WinForms-only pattern rather than doing the extraction, a deliberate choice (the standalone-element half of the exact same feature *is* fully unit tested in `CanvasElementEditor`, so multi-select's core logic has real test coverage even though the Address Control half doesn't) rather than scope-creeping an unrelated refactor into an already-large story. Still Open; the case for the extraction is now stronger with two features built on the untested side. |
+| 2026-10-26 | Sprint 8 ("Rotate the whole Address Control...") extracted its *new* rigid-group-rotation and drag-handle math into framework-free, unit-tested `EnvelopeRenderer.Desktop.Core.Design` classes (`AddressControlLayout.BoxCenter`, `PointRotation`, `AddressControlRotateHandle`), but the *pre-existing* (Sprint 6) Address Control interaction logic it builds on top of — `TemplateCanvasControl.HitTestAddressControl`, `HitTestAddressResizeHandle`, and the whole-control move/resize mouse handlers — still lives directly in the WinForms `Views` project rather than a `CanvasElementEditor`-equivalent for Address Controls, so it remains untestable by `EnvelopeRenderer.Desktop.Tests` (verified only via a live built-`.exe` reflection-driven smoke, not a unit test) unlike the standalone-element equivalents (`CanvasElementEditor.HitTest`/`HandlePosition`/`RotateDragTo`, which are unit tested). This is a proportional, in-scope decision for this sprint (retroactively refactoring Sprint 6's already-shipped, already-smoke-verified move/resize/hit-test code was not part of either committed story), not an oversight, but it leaves a real, growing architectural inconsistency between the two element kinds' testability. | Unintentional (a natural side effect of adding new, better-architected code alongside older code that predates the pattern, not a deliberate call to leave the old code as-is) | Low (no known behavioral defect — the smoke-tested code paths work correctly per this sprint's live verification; purely a testability/maintainability gap, and the two newly-added rotation classes themselves are fully unit tested) | Open | Candidate direction: extract `TemplateCanvasControl`'s Address-Control-specific hit-test/drag/resize state and math into a new `AddressControlEditor` (Desktop.Core), mirroring `CanvasElementEditor`'s existing role for standalone elements, so all four interaction kinds (select, move, resize, rotate) are unit tested consistently. Not required by any currently-committed story; revisit if Address Control interaction logic grows further (e.g. multi-select epic 6 work) or a live-verified defect is found in it. **2026-10-27 update:** this row's own trigger occurred — Sprint 10 Batch 2 ("Select multiple elements at once on the canvas") added `AddressControlsInRect`/`ApplyMultiDragToAddressControls`/`ToggleAddressControlMultiSelect` directly to `TemplateCanvasControl`, following the same untested-WinForms-only pattern rather than doing the extraction, a deliberate choice (the standalone-element half of the exact same feature *is* fully unit tested in `CanvasElementEditor`, so multi-select's core logic has real test coverage even though the Address Control half doesn't) rather than scope-creeping an unrelated refactor into an already-large story. Still Open; the case for the extraction is now stronger with two features built on the untested side. Sprint 10 Batch 3 ("Align and distribute multiple elements") added a third: `GetAddressControlWorldBounds`, in the same file, following the same pattern (its standalone-element counterpart, `CanvasElementEditor.GetWorldBounds`, is unit tested; this one isn't). |
 | 2026-10-27 | A standalone canvas element's selection highlight border (`TemplateCanvasControl.DrawElement`'s `isSelected` rectangle) visibly wraps only part of a multi-word `DisplayText` (e.g. the default "Static text" highlights only "Static") rather than the full rendered string, for every plain (non-boxed) element regardless of selection mode — discovered via a live built-form screenshot while verifying Sprint 10 Batch 2's multi-select highlighting, and confirmed pre-existing (present identically for the ordinary single-selection case, `MeasureElement`/`DrawElement`'s non-box branch is unmodified by Batch 2). Root cause not yet confirmed by code inspection beyond the immediate suspect: `MeasureElement`'s non-boxed path calls `Graphics.MeasureString(text, font)` (the no-layout-rectangle overload), a GDI+ API with a long-documented history of measuring narrower than what `Graphics.DrawString` actually renders for certain text/hint combinations — plausible but not yet isolated with a minimal repro. | Unintentional (a GDI+ measurement/render mismatch, not anything introduced by this sprint's own drawing changes) | Low (cosmetic, canvas-only design-time approximation — does not affect hit-testing correctness, which was unaffected in this session's own multi-select verification, nor the real PDF render path, which never calls this method) | Open | Not investigated further or fixed as part of Sprint 10 Batch 2 (out of scope for the multi-select story; affects the pre-existing single-selection highlight equally). Candidate direction: try `Graphics.MeasureString` with a generously large layout rectangle (rather than the no-rectangle overload) or `TextRenderer.MeasureText`, and compare against `DrawString`'s actual rendered extent on a real screenshot. Revisit if an operator reports the selection highlight looking wrong, or before any story that depends on this measurement being visually accurate (e.g. further canvas polish). |

state.md

@@ -24,7 +24,9 @@
 
 **Sprint 10 Batch 3 complete (2026-10-27):** "Align and distribute multiple elements" (5/13 points) shipped — Sprint 10's full committed scope (13/13 points) is now Done. New framework-free `AlignmentCalculator` (Desktop.Core) computes position deltas for 6 alignment edges and 2 distribution axes (center-to-center spacing, a documented MVP simplification) from a list of world-space bounding boxes; `TemplateCanvasControl` feeds it a mixed bounds list (standalone elements via a newly-public `CanvasElementEditor.GetWorldBounds`, Address Controls via a new `GetAddressControlWorldBounds` extracted from Batch 2's rubber-band code) and applies the deltas back. A new 8-button toolbar row triggers the operations, enabled only at 2+ selected items. Full suite 376/376 -> 388/388. Live-verified via reflection-driven harness including a **fresh** full-form screenshot (not reused from Batch 2, per this story's own carry-in note). Full detail: `backlog/sprints/sprint-10.md`, `backlog/epics/06_layout_efficiency_and_operator_tooling.md`.
 
-**Next action:** Sprint 10's committed scope is fully done (13/13 points, all 3 batches). Per the automated state-driven handoff, proceed directly into Sprint Review (`product-owner` leads, `process/04_sprint_review.md`) and then Sprint Retrospective (`scrum-master` leads, `process/05_sprint_retrospective.md`) without stopping to ask — the retrospective's own completion is this workflow's next designated pause point (per `AGENTS.md`), not before. Still open, non-blocking, to consider for Sprint 11 planning: "Add an adjustable width and height with text wrapping to Address Control lines" (epic 4, 8 pts, deferred from Sprint 10); "Undo and redo layout changes" (epic 6, 13 pts, now the last story in this epic); "Warn on text overflow before render" (epic 4, provisional 5 pts, still blocked); both epic 7 impediments (asset path strategy, UNC timeout/retry, open since 2026-09-04); "Complete the first text-only operator workflow" (epic 1, 5 pts) remains flagged as stale; the Proposed process-improvement log entry above awaiting a human decision; two technical debt items from Sprint 10's verification (Address-Control interaction logic still untested WinForms-only, growing with two features now built on it; the selection-highlight width cosmetic gap), both Open/Low and non-blocking.
+**Sprint 10 Review complete (2026-10-27):** `product-owner` verified all 3 committed stories against acceptance criteria (verdict: sprint goal met in full, 13/13 points). Independently re-ran the full test suite (388/388 Desktop.Tests, 124/124 CLI.Tests, confirmed directly) and read the actual `CanvasElementEditor`/`TemplateCanvasControl`/`AlignmentCalculator` code rather than trusting dev-team's report alone. Confirmed both mid-sprint ad-hoc items (the click-smoothness fix; two newly-logged technical debt entries) are correctly scoped and non-blocking. No new backlog items required. Full detail: `backlog/backlog.md`'s "Sprint 10 Review outcome" section.
+
+**Next action:** Proceed directly into Sprint Retrospective (`scrum-master` leads, `process/05_sprint_retrospective.md`) per the automated state-driven handoff — this is the workflow's next designated pause point (per `AGENTS.md`): stop and wait for the user once the retrospective is written, before starting Sprint 11 planning. Candidates to weigh for Sprint 11, once resumed: "Add an adjustable width and height with text wrapping to Address Control lines" (epic 4, 8 pts, deferred from Sprint 10); "Undo and redo layout changes" (epic 6, 13 pts, now the last story in that epic); "Warn on text overflow before render" (epic 4, provisional 5 pts, still blocked); both epic 7 impediments (asset path strategy, UNC timeout/retry, open since 2026-09-04); "Complete the first text-only operator workflow" (epic 1, 5 pts) remains flagged as stale; the Proposed process-improvement log entry above awaiting a human decision; two technical debt items from Sprint 10's verification (Address-Control interaction logic still untested WinForms-only, growing with each new feature built on it; the selection-highlight width cosmetic gap), both Open/Low and non-blocking.
 
 ## Phase reference