hcie-rust-v3.05/.kilo/plans/commit-impact-review-staged.md

# Staged Changes Impact Review — Commit Decision Support

## Goal

The user asked for a review of the currently staged changes to determine whether any functionality is lost compared to previous commits, and whether it is safe to commit.

## Scope

- **Base commit:** `dfef0de` (`HEAD` of `main` at review time).
- **Staged tree:** the 16 paths listed by `git diff --cached`.
- **Focus:** public API surface preservation, behaviour preservation, and compliance with the performance-protection rules in `AGENTS.md`.

## Staged Files

```
.kilo/plans/4k-multilayer-psd-performance-regression.md
AGENTS.md
build.id
hcie-composite/src/tiled.rs
hcie-egui-app/crates/hcie-gui-egui/src/app/mod.rs
hcie-egui-app/crates/hcie-gui-egui/src/canvas/mod.rs
hcie-egui-app/crates/hcie-gui-egui/src/canvas/render.rs
hcie-engine-api/src/layer_property_ops.rs            (new)
hcie-engine-api/src/lib.rs
hcie-engine-api/src/partial_composite.rs            (new)
hcie-engine-api/src/stroke_brush.rs                 (new)
hcie-engine-api/src/stroke_cache.rs                 (new)
hcie-engine-api/tests/performance_stroke_4k.rs      (new)
hcie-tile/src/lib.rs
logs/functional_regression_report.txt
unlock.sh
```

## 1. Public API Surface

The `Engine` public API in `hcie-engine-api` was extracted from the single monolithic `lib.rs` into four new internal modules (`layer_property_ops`, `partial_composite`, `stroke_brush`, `stroke_cache`). The modules are declared as `mod`, not `pub mod`, so the only public surface remains the items exported from `lib.rs`.

Comparison of public symbols (`pub fn` / `pub struct` / `pub enum` / `pub use`) between `HEAD` and the staged tree shows that **the same set of public methods is still available**, with one exception:

- `capture_canvas_png(&mut self) -> Option<String>` is removed.
- Search across the Rust source tree shows no live caller of this method (it only exists in the legacy backup tree `hcie-engine-api-orig/src_locked/lib.rs`).

Therefore, **no used public API is lost**. The removed function appears to be dead code.

## 2. Behaviour / Feature Loss Analysis

### 2.1 `hcie-engine-api/src/lib.rs` refactor

- Large functions (`render_composite_region`, `get_composite_pixels`, `begin_stroke`, `end_stroke`, stroke helpers, layer property setters) were moved into dedicated modules.
- No logic changes observed in the moved code other than the performance improvements documented below.
- `Engine::new` now initializes the new field `below_cache_dirty: true`, which is consistent with starting from a clean state.

### 2.2 `hcie-engine-api/src/stroke_cache.rs`

- `begin_stroke` / `end_stroke` keep the existing `active_stroke_mask` pooling, `stroke_before` undo snapshot, and `last_stroke_bounds` sub-rect snapshot logic.
- Introduces `rebuild_below_cache_if_needed`, which reuses the existing `below_cache` buffer when the active layer index and dimensions match.
- This is an **extension** of the existing `below_cache` mechanism, not a removal.

### 2.3 `hcie-engine-api/src/partial_composite.rs`

- `render_composite_region` and `get_composite_pixels` still follow the same sequence:
  1. render dirty vector layers,
  2. apply dirty effects/styles,
  3. sync dirty tiles,
  4. composite into scratch/full buffer.
- The `composite_scratch` buffer is still pooled and reused.
- `below_cache` reuse now guarded by `below_cache_dirty`.

### 2.4 `hcie-engine-api/src/layer_property_ops.rs`

- Property changes that do **not** alter pixel data (`opacity`, `visibility`, `blend_mode`, `parent`, `move_layer`, `clipping_mask`, `set_active_layer`, etc.) now:
  - set `document.composite_dirty = true`,
  - set `document.dirty_bounds = full_canvas`,
  - set `self.below_cache_dirty = true`,
  - and **no longer clear `tile_layers`**.
- This directly implements the AGENTS.md rule **"Tile cache over-clear removal"**.
- Structural changes (`add_layer`, `delete_layer`, `add_group`, `add_raw_layer`, `clear_all_layers`, `undo`, `redo`, `jump_to_history`) still clear `tile_layers`, which is correct because the layer count/content changes.

Risk note: `set_layer_parent` previously called `self.tile_layers.clear(); self.pre_tile_all_layers()`. The new code keeps the tile cache. This is safe only if the compositor (`hcie-composite/src/tiled.rs`) evaluates parent/clip hierarchy at composite time. The `base_indices` pre-computation in `tiled.rs` confirms that clip/parent logic is applied during compositing, so tile pixel data does not need to be rebuilt.

### 2.5 `hcie-composite/src/tiled.rs`

- Adds `SEQUENTIAL_PIXEL_THRESHOLD` and uses a sequential row loop for small dirty regions. Matches the AGENTS.md rule **"Small-region sequential compositing"**.
- Adds effects-cache reuse (`layer.effects_cache`) before calling `apply_layer_effects`. Matches the AGENTS.md rule **"Effects cache reuse"**.
- Helper functions (`blend_adjustment_pixel`, `blend_tile_pixel`, `blend_dense_pixel`) are pure refactoring; the blend math is unchanged.

### 2.6 `hcie-tile/src/lib.rs`

- `update_tiles_in_region` now writes only the intersecting sub-rectangle into an existing tile instead of replacing the whole tile. Matches the AGENTS.md rule **"Incremental tile update"**.
- Empty tiles are dropped only after verifying no remaining content.
- Logic is more conservative than the previous version (less data discarded), so no pixel content should be lost.

### 2.7 GUI changes

- `hcie-egui-app/crates/hcie-gui-egui/src/canvas/render.rs`: adds `SelectionEdgeCache` and `extract_selection_edges`. The previous inline edge-extraction loop is replaced by a cached version that is keyed by the mask buffer pointer/length. No engine API change; pure GUI optimisation.
- `hcie-egui-app/crates/hcie-gui-egui/src/canvas/mod.rs`: pointer-event downsampling for brush strokes (only forwards events when distance exceeds a spacing threshold). GUI-side performance improvement.
- `hcie-egui-app/crates/hcie-gui-egui/src/app/mod.rs`: adds `selection_edge_cache` field and resets it when the document is replaced.

These changes do not touch engine API usage except for passing `&mut selection_edge_cache` to the overlay renderer.

### 2.8 Documentation / metadata

- `AGENTS.md` rewritten for v4 terminology and expanded with the performance-protection table. No code impact.
- `build.id`, `unlock.sh`, `logs/functional_regression_report.txt`: metadata/build helper updates.
- New plan file: `.kilo/plans/4k-multilayer-psd-performance-regression.md`.

## 3. Potential Risks / Open Items

1. **`below_cache_dirty` consistency.** The new flag is set to `true` in `Engine::new` and on every non-structural property change. It is reset to `false` after a successful cache build or when no cache is needed. This logic appears consistent, but because it is new state, any missed setter that should invalidate it could cause stale composites. The staged code invalidates it in all observed property setters.

2. **Effects cache path in `tiled.rs`.** When `layer.effects_cache` exists and matches layer dimensions, the compositor uses `cached.rendered.clone()`. This introduces an extra buffer clone. The AGENTS.md rule states this is intended to avoid re-running the expensive effects pipeline. The clone cost is expected to be lower than `apply_layer_effects`, but this should be verified with the performance test.

3. **GUI downsampling.** The pointer-event spacing in `canvas/mod.rs` may subtly change the density of brush stamps for fast movements. This is a GUI-side change and does not affect engine correctness, but it could affect perceived stroke quality for some brush styles.

## 4. Performance-Protection Compliance

| Mechanism from `AGENTS.md` | Status in staged changes |
|----------------------------|--------------------------|
| `active_stroke_mask` pooling | Preserved in `stroke_cache.rs` |
| `composite_scratch` pooling | Preserved in `partial_composite.rs` |
| `below_cache` + `below_cache_dirty` | Extended with explicit `below_cache_dirty` flag |
| Conditional `effects_dirty` set | Preserved in `stroke_brush.rs` |
| Tile cache over-clear removal | Implemented in `layer_property_ops.rs` |
| Small-region sequential compositing | Implemented in `hcie-composite/src/tiled.rs` |
| Effects cache reuse | Implemented in `hcie-composite/src/tiled.rs` |
| Incremental tile update | Implemented in `hcie-tile/src/lib.rs` |

## 5. Recommended Validation Before Commit

Because this is a plan-only review and no build/test commands were executed, the following tests should pass before the user commits:

```bash
# Core engine regression tests
cargo test -p hcie-composite
cargo test -p hcie-tile
cargo test -p hcie-engine-api --test visual_regression

# Performance smoke test (non-gating, but useful for before/after numbers)
cargo test -p hcie-engine-api --test performance_stroke_4k -- --nocapture

# Ensure the workspace still compiles
cargo check -p hcie-engine-api
cargo check -p hcie-egui-app/crates/hcie-gui-egui
```

## 6. Conclusion

- **No used public API is removed.** The only removed symbol, `capture_canvas_png`, is dead code in the current tree.
- **No obvious feature loss.** The changes are structural refactoring plus performance improvements.
- **Performance-protection rules are honoured, not bypassed.** Several rules are newly implemented or strengthened.
- **Commit recommendation:** Proceed, **after** running the regression tests above and confirming a clean build.
- If the tests fail, the failures should be fixed before committing; the pre-commit hook already blocks commits that touch high-risk crates without manual review.