| 1 | Coordinate frame: scene stores cm in the nominal Qt-native y-down, origin top-left labeling, but the data path consumes raw scene y directly as CAD Y-up — the view flips display via transform.scale(zoom, -zoom) (ui/canvas/canvas_view.py:609) so larger scene-y renders higher/bottom-left origin (ADR-002, §8.1). The status bar, DXF (dxf_y = scene_y), and serialization consume raw scene y with no conversion — the scene_to_canvas helper has zero production callers. Explicit Y-flip seams that DO convert: the coordinate-input parser (ADR-021, flips typed math-up Y to scene at core/coordinate_input/parser.py — inline -b, not via the helper), thumbnail/minimap mapping (§8.9.5/8.9.6), and the agent render-pixel formula (ADR-034 D1.3). Full reconciliation (why §8.10 abstract vs §11.4 operative disagree) is owned by ogp-qt-cad-reference §1. | One flip, in one place, keeps every geometry computation in a single frame. | Double-flipped or unflipped geometry: items land mirrored, typed coordinates go the wrong way, angles negate, text renders upside-down. | test_agent_api_render_coordinate_frame.py pins the render frame; integration tests use scene coords per §8.10. |
| 2 | Rotatable rect-bearing items (CircleItem/RectangleItem/EllipseItem) serialize as pos + rect().center() with rotation as a separate angle, so every geometry mutation must end with transformOriginPoint() == rect().center(). Route resizes through the single primitive resize_handle.resize_rect_item_keeping_anchor(...); rotation pivots on rect().center(), never boundingRect().center() (a runtime-only badge expands boundingRect asymmetrically). (§8.9.8, ADR-028, #218/#219.) | The save format assumes pivot == geometric centre; badges are never serialized, so a badge-dependent pivot disagrees between save and load. | Rotated items drift on save/reload and jump on the next rotation — silent geometry corruption. | tests/integration/test_rotation_aware_resize.py (shape×angle×handle matrix), test_rotation_pivots_about_rect_center_with_badge, test_apply_keeps_rotated_plant_centered. |
| 3 | Exactly two ways onto the undo stack: CommandManager.execute(cmd) (runs it) and CommandManager.register_applied(cmd) (for changes already applied live, e.g. a finished drag). Both emit stack_changed → mark_dirty. Never hand-append _undo_stack or hand-emit signals (#209; core/commands.py:68/86). undo()/redo() also emit stack_changed; clear() does not (so new/load stays clean). | Dirtiness and every panel-refresh path hang off stack_changed; a third path silently skips them. | Post-save undo/close silently discards changes (the #209 data-loss bug); panels go stale. | Docstring contract in commands.py; tests/integration/test_panel_refresh_wiring.py; senior review treats a raw append as P0. |
| 4 | One user gesture = one undo step. A multi-part change (geometry+position, metadata+resize+override, mirror of N items, agent write in D2) is one composite command. Live drags mutate directly and register_applied one command on release; curve reshapes use whole-geometry snapshot commands (SetCurveGeometryCommand, ADR-025). Free-text fields commit on 600 ms debounce/focus-out, not per keystroke (#210, #214). | Undo must match user intent; per-keystroke or per-sub-step commands make Ctrl+Z useless and (post-#209) trigger N heavyweight refreshes. | "Undo does half of it"; undo-stack spam; calendar-refresh churn. | §8.2 rule; test_properties_panel_incremental.py; ADR-025 point 5 idiom. |
| 5 | LayersPanel is a pure view. Every layer mutation (add/delete/rename/reorder/visibility/lock/opacity) goes through the layer commands in core/commands.py (AddLayerCommand, DeleteLayerCommand, RenameLayerCommand, ReorderLayersCommand, SetLayerPropertyCommand, MoveToLayerCommand — verified lines 1476–1724). Opacity drags coalesce via canvas_scene.preview_layer_opacity() + commit-on-release (#207/#208). Never hand the panel a mutable alias of scene.layers (defensive copy). | Layer ops must be undoable and dirty the document like everything else (invariant 3). | Un-undoable layer edits; the §11.4 panel↔scene list-aliasing bug. | FR-LAYER-08/09; layer command tests; comments in ui/panels/layers_panel.py. |
| 6 | Task status has ONE write path: ProjectManager.set_task_status (core/project.py:687). set_task_completion (:298) is a compat shim delegating to it; the .ogp task_completions key is a write-only serialized mirror (older binaries read it; nothing in-app does). Every surface reading/writing status must derive task_id via the shared make_calendar_task_id() (services/task_generator.py:172) + canonical species_key() (models/plant_data.py:370, ADR-016: source_id → scientific_name → common_name, strip+lower). (ADR-029 + #227/#228 addenda.) | Two surfaces (Tasks tab, calendar dashboard) share one status store; ids derived two ways silently diverge even with correct writes. | "Done on one tab, still open on the other" — the exact PR #227 bug. | test_calendar_task_convergence.py, test_tasks.py::TestCrossSurfaceSync / TestStatusFlows. |
| 7 | Bed-only features are built centrally, never per-shape: GardenItemMixin.build_bed_context_menu(menu, *, grid_enabled, supports_grid, supports_soil) + dispatch_bed_action returning BedMenuActions (ui/canvas/items/garden_item.py:37/490/533; ADR-017, §8.14). The context-menu guard is is_plant_parent_type, with supports_grid=supports_soil=is_bed_type(...). New bed feature = the 6-step playbook in §8.14, ending with a new assert actions.<field> is not None in the parametrised test. | Bed-capable shapes are FOUR classes (Rect/Polygon/Ellipse/Circle) + containers + trellis; per-shape copies shipped broken twice in three months. | The new feature is missing from one or more shapes and nobody notices until a user report. | tests/integration/test_bed_context_menu.py (parametrised over every plant-parent shape). |
| 8 | Predicate split (ADR-031): is_bed_type = soil-capable (GARDEN_BED, RAISED_BED, CONTAINER, CONTAINER_ROUND, WALL_PLANTER); is_plant_parent_type = soil set + TRELLIS; is_container_type = litres-by-height subset. All in core/object_types.py:595/608/622. Pick by seam: soil features (tests, mismatch, amendment volume, grid overlay) → is_bed_type; parent/relationship features (reparenting, child propagation, context menu, "Contained Plants") → is_plant_parent_type. New "things plants live in/on" are ObjectType tags on existing shape items, not new QGraphicsItem subclasses. | One predicate can't express "parent but no soil" (trellis); new item classes cost ~16 dispatch sites each (rejected repeatedly in ADR-022/031/032). | Trellis gets soil tests, or containers miss reparenting; or you inherit the "new serializer + resize + constraint wiring" tax. | Predicate docstrings; test_container_gardening.py, test_trellis.py; agent-api drift guard test_agent_api_mapping.py (asserts inlined name sets == SOIL_CONTAINER_TYPES). |
| 9 | .ogp evolution is additive-first. New persisted data = a new top-level key or additive metadata/item key that old apps ignore and old files load without (defaults). FILE_VERSION (currently "1.4", core/project.py:34) bumps are rare and deliberate — only for changes an old app cannot safely ignore (e.g. new item types, 1.4 = arc/bezier). The loader rejects files with version > FILE_VERSION. Smart symbols are the model case: serialized as type:"group" + smart_symbol metadata, old apps degrade to a plain group (ADR-032). | Every bump locks all older installs out of new files; the additive ethos keeps files exchangeable across versions. | Users on older versions can't open shared plans; or (worse) an old app silently drops data it didn't know it had to preserve. | test_container_roundtrip.py (round-trip + unbumped FILE_VERSION), _is_newer_file_version guard in project.py, ADR review of any bump. |
| 10 | Qt-free/Qt-touching split. Domain logic that can be Qt-free must be: core/plant_sizing.py, core/container_model.py, core/parametric_eval.py, core/coordinate_input/parser.py, services/task_generator.py, services/task_status.py, services/harvest_aggregation.py, models/smart_symbol.py, and in agent_api/: schema.py, mapping.py, queries.py, diagnostics.py, providers.py (the agent_api/ modules are verified: no PyQt6 import — see the anchored grep in Provenance). Note services/task_generator.py is Qt-free logic except from PyQt6.QtCore import QCoreApplication used solely for translate() (it holds no QObject state and is unit-tested without qtbot). Qt-touching agent modules are exactly bridge.py (QObject signal marshaling) and render.py (the one documented exception). | Qt-free code is unit-testable without a QApplication, reusable off the main thread, and immune to teardown crashes. | Logic becomes untestable-without-GUI; agent tools gain hidden main-thread requirements. | Module docstrings state Qt-free-ness; grep PyQt6 (see Provenance); tests import these modules without qtbot. |
| 11 | Import direction: ui imports core/models/services; core never imports ui at module level — where core code must dispatch on item classes (mirror_geometry, auto_constraint, measure_snapper), the ui.canvas.items import is function-local or TYPE_CHECKING-only, explicitly "to avoid import cycles (items import core)". app wires everything and may import anything. | Prevents import cycles; keeps core loadable headless. | ImportError cycles at startup; core silently grows a hard GUI dependency. | Convention + the in-code comments; verify with the Provenance grep. |
| 12 | Agent API safety model (ADR-033/034, §8.19): (a) tool handlers run on the uvicorn thread and touch Qt only via MainThreadBridge.run_on_main (queued signal + Future); (b) Qt-touching handlers are async def and offload the blocking hop via anyio.to_thread.run_sync (a sync handler blocks the event loop — verified against mcp 1.28.1); (c) reads use ProjectManager.snapshot_dict() which never mutates state (sync_journal=False); (d) server binds 127.0.0.1 only ("never 0.0.0.0"); (e) reads are unauthenticated (loopback trust) but every scene-mutating write tool is double-gated (D2.0, v1.24.3): the tool is registered at all only when agent_api_writes_enabled AND agent_api_token are both set (writes_active in build_server), and each handler calls _require_write_auth — constant-time compare, isascii()-guarded — in the same request task, before the anyio.to_thread.run_sync hop. The token arrives via ?token= URL param (preferred — Claude Code drops configured headers on tool calls) or Authorization: Bearer; the gate is per-tool, not blanket middleware, so read-only D1 clients keep working; (f) shutdown calls bridge.abort_pending() before server.stop(). | Qt is main-thread-only; an unauthenticated localhost server must not be able to mutate the user's plan; close must not deadlock on an in-flight hop. | UI freezes/crashes from off-thread Qt access; any local process could edit the plan; hang on app close. | test_agent_api_bridge.py (marshaling/timeout/abort), test_agent_api_server.py (end-to-end), the bridge.py house-rule docstring. |
| 13 | Agent writes go through the command system — one agent operation = one undoable command on the same CommandManager (invariants 3–4 unchanged); never a parallel mutation path. D2.0 shipped move_object/delete_object (ADR-036) and set the binding precedent: a write tool must mirror the GUI's whole orchestration, not just its simplest Command. move_object carries a bed's contained plants and re-evaluates the moved plant's bed membership; delete_object also strips referencing constraints and a HOUSE's linked roof ridge. Both refuse (raise) rather than half-do what the GUI forbids — constrained items, journal pins, locked-layer items, individual group members — in the shared _resolve_agent_item chokepoint (app/application.py), which every new write tool inherits. | The user must be able to Ctrl+Z anything an agent did; a tool that runs only the obvious command silently corrupts relationships the GUI maintains. | The D2.0 review P0: a bare MoveItemsCommand([item], delta) abandoned a bed's plants and left stale parent/child links for soil diagnostics to act on. | ADR-033/036; test_agent_api_writes.py (bed-children, boundary-crossing both directions, constrained/pin/locked-layer/group-member refusals); design review of any D2 PR. |
| 14 | Exception-handling trust rule (PR #236, §11.4): at a seam that ingests untrusted input (user-dropped JSON, network responses, opened files), put one broad except Exception at the ingestion chokepoint (log-and-skip / degrade); keep narrow typed catches only for first-party/bundled input so our own bugs crash loud. Never enumerate exception families at a trust boundary. BaseException still propagates. Model: services/smart_symbol_library.py (user loop broad, bundled loop narrow). | Enumerating families lost three rounds in a row — each new poison file found an unlisted family. Untrusted input must be non-fatal by construction. | A crafted/corrupt user file crashes the app; or, inverted, a packaging bug in bundled data gets silently swallowed. | tests/unit/test_smart_symbol_library.py (poison files skipped; malformed bundled crashes loud). |
| 15 | Plant sizing precedence has one home: Qt-free core/plant_sizing.py (PlantSizing, sizing_for_item, db_spacing_radius_cm). Precedence: manual spacing_radius_cm override > DB max_spread_cm/2 > None; spacing ring shown only when effective > drawn footprint. Species assignment routes through the single undoable ApplySpeciesCommand path (never bare item.metadata["plant_species"] = ..., which doesn't repaint and is masked by overrides). (ADR-028, #213/#218.) | The precedence was re-encoded inline in three files and drifted; bare metadata writes skip prepareGeometryChange(). | Ring size wrong on screen vs. data; silent no-op species assignment. | tests/unit/test_plant_sizing.py; §11.4 contract entry. |
| 16 | i18n: every user-visible string goes through self.tr() (QWidget/QDialog), QCoreApplication.translate("Context", ...) (non-QObject, e.g. QGraphicsItem menus, command descriptions under context "Commands"), or QT_TR_NOOP (module-level dicts). Hardcoded English f-strings bypass Qt Linguist and the i18n gate test cannot see them — it only checks registered strings for unfinished translations (full gate mechanics: ogp-diagnostics-and-tooling §1.2/1.3, the canonical home). Exemptions: MCP tool/prompt descriptions (English API contract), Latin plant names, data-baked names like smart-symbol name/name_de. | German is a shipped language; the gate test has a documented blind spot for unregistered strings. | English leaks into the German UI, undetectably by CI. | tests/unit/test_i18n.py::TestTranslationFiles::test_german_ts_has_no_unfinished (registered strings only — the review must catch plain strings); §8.3. |
| 17 | In-scene handle grab allow-list (ADR-025 pt. 7): any new ItemIgnoresTransformations in-scene drag handle must be added to CanvasView's dropped-grab re-grab tuple (ResizeHandle, RotationHandle, VertexHandle, RectCornerHandle, MidpointHandle, CurveControlHandle). | PyQt6 silently drops the mouse grab on such children between events; the view re-grabs only for listed types. | The handle gets the press but no move events — drag dead on arrival (bit US-B9 in manual test round 1). | Manual testing; the tuple's comment in canvas_view.py. |
| 18 | Teardown-safe chatty-signal slots: any scene.changed slot that starts a QTimer wraps .start() in contextlib.suppress(RuntimeError) (never a bare lambda: timer.start()); any slot on an app-global signal (focusChanged, …) must survive firing after its widget died. Sidebar lists store item ids, resolve via scene.find_item_by_id at click time, and defer scene mutations with QTimer.singleShot(0, ...) (#212, #230, #235). | scene.changed fires during teardown after child QTimers are deleted → Fatal Python error: Aborted (interpreter abort, CI-killing). | Whole test suite aborts mid-run; "selection works only sometimes". | §11.4 entries; canvas_view._on_scene_changed_for_soil et al. as the pattern. |
