From 3ecc93d654ba48a108aa3134b2a4730cb89f400a Mon Sep 17 00:00:00 2001 From: CST-100 <42722690+CST-100@users.noreply.github.com> Date: Fri, 12 Jun 2026 01:54:21 -0700 Subject: [PATCH 01/79] =?UTF-8?q?Amendment=205:=20tables=20are=20the=20str?= =?UTF-8?q?ucture=20=E2=80=94=20state-dependent=20sections?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Master's structure, devel's register: the local optimum the redesign was hunting. Data renders in tables — detail rows for facts, headed data-tables for collections. A section's structure follows population: empty stays the one-line rule (ok.empty_line, reimplemented to emit the ledger row it always described), populated promotes to the panel grammar at ledger pitch (panel-ledger: header bar carrying the count and its flags in the value slot, + ADD at the edge, body a headed data-table at the parts-list 5px pitch). - Identity readout: the wrapping mono line becomes a part-identity detail table — spelled-out labels (CATEGORY, TRACKING, REORDER…), full-ISO tooltips on CREATED/UPDATED/ACTIVATED, absence as a muted dash in a row that always renders. DESCRIPTION joins as a row. - Columns regrouped: left is identity + DESIGN (REQUIREMENTS, TESTS); right is STOCK & USE (BOM — moved per amendment, STOCK, PO LINES, SUPPLIERS, WHERE USED, PROCEDURE USE, CONSUMED last — the one unbounded section). Zone captions are two nouns over the columns; the grid is a bounded identity rail plus a flexible collection track the wide tables actually spend. BOM regains its NOTES column, dropped silently by Amendment 1. - Stock carries its rung: the on-hand figure renders at 1.125rem/600 in the STOCK bar, with a derived LOW flag (coalesce(total, 0) < reorder, matching the parts-list derivation) on both the populated bar and the empty row. + ADD opens an inline receive micro-form (qty/location/lot posting to the inventory API); FULL RECORD remains the long form. - Contrast floor: --text-muted #666 (3.44:1, below AA at ledger sizes) becomes #7a7a7a (4.61:1); interactive elements never render muted — the [+] was dimmest exactly when it was the only action. One pitch for all ledger rows. - Labels spelled out (REQUIREMENTS, WHERE USED, PROCEDURE USE, RECEIVED); PRINT LABEL returns (the compression rode Amendment 1's register pass with no rationale of its own). The tier control degrades to a select above five configured tiers — tiers are project config, not a fixed trio. The tag macro drops its never-rendered activated_at parameter. - Style guide: the state-dependent section is documented, empty_line is the empty-state primitive, id-cell examples marked legacy (rule 7). CLAUDE.md: empty-state rule rewritten to name the promotion, plus a new layer rule — structure (panels, header bars, headed tables, grids) may not be deleted by a register pass; register (pitch, accent budget, voice, contrast ladder) is where density lives. Produced by a 22-agent design review (4 readers, 5 critics, adversarial verification of every recommendation against the amendment lineage) and verified by a 21-agent audit (conformance, voice, regression, consistency); all confirmed findings fixed. 578 tests pass; runtime smoke on seeded data confirmed all three population states, the receive flow end-to-end, and LOW derivation parity with the parts list. Co-Authored-By: Claude Fable 5 --- CLAUDE.md | 6 +- src/opal/web/static/css/main.css | 152 ++++++---- src/opal/web/templates/opalkit/_macros.html | 15 +- .../templates/opalkit/styleguide/index.html | 79 ++++- src/opal/web/templates/parts/_ledger_is.html | 95 ------ .../templates/parts/_ledger_provenance.html | 123 -------- src/opal/web/templates/parts/_part_form.html | 21 +- .../web/templates/parts/_section_design.html | 101 +++++++ .../web/templates/parts/_section_stock.html | 280 ++++++++++++++++++ src/opal/web/templates/parts/_tag.html | 2 +- src/opal/web/templates/parts/detail.html | 93 ++++-- src/opal/web/templates/parts/label.html | 1 - tests/unit/test_part_lifecycle.py | 19 +- tests/unit/test_se_dashboard.py | 4 +- 14 files changed, 659 insertions(+), 332 deletions(-) delete mode 100644 src/opal/web/templates/parts/_ledger_is.html delete mode 100644 src/opal/web/templates/parts/_ledger_provenance.html create mode 100644 src/opal/web/templates/parts/_section_design.html create mode 100644 src/opal/web/templates/parts/_section_stock.html diff --git a/CLAUDE.md b/CLAUDE.md index 2543064..77b4519 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -87,9 +87,11 @@ Dense, explicit, functional. Expose state and inner workings. Data tables over c **One fact, one home.** Every other appearance is a live reference, never a copy. Test: if updating something requires touching two places, the design is wrong — delete one occurrence or derive it. PRs that violate this must argue against it by name. -**The empty-state rule.** An empty section is one line, never a box: `{Section} — none · [+ action]`, muted text, 0.5-border row (`ok.empty_line` macro). A section earns vertical space only when populated. Empty states are facts, not features; six boxes announcing nothingness is anti-density. +**The empty-state rule.** An empty section is one line, never a box: `LABEL — [+]` (`ok.empty_line` macro) — label secondary, dash muted, half-strength rule, add affordance never muted. A section earns vertical space only when populated: it then promotes to the panel grammar at ledger pitch (`panel-ledger` — header bar carrying the count and its flags in the value slot, body a headed `data-table`). Empty states are facts, not features; six boxes announcing nothingness is anti-density. -**Voice rules.** Spec prose is rationale for the implementer, never interface copy. (1) No interface copy that explains the interface — absent features are not apologized for. (2) Consequence sentences live only in confirmation dialogs and errors; each such sentence has exactly one home. (3) Labels are nouns, values are facts — no clauses, no narration. (4) Nothing hides behind disclosure: meta fields render always, as dense mono readouts. (5) Width is an information budget — max-width the content or fill the viewport with columns of data, never one stretched sparse column. (6) Context pre-fill: the form never asks what the invoking context already knows. (7) Database ids never render in lists, headers, or titles. +**Structure and register are separate layers.** Structure — panels, header bars, column-headed tables, full-width grids — is the app's shared grammar and may not be deleted by a register pass. Register — row pitch, accent budget, voice, contrast ladder — is where density lives. Data renders in tables: detail rows (`ok.detail_row`) for facts, headed `data-table`s for collections; a register amendment tightens a table's pitch, it does not dissolve the table. The parts list and part page are the reference implementations. + +**Voice rules.** Spec prose is rationale for the implementer, never interface copy. (1) No interface copy that explains the interface — absent features are not apologized for. (2) Consequence sentences live only in confirmation dialogs and errors; each such sentence has exactly one home. (3) Labels are nouns, values are facts — no clauses, no narration. (4) Nothing hides behind disclosure: meta fields render always, as detail-row tables; absence is a muted dash, never an omitted row. (5) Width is an information budget — max-width the content or fill the viewport with columns of data, never one stretched sparse column. (6) Context pre-fill: the form never asks what the invoking context already knows. (7) Database ids never render in lists, headers, or titles. ## Linting (Ruff) diff --git a/src/opal/web/static/css/main.css b/src/opal/web/static/css/main.css index 71e3784..0ad0164 100644 --- a/src/opal/web/static/css/main.css +++ b/src/opal/web/static/css/main.css @@ -30,7 +30,7 @@ --text-primary: #e0e0e0; --text-secondary: #888; - --text-muted: #666; + --text-muted: #7a7a7a; /* AA floor: 4.61:1 on bg-primary at small sizes */ --accent-blue: #4a9eff; --accent-green: #4ade80; @@ -2553,10 +2553,9 @@ details.disclosure > summary { details.disclosure > summary::-webkit-details-marker { display: none; } details.disclosure[open] > summary { color: var(--text-secondary); } -/* The ledger — part page. An empty section is one line, never a box. - Grouping is carried by column placement: design-side left, world-side - right. Width is an information budget — never one stretched sparse - column. */ +/* The part page — data renders in tables: detail rows for facts, headed + tables for collections. An empty section is one line, never a box; a + populated section promotes to the panel grammar at ledger pitch. */ .part-actions { display: flex; align-items: center; @@ -2569,50 +2568,33 @@ details.disclosure[open] > summary { color: var(--text-secondary); } margin-left: auto; } -.empty-line { - color: var(--text-muted); - font-size: 0.8rem; - padding: var(--space-xs) 2px; - border-bottom: 1px solid var(--border-color); -} - -@supports (color: color-mix(in srgb, red 50%, transparent)) { - .empty-line { - border-bottom-color: color-mix(in srgb, var(--border-color) 50%, transparent); - } -} - -.empty-line a { - color: var(--text-secondary); - text-decoration: none; +/* Identity readout table — every meta fact, always rendered */ +.part-identity { + margin: var(--space-sm) 0 var(--space-md); } -.part-description { - margin: var(--space-md) 0; - max-width: 80ch; +.part-identity th { + width: 130px; } -/* Dense mono readout — labels are nouns, values are facts */ -.part-readout { +/* Zone captions — two nouns, no boxes, no counts */ +.zone-caption { font-family: var(--font-mono); font-size: 0.75rem; - color: var(--text-primary); - margin: var(--space-sm) 0 var(--space-md); - line-height: 1.7; -} - -.part-readout .ro-label { + letter-spacing: 0.1em; color: var(--text-muted); + border-top: 2px solid var(--border-color); + padding-top: var(--space-xs); + margin: var(--space-md) 0 var(--space-sm); } -/* Two-column part page — each column a bounded information budget; - row actions sit at the column edge, adjacent to content */ +/* Two-column part page — identity rail bounded, collection tables get + the viewport width their columns spend */ .part-layout { display: grid; - grid-template-columns: repeat(2, minmax(0, 640px)); + grid-template-columns: minmax(420px, 560px) minmax(0, 1fr); gap: var(--space-lg); align-items: start; - justify-content: center; } @media (max-width: 1000px) { @@ -2621,6 +2603,8 @@ details.disclosure[open] > summary { color: var(--text-secondary); } } } +/* Empty section — one ruled line at half strength; the dash is the only + muted element, the add affordance never is */ .ledger-row { display: flex; align-items: baseline; @@ -2631,14 +2615,10 @@ details.disclosure[open] > summary { color: var(--text-secondary); } font-size: 0.8rem; } -.ledger-row-empty { - padding: 2px 2px; -} - -.ledger-row-empty .ledger-label, -.ledger-row-empty .ledger-value, -.ledger-row-empty .ledger-add { - color: var(--text-muted); +@supports (color: color-mix(in srgb, red 50%, transparent)) { + .ledger-row-empty { + border-bottom-color: color-mix(in srgb, var(--border-color) 50%, transparent); + } } .ledger-row .ledger-label { @@ -2652,49 +2632,95 @@ details.disclosure[open] > summary { color: var(--text-secondary); } color: var(--text-primary); } +.ledger-row-empty .ledger-value { + color: var(--text-muted); +} + .ledger-flag { color: var(--status-warn); } -.ledger-row .ledger-add { +.ledger-add { color: var(--text-secondary); text-decoration: none; + padding: 2px 4px; } -.ledger-detail { - font-family: var(--font-mono); - font-size: 0.8rem; +.ledger-add:hover { color: var(--text-primary); - padding: var(--space-xs) 0 var(--space-xs) var(--space-md); - border-bottom: 1px solid var(--border-color); } -.ledger-detail-line { - display: flex; - align-items: baseline; - gap: var(--space-sm); - padding: 1px 0; +/* Populated section — the app's panel grammar at ledger pitch */ +.panel-ledger { + margin-bottom: var(--space-md); } -.ledger-detail-line .ldl-muted { - color: var(--text-muted); +.panel-ledger .panel-header { + padding: 4px var(--space-sm); + font-size: 0.8rem; } -.ledger-detail-line .ldl-actions { +.panel-ledger .header-count { margin-left: auto; - display: flex; - gap: var(--space-xs); + margin-right: var(--space-sm); + font-weight: 400; + color: var(--text-primary); } -.ledger-detail-line .ldl-actions a { - color: var(--text-muted); +.panel-ledger .header-count:last-child { + margin-right: 0; +} + +.panel-ledger .stock-figure { + font-size: 1.125rem; + font-weight: 600; +} + +.panel-ledger .data-table { + font-size: 0.8rem; +} + +.panel-ledger .data-table th, +.panel-ledger .data-table td { + padding: 5px 8px; +} + +.panel-ledger .data-table .row-actions { + white-space: nowrap; + text-align: right; +} + +.panel-ledger .data-table .row-actions a { + color: var(--text-secondary); text-decoration: none; + padding: 2px 6px; } -.ledger-detail-line .ldl-actions a:hover { +.panel-ledger .data-table .row-actions a:hover { color: var(--text-primary); } +.panel-ledger .data-table .row-actions a.act-remove:hover { + color: var(--accent-red); +} + +.panel-ledger .bom-tree { + font-family: var(--font-mono); + font-size: 0.8rem; + padding: var(--space-xs) var(--space-sm); + border-top: 1px solid var(--border-color); +} + +/* Inline add rows (assign requirement, receive stock) */ +.inline-add-row { + display: none; + gap: var(--space-sm); + align-items: center; + padding: var(--space-xs) 2px; + font-family: var(--font-mono); + font-size: 0.8rem; +} + /* Parts list — one line per row, pitch matched to the part-page ledger. Accent color budget: PN only; names render primary. */ .parts-list td, diff --git a/src/opal/web/templates/opalkit/_macros.html b/src/opal/web/templates/opalkit/_macros.html index 4fca3fa..ff3a3d7 100644 --- a/src/opal/web/templates/opalkit/_macros.html +++ b/src/opal/web/templates/opalkit/_macros.html @@ -145,10 +145,17 @@ {{ ok.empty("No inventory records", action_text="Add Inventory", action_href="/inventory/new") }} #} {# The empty-state rule: an empty section is one line, never a box. - "{Section} — none · [+ action]", muted, 0.5-border row. A section earns - vertical space only when populated. #} -{% macro empty_line(label, action_text=none, href=none, onclick=none) %} -
{{ label }} — none{% if action_text %} · [+ {{ action_text }}]{% endif %}
+ "LABEL — [+]": label secondary, dash muted, half-strength rule, + add affordance never muted. A section earns vertical space (a panel + with a table) only when populated. #} +{% macro empty_line(label, href=none, onclick=none) %} +
+ {{ label }} + — + {%- if href or onclick %} + [+] + {%- endif %} +
{% endmacro %} diff --git a/src/opal/web/templates/opalkit/styleguide/index.html b/src/opal/web/templates/opalkit/styleguide/index.html index 1f45bff..62948fb 100644 --- a/src/opal/web/templates/opalkit/styleguide/index.html +++ b/src/opal/web/templates/opalkit/styleguide/index.html @@ -75,7 +75,7 @@

OPALk .text-muted - ID Cell + ID Cell (legacy — ids never render in lists) 42 .id-cell .mono @@ -196,11 +196,11 @@

OPALk mono (colored) - {{ ok.mono("42", color="orange") }} - {% raw %}{{ ok.mono(part.id, color="orange") }}{% endraw %} + {{ ok.mono("RV1-F-0001", color="orange") }} + {% raw %}{{ ok.mono(part.internal_pn, color="orange") }}{% endraw %} - id_cell + id_cell (legacy — ids never render in lists; link the PN or designator) {{ ok.id_cell("42", "#") }} {% raw %}{{ ok.id_cell(part.id, "/parts/" ~ part.id) }}{% endraw %} @@ -284,12 +284,75 @@

OPALk EMPTY STATE
-
- {{ ok.empty("No items found", action_text="Add Item", action_href="#") }} +

+ An empty section is one line, never a box: label secondary, dash muted, + half-strength rule. The add affordance never renders at muted. +

+
+ {{ ok.empty_line("BOM", href="#") }} + {{ ok.empty_line("PO LINES") }}
+
+
{% raw %}{{ ok.empty_line("BOM", onclick="showAddComponent()") }}
+{{ ok.empty_line("PO LINES") }}{% endraw %}
+
+

+ ok.empty is legacy — full-page zero states only, never sections. +

-
{% raw %}{{ ok.empty("No parts found") }}
-{{ ok.empty("No items", action_text="Add Item", action_href="/items/new") }}{% endraw %}
+
{% raw %}{{ ok.empty("No parts found") }}{% endraw %}
+
+
+
+ + +
+
+ STATE-DEPENDENT SECTION (PANEL-LEDGER) +
+
+

+ A section's structure follows its population. Empty: the one-line empty + state. Populated: a panel at ledger pitch — header bar with the count + (and its flags) in the value slot, body a headed data table. +

+ {{ ok.empty_line("TESTS", href="#") }} +
+
+ STOCK + 42 EA · 3 REC · LOW +
+ ADD
+
+ + + + {{ ok.th("OPAL #", width="120px") }} + {{ ok.th("LOCATION") }} + {{ ok.th("QTY", width="70px", align="right") }} + + + + + + + + + +
{{ ok.link("OPAL-00038", "#", mono=true) }}STORE-E26
+
+
+
{% raw %}{% if records %}
+

+    

+        STOCK
+        …count · flags…
+        
+ ADD

+    

+    …

+

+{% else %}
+{{ ok.empty_line("STOCK", onclick="…") }}
+{% endif %}{% endraw %}
diff --git a/src/opal/web/templates/parts/_ledger_is.html b/src/opal/web/templates/parts/_ledger_is.html deleted file mode 100644 index d361b84..0000000 --- a/src/opal/web/templates/parts/_ledger_is.html +++ /dev/null @@ -1,95 +0,0 @@ -{% import 'opalkit/_macros.html' as ok %} -{# What this part IS — BOM, requirements, tests. LABEL left, count/value - right, [+] inline; populated sections expand with indented mono detail - lines. Order is fixed and global; emphasis is carried by weight only. #} - -{# --- BOM --- #} -
- BOM - {{ bom_lines | length if bom_lines else '—' }} - [+] -
-{% if bom_lines %} -
- {% for line in bom_lines %} -
- {{ line.component.internal_pn }} - {{ line.component.name }} - ×{{ line.quantity }}{% if line.reference_designator %} [{{ line.reference_designator }}]{% endif %} - - ✎ - × - -
- {% endfor %} - -
-{% endif %} - -{# --- Requirements: counts carry state inline, tersely --- #} -{% set req_rows = part_requirements | selectattr('req') | list %} -{% set tbd_count = req_rows | selectattr('req.tbd') | list | length %} -{% set tbr_count = req_rows | selectattr('req.tbr') | list | length %} -{% set stale_count = req_rows | selectattr('req.stale') | list | length %} -
- REQS - - {{- part_requirements | length if part_requirements else '—' -}} - {%- if tbd_count %} · {{ tbd_count }} TBD{% endif -%} - {%- if tbr_count %} · {{ tbr_count }} TBR{% endif -%} - {%- if stale_count %} · {{ stale_count }} STALE{% endif -%} - - [+] -
- -{% if part_requirements %} -
- {% for row in part_requirements %} - {% set r = row.req %} - {% set a = row.allocation %} -
- {% if r %} - {{ r.req_number }} - {{ r.lifecycle_state | upper }} - {{ r.title }} - {% if r.tbd %}TBD{% endif %} - {% if r.tbr %}TBR{% endif %} - {% if r.stale %}≠ STALE{% endif %} - {% else %} - {{ a.requirement_id }} - {% endif %} - {{ a.status | upper | replace('_', ' ') }} - - × - -
- {% endfor %} -
-{% endif %} - -{# --- Test templates --- #} -
- TESTS - {{ test_templates | length if test_templates else '—' }} - [+] -
-{% if test_templates %} -
- {% for tt in test_templates %} -
- {{ tt.name }} - {{ tt.test_type | upper }}{% if tt.required %} REQ{% endif %} - {% if tt.test_type == 'numeric' %} - {{ '%g' % tt.min_value if tt.min_value is not none else '' }}–{{ '%g' % tt.max_value if tt.max_value is not none else '' }}{{ ' ' ~ tt.unit if tt.unit else '' }} - {% endif %} - - × - -
- {% endfor %} -
-{% endif %} diff --git a/src/opal/web/templates/parts/_ledger_provenance.html b/src/opal/web/templates/parts/_ledger_provenance.html deleted file mode 100644 index 1344ce3..0000000 --- a/src/opal/web/templates/parts/_ledger_provenance.html +++ /dev/null @@ -1,123 +0,0 @@ -{% import 'opalkit/_macros.html' as ok %} -{# Where it came from / where used / how many exist — stock, purchasing, - suppliers, consumption, where-used, procedure usage. Order is fixed - and global; emphasis is carried by weight only. #} - -{# --- Stock + inventory records, merged --- #} -
- STOCK - - {%- if inventory_records -%} - {{ '%g' % part.total_quantity }} {{ (part.unit_of_measure or 'EA') | upper }} · {{ inventory_records | length }} REC - {%- else -%}—{%- endif -%} - - [+] -
-{% if inventory_records %} -
- {% for inv in inventory_records %} -
- {{ inv.opal_number }} - {{ inv.location or '—' }} - ×{{ '%g' % inv.quantity }}{% if inv.lot_number %} · {{ inv.lot_number }}{% endif %} -
- {% endfor %} -
-{% endif %} - -{# --- Open PO lines --- #} -
- PO LINES - {{ open_po_lines | length if open_po_lines else '—' }} -
-{% if open_po_lines %} -
- {% for line in open_po_lines %} -
- {{ line.purchase.reference or 'PO-' ~ line.purchase_id }} - {{ (line.purchase.status.value if line.purchase.status.value is defined else line.purchase.status) | upper }} · {{ '%g' % line.qty_received }}/{{ '%g' % line.qty_ordered }} -
- {% endfor %} -
-{% endif %} - -{# --- Suppliers --- #} -
- SUPPLIERS - {{ supplier_entries | length if supplier_entries else '—' }} - [+] -
-{% if supplier_entries %} -
- {% for entry in supplier_entries %} -
- {{ entry.supplier.name }} - {{ entry.vendor_pn }}{% if entry.is_preferred %} · PREF{% endif %} - - × - -
- {% endfor %} -
-{% endif %} - -{# --- Consumption history --- #} -
- CONSUMED - {{ (consumption_history | length) ~ ('+' if consumption_history | length == 50 else '') if consumption_history else '—' }} -
-{% if consumption_history %} -
- {% for c in consumption_history %} -
- {{ c.created_at.date().isoformat() }} - ×{{ '%g' % c.quantity }} - {% if c.procedure_instance_id %} - {{ c.procedure_instance.work_order_number if c.procedure_instance and c.procedure_instance.work_order_number else 'EXEC' }} - {% else %} - MANUAL - {% endif %} - {% if c.notes %}{{ c.notes }}{% endif %} -
- {% endfor %} -
-{% endif %} - -{# --- Where used --- #} -
- WHERE-USED - {{ where_used | length if where_used else '—' }} -
-{% if where_used %} -
- {% for line in where_used %} -
- {{ line.assembly.internal_pn }} - {{ line.assembly.name }} - ×{{ line.quantity }}{% if line.reference_designator %} [{{ line.reference_designator }}]{% endif %} -
- {% endfor %} -
-{% endif %} - -{# --- Procedure usage (kits reference the design, not the stock) --- #} -
- PROC USE - {{ (kit_usages | length) + (step_kit_usages | length) if kit_usages or step_kit_usages else '—' }} -
-{% if kit_usages or step_kit_usages %} -
- {% for kit in kit_usages %} -
- {{ kit.procedure.name }} - ×{{ '%g' % kit.quantity_required }} · {{ (kit.procedure.status.value if kit.procedure.status.value is defined else kit.procedure.status) | upper }} -
- {% endfor %} - {% for sk in step_kit_usages %} -
- {{ sk.step.procedure.name }} - {{ sk.step.step_number }} · ×{{ '%g' % sk.quantity_required }} · {{ (sk.usage_type.value if sk.usage_type.value is defined else sk.usage_type) | upper }} -
- {% endfor %} -
-{% endif %} diff --git a/src/opal/web/templates/parts/_part_form.html b/src/opal/web/templates/parts/_part_form.html index d6b372a..7075d18 100644 --- a/src/opal/web/templates/parts/_part_form.html +++ b/src/opal/web/templates/parts/_part_form.html @@ -26,7 +26,6 @@ uom=form_part.unit_of_measure, tracking=form_part.tracking_type.value if form_part.tracking_type else '', state=form_part.lifecycle_state, - activated_at=form_part.activated_at, qr_url='/api/parts/' ~ form_part.id ~ '/qrcode', pn_segments=pn_segments) }} {% else %} @@ -38,6 +37,19 @@ {# Identity rows — absent (not disabled) once active #}
+ {# Tiers are project config, not a fixed trio — the + segmented control holds to five, then a select #} + {% if tiers | length > 5 %} + + {% else %}
{% for tier in tiers %}
+ {% endif %}
@@ -194,8 +207,10 @@ async function pfSelectTier(tier, btn) { pfTier = tier; - document.querySelectorAll('#pf-tier-segments button').forEach(b => b.classList.remove('active')); - btn.classList.add('active'); + if (btn) { + document.querySelectorAll('#pf-tier-segments button').forEach(b => b.classList.remove('active')); + btn.classList.add('active'); + } try { const r = await fetch('/api/parts/next-pn?tier=' + tier); if (!r.ok) return; diff --git a/src/opal/web/templates/parts/_section_design.html b/src/opal/web/templates/parts/_section_design.html new file mode 100644 index 0000000..d403637 --- /dev/null +++ b/src/opal/web/templates/parts/_section_design.html @@ -0,0 +1,101 @@ +{% import 'opalkit/_macros.html' as ok %} +{# DESIGN — requirements and tests. Empty: one line. Populated: panel + + headed table at ledger pitch; the count and its flags live in the + header's value slot. #} + +{# --- Requirements --- #} +{% set req_rows = part_requirements | selectattr('req') | list %} +{% set tbd_count = req_rows | selectattr('req.tbd') | list | length %} +{% set tbr_count = req_rows | selectattr('req.tbr') | list | length %} +{% set stale_count = req_rows | selectattr('req.stale') | list | length %} +{% if part_requirements %} +
+
+ REQUIREMENTS + + {{- part_requirements | length -}} + {%- if tbd_count %} · {{ tbd_count }} TBD{% endif -%} + {%- if tbr_count %} · {{ tbr_count }} TBR{% endif -%} + {%- if stale_count %} · {{ stale_count }} STALE{% endif -%} + +
+ + ADD +
+
+ + + + {{ ok.th("REQ", width="110px") }} + {{ ok.th("STATE", width="100px") }} + {{ ok.th("TITLE") }} + {{ ok.th("ALLOCATION", width="120px") }} + {{ ok.th("", width="40px") }} + + + + {% for row in part_requirements %} + {% set r = row.req %} + {% set a = row.allocation %} + + {% if r %} + + + + {% else %} + + + + {% endif %} + + + + {% endfor %} + +
{{ r.req_number }}{{ r.lifecycle_state | upper }}{{ r.title }} + {%- if r.tbd %} TBD{% endif %} + {%- if r.tbr %} TBR{% endif %} + {%- if r.stale %} ≠ STALE{% endif %}{{ a.requirement_id }}——{{ a.status | upper | replace('_', ' ') }}×
+
+{% else %} +{{ ok.empty_line("REQUIREMENTS", onclick="showAssignRequirement()") }} +{% endif %} +
+ + {{ ok.btn("ASSIGN", variant="primary", size="sm", attrs='onclick="assignRequirement()"') }} +
+ +{# --- Test templates --- #} +{% if test_templates %} +
+
+ TESTS + {{ test_templates | length }} +
+ + ADD +
+
+ + + + {{ ok.th("NAME") }} + {{ ok.th("TYPE", width="120px") }} + {{ ok.th("RANGE", width="140px") }} + {{ ok.th("", width="40px") }} + + + + {% for tt in test_templates %} + + + + + + + {% endfor %} + +
{{ tt.name }}{{ tt.test_type | upper }}{% if tt.required %} · REQ{% endif %}{% if tt.test_type == 'numeric' %}{{ '%g' % tt.min_value if tt.min_value is not none else '' }}–{{ '%g' % tt.max_value if tt.max_value is not none else '' }}{{ ' ' ~ tt.unit if tt.unit else '' }}{% else %}—{% endif %}×
+
+{% else %} +{{ ok.empty_line("TESTS", onclick="showAddTest()") }} +{% endif %} diff --git a/src/opal/web/templates/parts/_section_stock.html b/src/opal/web/templates/parts/_section_stock.html new file mode 100644 index 0000000..22eabda --- /dev/null +++ b/src/opal/web/templates/parts/_section_stock.html @@ -0,0 +1,280 @@ +{% import 'opalkit/_macros.html' as ok %} +{# STOCK & USE — composition, stock, purchasing, suppliers, usage, and the + consumption log (last: it is the one unbounded section). Empty: one + line. Populated: panel + headed table at ledger pitch. #} + +{# --- BOM --- #} +{% if bom_lines %} +
+
+ BOM + {{ bom_lines | length }} +
+ + ADD +
+
+ + + + {{ ok.th("PN", width="130px") }} + {{ ok.th("NAME") }} + {{ ok.th("QTY", width="60px", align="right") }} + {{ ok.th("REFDES", width="90px") }} + {{ ok.th("NOTES") }} + {{ ok.th("", width="64px") }} + + + + {% for line in bom_lines %} + + + + + + + + + {% endfor %} + +
{{ line.component.internal_pn }}{{ line.component.name }}{{ line.quantity }}{{ line.reference_designator or '—' }}{{ line.notes or '—' }} + ✎ + × +
+ {# Tree renders only when it carries depth the direct lines don't #} + +
+{% else %} +{{ ok.empty_line("BOM", onclick="showAddComponent()") }} +{% endif %} + +{# --- Stock — the on-hand figure is the section's value; LOW derives + from the reorder point #} +{% if inventory_records %} +
+
+ STOCK + + {{ '%g' % part.total_quantity }} {{ (part.unit_of_measure or 'EA') | upper }} + · {{ inventory_records | length }} REC + {%- if part.reorder_point is not none and part.total_quantity < part.reorder_point %} · LOW{% endif %} + +
+ + ADD +
+
+ + + + {{ ok.th("OPAL #", width="120px") }} + {{ ok.th("LOCATION") }} + {{ ok.th("QTY", width="70px", align="right") }} + {{ ok.th("LOT", width="140px") }} + + + + {% for inv in inventory_records %} + + + + + + + {% endfor %} + +
{{ inv.opal_number }}{{ inv.location or '—' }}{{ '%g' % inv.quantity }}{{ inv.lot_number or '—' }}
+
+{% else %} +{# Inlined empty row: LOW derives the same way the parts list derives it + (coalesce(total, 0) < reorder) — zero records is the most-low state #} +
+ STOCK + —{% if part.reorder_point is not none and (part.total_quantity or 0) < part.reorder_point %} · LOW{% endif %} + [+] +
+{% endif %} +
+ + + + {{ ok.btn("RECEIVE", variant="primary", size="sm", attrs='onclick="submitReceiveStock()"') }} + FULL RECORD + +
+ +{# --- Open PO lines --- #} +{% if open_po_lines %} +
+
+ PO LINES + {{ open_po_lines | length }} +
+ + + + {{ ok.th("PO", width="120px") }} + {{ ok.th("STATUS") }} + {{ ok.th("RECEIVED", width="100px", align="right") }} + + + + {% for line in open_po_lines %} + + + + + + {% endfor %} + +
{{ line.purchase.reference or 'PO-' ~ line.purchase_id }}{{ (line.purchase.status.value if line.purchase.status.value is defined else line.purchase.status) | upper }}{{ '%g' % line.qty_received }}/{{ '%g' % line.qty_ordered }}
+
+{% else %} +{{ ok.empty_line("PO LINES") }} +{% endif %} + +{# --- Suppliers --- #} +{% if supplier_entries %} +
+
+ SUPPLIERS + {{ supplier_entries | length }} +
+ + ADD +
+
+ + + + {{ ok.th("SUPPLIER") }} + {{ ok.th("VENDOR PN", width="140px") }} + {{ ok.th("PREFERRED", width="100px") }} + {{ ok.th("", width="40px") }} + + + + {% for entry in supplier_entries %} + + + + + + + {% endfor %} + +
{{ entry.supplier.name }}{{ entry.vendor_pn }}{{ '✓' if entry.is_preferred else '—' }}×
+
+{% else %} +{{ ok.empty_line("SUPPLIERS", onclick="showAddSupplier()") }} +{% endif %} + +{# --- Where used --- #} +{% if where_used %} +
+
+ WHERE USED + {{ where_used | length }} +
+ + + + {{ ok.th("PN", width="130px") }} + {{ ok.th("NAME") }} + {{ ok.th("QTY", width="60px", align="right") }} + {{ ok.th("REFDES", width="90px") }} + + + + {% for line in where_used %} + + + + + + + {% endfor %} + +
{{ line.assembly.internal_pn }}{{ line.assembly.name }}{{ line.quantity }}{{ line.reference_designator or '—' }}
+
+{% else %} +{{ ok.empty_line("WHERE USED") }} +{% endif %} + +{# --- Procedure usage (kits reference the design, not the stock) --- #} +{% if kit_usages or step_kit_usages %} +
+
+ PROCEDURE USE + {{ (kit_usages | length) + (step_kit_usages | length) }} +
+ + + + {{ ok.th("PROCEDURE") }} + {{ ok.th("STEP", width="60px", align="right") }} + {{ ok.th("QTY", width="60px", align="right") }} + {{ ok.th("USAGE", width="100px") }} + {{ ok.th("STATUS", width="110px") }} + + + + {% for kit in kit_usages %} + + + + + + + + {% endfor %} + {% for sk in step_kit_usages %} + + + + + + + + {% endfor %} + +
{{ kit.procedure.name }}—{{ '%g' % kit.quantity_required }}—{{ (kit.procedure.status.value if kit.procedure.status.value is defined else kit.procedure.status) | upper }}
{{ sk.step.procedure.name }}{{ sk.step.step_number }}{{ '%g' % sk.quantity_required }}{{ (sk.usage_type.value if sk.usage_type.value is defined else sk.usage_type) | upper }}{{ (sk.step.procedure.status.value if sk.step.procedure.status.value is defined else sk.step.procedure.status) | upper }}
+
+{% else %} +{{ ok.empty_line("PROCEDURE USE") }} +{% endif %} + +{# --- Consumption history --- #} +{% if consumption_history %} +
+
+ CONSUMED + {{ consumption_history | length }}{{ '+' if consumption_history | length == 50 }} +
+ + + + {{ ok.th("DATE", width="110px") }} + {{ ok.th("QTY", width="60px", align="right") }} + {{ ok.th("SOURCE", width="140px") }} + {{ ok.th("NOTES") }} + + + + {% for c in consumption_history %} + + + + + + + {% endfor %} + +
{{ c.created_at.date().isoformat() }}{{ '%g' % c.quantity }} + {% if c.procedure_instance_id %} + {{ c.procedure_instance.work_order_number if c.procedure_instance and c.procedure_instance.work_order_number else 'EXEC' }} + {% else %} + MANUAL + {% endif %} + {{ c.notes or '—' }}
+
+{% else %} +{{ ok.empty_line("CONSUMED") }} +{% endif %} diff --git a/src/opal/web/templates/parts/_tag.html b/src/opal/web/templates/parts/_tag.html index 5433c42..d0239b1 100644 --- a/src/opal/web/templates/parts/_tag.html +++ b/src/opal/web/templates/parts/_tag.html @@ -4,7 +4,7 @@ differ, that is a bug by definition. Labels are nouns, values are facts — the tag narrates nothing. #} {% macro part_tag(mode, pn=none, name=none, tier_level=none, tier_name=none, - uom="EA", tracking="", state="draft", activated_at=none, + uom="EA", tracking="", state="draft", qr_url=none, pn_segments=none) %} {% set trk = 'SER' if tracking == 'serialized' else ('BULK' if tracking == 'bulk' else tracking | upper) %}
diff --git a/src/opal/web/templates/parts/detail.html b/src/opal/web/templates/parts/detail.html index 896ef64..e78c517 100644 --- a/src/opal/web/templates/parts/detail.html +++ b/src/opal/web/templates/parts/detail.html @@ -11,11 +11,12 @@ {% block content %} {% set is_draft = part.lifecycle_state == 'draft' %} -{# ============ THE PAGE — left: what this part IS (tag · actions · desc · - readout · BOM · REQS · TESTS); right: where it came from / where used / - how many exist (STOCK · PO LINES · SUPPLIERS · CONSUMED · WHERE-USED · - PROC USE). Order is fixed and global; emphasis is weight, not position. - Stacks IS-first under 1000px. ============ #} +{# ============ THE PAGE — left: identity (tag · actions · identity table) + + DESIGN (REQUIREMENTS · TESTS); right: STOCK & USE (BOM · STOCK · + PO LINES · SUPPLIERS · WHERE USED · PROCEDURE USE · CONSUMED). Order is + fixed and global. Data renders in tables: detail rows for facts, headed + tables for collections; absence is one line. Stacks identity-first + under 1000px. ============ #}
{{ part_tag('header', @@ -26,12 +27,11 @@ uom=part.unit_of_measure, tracking=part.tracking_type.value if part.tracking_type else '', state=part.lifecycle_state, - activated_at=part.activated_at, qr_url='/api/parts/' ~ part.id ~ '/qrcode', pn_segments=pn_segments) }}
- {{ ok.btn("PRINT", attrs='onclick="window.open(\'/label?type=part&id=' ~ part.id ~ '\', \'_blank\', \'width=600,height=350\')"') }} + {{ ok.btn("PRINT LABEL", attrs='onclick="window.open(\'/label?type=part&id=' ~ part.id ~ '\', \'_blank\', \'width=600,height=350\')"') }} {{ ok.btn("EDIT", attrs='onclick="openPartForm()"') }} {% if is_draft %} {{ ok.btn("ACTIVATE", variant="primary", attrs='onclick="openActivateConfirm()"') }} @@ -62,26 +62,31 @@
{% endif %} -{# Dense readout — labels are nouns, values are facts. Edits live behind EDIT. #} -
- CAT {{ part.category | upper if part.category else '—' }} · - UOM {{ (part.unit_of_measure or 'EA') | upper }} · - TRK {{ 'SER' if part.tracking_type and part.tracking_type.value == 'serialized' else 'BULK' }} · - REORD {{ '%g' % part.reorder_point if part.reorder_point is not none else '—' }} · - TOOL {{ ('CAL ' ~ part.calibration_interval_days ~ 'D' if part.calibration_interval_days else 'YES') if part.is_tooling else '—' }} · - PARENT {% if part.parent_id %}{{ part.parent.internal_pn }}{% else %}—{% endif %} · - EXT {{ part.external_pn or '—' }} · - CRE {{ part.created_at.date().isoformat() }} · - UPD {{ part.updated_at.date().isoformat() }} - {%- if part.lifecycle_state == 'active' and part.activated_at %} · ACT {{ part.activated_at.date().isoformat() }}{% endif %} - {%- if part.description %}
DESC {{ part.description }}{% endif %} -
- -{% include 'parts/_ledger_is.html' %} +{# Identity readout — every meta fact, always rendered; absence is a + muted dash. Labels are nouns, values are facts. Edits live behind EDIT. #} + + + + + + + + + + + + + + +
CATEGORY{% if part.category %}{{ part.category | upper }}{% else %}—{% endif %}
UOM{{ (part.unit_of_measure or 'EA') | upper }}
TRACKING{{ 'SERIALIZED' if part.tracking_type and part.tracking_type.value == 'serialized' else 'BULK' }}
REORDER{% if part.reorder_point is not none %}{{ '%g' % part.reorder_point }}{% else %}—{% endif %}
TOOLING{% if part.is_tooling %}{{ 'CAL ' ~ part.calibration_interval_days ~ 'D' if part.calibration_interval_days else 'YES' }}{% else %}—{% endif %}
PARENT{% if part.parent_id %}{{ part.parent.internal_pn }}{% else %}—{% endif %}
EXTERNAL PN{% if part.external_pn %}{{ part.external_pn }}{% else %}—{% endif %}
CREATED{{ part.created_at.date().isoformat() }}
UPDATED{{ part.updated_at.date().isoformat() }}
ACTIVATED{% if part.activated_at %}{{ part.activated_at.date().isoformat() }}{% else %}—{% endif %}
DESCRIPTION{% if part.description %}{{ part.description }}{% else %}—{% endif %}
+ +
DESIGN
+{% include 'parts/_section_design.html' %}
-{% include 'parts/_ledger_provenance.html' %} +
STOCK & USE
+{% include 'parts/_section_stock.html' %}
@@ -341,6 +346,46 @@ document.getElementById('assign-req-number').focus(); } +// Inline stock receive — the quick path; FULL RECORD is the long form +function toggleReceiveStock() { + const row = document.getElementById('receive-stock-row'); + const visible = row.style.display === 'flex'; + row.style.display = visible ? 'none' : 'flex'; + if (!visible) document.getElementById('rs-qty').focus(); +} + +async function submitReceiveStock() { + const errorEl = document.getElementById('rs-error'); + errorEl.style.display = 'none'; + const qty = parseFloat(document.getElementById('rs-qty').value); + const location = document.getElementById('rs-location').value.trim(); + if (!qty || qty <= 0 || !location) { + errorEl.textContent = 'Quantity and location are required.'; + errorEl.style.display = 'inline'; + return; + } + try { + const r = await fetch('/api/inventory', { + method: 'POST', headers: getHeaders(), + body: JSON.stringify({ + part_id: partId, + quantity: qty, + location: location, + lot_number: document.getElementById('rs-lot').value.trim() || null, + }), + }); + if (r.ok) { window.location.reload(); } + else { + const err = await r.json(); + errorEl.textContent = formatApiError(err.detail, 'Failed to create inventory record'); + errorEl.style.display = 'inline'; + } + } catch (e) { + errorEl.textContent = 'Network error: ' + e.message; + errorEl.style.display = 'inline'; + } +} + // Part search component function selectPart(el) { const partId = el.dataset.partId; diff --git a/src/opal/web/templates/parts/label.html b/src/opal/web/templates/parts/label.html index 7dfd546..4783773 100644 --- a/src/opal/web/templates/parts/label.html +++ b/src/opal/web/templates/parts/label.html @@ -39,7 +39,6 @@ uom=part.unit_of_measure, tracking=part.tracking_type.value if part.tracking_type else '', state=part.lifecycle_state, - activated_at=part.activated_at, qr_url='/api/parts/' ~ part.id ~ '/qrcode', pn_segments=pn_segments) }} diff --git a/tests/unit/test_part_lifecycle.py b/tests/unit/test_part_lifecycle.py index 6bd2699..011e058 100644 --- a/tests/unit/test_part_lifecycle.py +++ b/tests/unit/test_part_lifecycle.py @@ -351,8 +351,8 @@ def test_draft_part_detail_page_renders_activate_confirm(web_client): # Ledger rows render with empty values as facts, not boxes or apologies assert "ledger-row" in page.text assert "nothing physical yet" not in page.text - # Nothing hides behind disclosure: the dense readout is always visible - assert "part-readout" in page.text + # Nothing hides behind disclosure: the identity table is always visible + assert "part-identity" in page.text assert "+ details" not in page.text @@ -400,10 +400,17 @@ def test_part_page_two_column_layout(web_client): assert page.status_code == 200 assert 'class="part-layout"' in page.text assert page.text.count('class="part-col"') == 2 - # Order is fixed and global: IS column (BOM/REQS/TESTS) renders before - # the provenance column (STOCK first), in both lifecycle states - assert page.text.index(">BOM<") < page.text.index(">STOCK<") - assert page.text.index(">TESTS<") < page.text.index(">STOCK<") + # Order is fixed and global: DESIGN (REQUIREMENTS, TESTS) in the left + # column, then STOCK & USE (BOM first, CONSUMED last) in the right — + # in both lifecycle states + assert page.text.index(">DESIGN<") < page.text.index(">STOCK & USE<") + second_col = page.text[page.text.index(">STOCK & USE<") :] + assert page.text.index(">REQUIREMENTS<") < page.text.index(">TESTS<") < page.text.index( + ">STOCK & USE<" + ) + order = [">BOM<", ">STOCK<", ">PO LINES<", ">SUPPLIERS<", ">WHERE USED<", ">PROCEDURE USE<", ">CONSUMED<"] + positions = [second_col.index(label) for label in order] + assert positions == sorted(positions) web_client.post(f"/api/parts/{part['id']}/activate", json={}) page2 = web_client.get(f"/parts/{part['id']}") assert page2.text.index(">BOM<") < page2.text.index(">STOCK<") diff --git a/tests/unit/test_se_dashboard.py b/tests/unit/test_se_dashboard.py index 9397073..3356892 100644 --- a/tests/unit/test_se_dashboard.py +++ b/tests/unit/test_se_dashboard.py @@ -114,7 +114,7 @@ def test_part_page_shows_allocated_requirement_with_state(client, db_session, te page = client.get(f"/parts/{part['id']}") assert page.status_code == 200 - assert ">REQS<" in page.text # ledger row label + assert ">REQUIREMENTS<" in page.text # section header label assert req.req_number in page.text assert "DRAFT" in page.text - assert f"/requirements/{req.id}" in page.text # detail line links to the dossier + assert f"/requirements/{req.id}" in page.text # table row links to the dossier From c2debe544901e2a3278fcd70d5c5413debdd497d Mon Sep 17 00:00:00 2001 From: CST-100 <42722690+CST-100@users.noreply.github.com> Date: Fri, 12 Jun 2026 01:56:25 -0700 Subject: [PATCH 02/79] Issues R2: nonconformance, containment, and disposition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two gates, not one: raised → UNDISPOSITIONED → (disposition signed) → DISPOSITIONED → (corrective action done) → CLOSED. The blocking predicate is `undispositioned`, never `open`; holds are derived from issues at the commitment moments (COMPLETE/START/SKIP/sign-off/close-out), never stored on the step. Schema (a7c41d92e803): containment (step|op|wo|advisory) + containment_step_id, raised_step_id/raised_by_id, disposition signature (dispositioned_by_id, dispositioned_at, disposition_rationale), is_condition→actual, MIL-STD-1520 disposition set, IssueStatus collapses to open|closed, stored ON_HOLD step rows revert to in_progress. - core/holds.py: containment semantics §2 — step blocks its COMPLETE (and its OP's), bound future step blocks START, op blocks OP COMPLETE, wo blocks WO close-out, advisory blocks nothing - execution API: COMPLETE/SKIP/sign-off/START gated, 400 names the issue; auto-completion gated; capture endpoint gains should-be/is, containment, assign; ON_HOLD flipping and auto-resume machinery deleted - issues API: POST /disposition (signature releases holds live, SSE), POST /containment (narrowing audited with a note), close rules (disposition first; NC close requires corrective action), /holding - execution UI: ANOMALY capture modal (photo-first, no alert(), toast names the ticket), held controls absent + blocker lines, hold chips, live release via issue_dispositioned SSE - issue page §5: HOLDING readout, disposition panel with readiness + SIGN (confirm names what it releases), SELF-DISPOSITIONED badge, attachments upload, spawn-redline deep link for rework/repair - issues list §6: IT · TITLE · TYPE · DISP-STATE · HOLDING · PRIORITY · AGE; undispositioned-with-containment sorts first - MCP §7: raise_issue capture fields, sign_disposition, set_containment, get_holds(execution_id) Co-Authored-By: Claude Fable 5 --- ...2e803_issue_containment_and_disposition.py | 122 ++++++ src/opal/api/routes/execution.py | 194 +++++---- src/opal/api/routes/issues.py | 411 +++++++++++------- src/opal/api/routes/reports.py | 15 +- src/opal/core/events.py | 21 + src/opal/core/holds.py | 224 ++++++++++ src/opal/db/models/issue.py | 128 +++++- src/opal/mcp/server.py | 304 ++++++++++++- src/opal/seed.py | 14 +- src/opal/tui/api_client.py | 8 + src/opal/tui/screens/issues.py | 33 +- src/opal/web/routes.py | 116 +++-- src/opal/web/templates/executions/detail.html | 172 +++++++- .../web/templates/executions/tabs/issues.html | 14 +- .../templates/executions/tabs/operations.html | 69 +-- src/opal/web/templates/issues/detail.html | 313 +++++++++---- src/opal/web/templates/issues/list.html | 29 +- src/opal/web/templates/issues/new.html | 16 +- src/opal/web/templates/issues/table_rows.html | 13 +- tests/unit/test_execution.py | 365 +++++++++++----- tests/unit/test_issue_hold_rendering.py | 127 ++++++ tests/unit/test_issues.py | 225 ++++++++-- 22 files changed, 2274 insertions(+), 659 deletions(-) create mode 100644 migrations/versions/a7c41d92e803_issue_containment_and_disposition.py create mode 100644 src/opal/core/holds.py create mode 100644 tests/unit/test_issue_hold_rendering.py diff --git a/migrations/versions/a7c41d92e803_issue_containment_and_disposition.py b/migrations/versions/a7c41d92e803_issue_containment_and_disposition.py new file mode 100644 index 0000000..dd6b9de --- /dev/null +++ b/migrations/versions/a7c41d92e803_issue_containment_and_disposition.py @@ -0,0 +1,122 @@ +"""Issue containment and disposition signature + +Issues R2: the blocking predicate becomes `undispositioned`, never `open`. +- containment (step|op|wo|advisory) + containment_step_id: what an issue holds +- raised_step_id (renamed from step_execution_id) + raised_by_id: where/who found it +- disposition signature: dispositioned_by_id (renamed from + disposition_approved_by_id) + dispositioned_at +- disposition_rationale (renamed from disposition_notes) +- actual (renamed from is_condition) — the should-be/is capture pair +- IssueStatus collapses to open|closed; disposition state is derived +- DispositionType moves to the MIL-STD-1520 set (return_to_vendor → + return_to_supplier; repair and no_defect added) +- step_execution.on_hold rows revert to in_progress: holds are now derived + from undispositioned issues, never stored on the step + +Revision ID: a7c41d92e803 +Revises: ccf93c36170a +Create Date: 2026-06-12 00:00:00.000000 + +""" +from typing import Sequence, Union + +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision: str = "a7c41d92e803" +down_revision: Union[str, None] = "ccf93c36170a" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + # 1. Issue table: new containment/signature columns + renames + with op.batch_alter_table("issue") as batch_op: + batch_op.add_column( + sa.Column("containment", sa.String(20), nullable=False, server_default="advisory") + ) + batch_op.add_column(sa.Column("containment_step_id", sa.Integer(), nullable=True)) + batch_op.add_column(sa.Column("raised_by_id", sa.Integer(), nullable=True)) + batch_op.add_column(sa.Column("dispositioned_at", sa.DateTime(timezone=True), nullable=True)) + batch_op.alter_column("step_execution_id", new_column_name="raised_step_id") + batch_op.alter_column("is_condition", new_column_name="actual") + batch_op.alter_column("disposition_notes", new_column_name="disposition_rationale") + batch_op.alter_column("disposition_approved_by_id", new_column_name="dispositioned_by_id") + batch_op.create_foreign_key( + "fk_issue_containment_step", + "step_execution", + ["containment_step_id"], + ["id"], + ondelete="SET NULL", + ) + batch_op.create_foreign_key( + "fk_issue_raised_by", "user", ["raised_by_id"], ["id"], ondelete="SET NULL" + ) + + # The index on the renamed column survives the batch rebuild under its old + # name; rename it to match the column. + op.execute("DROP INDEX IF EXISTS ix_issue_step_execution_id") + op.create_index("ix_issue_raised_step_id", "issue", ["raised_step_id"]) + + # 2. Data migration + # Disposition enum: vendor → supplier (MIL-STD-1520 set) + op.execute( + "UPDATE issue SET disposition_type = 'return_to_supplier' " + "WHERE disposition_type = 'return_to_vendor'" + ) + # Issues previously approved get a best-effort signature timestamp so the + # derived `dispositioned` gate stays true for them. + op.execute( + "UPDATE issue SET dispositioned_at = updated_at " + "WHERE status = 'disposition_approved' AND disposition_type IS NOT NULL " + "AND dispositioned_at IS NULL" + ) + # Execution-raised NCs held their step under the old model; preserve that + # as step containment (dispositioned/closed ones don't block anyway). + op.execute( + "UPDATE issue SET containment = 'step' " + "WHERE issue_type = 'non_conformance' AND raised_step_id IS NOT NULL" + ) + # Status collapses to open|closed. + op.execute( + "UPDATE issue SET status = 'open' " + "WHERE status IN ('investigating', 'disposition_pending', 'disposition_approved')" + ) + + # 3. Step holds are derived now — stored ON_HOLD rows revert to in_progress; + # their undispositioned issues keep blocking via containment. + op.execute("UPDATE step_execution SET status = 'in_progress' WHERE status = 'on_hold'") + + +def downgrade() -> None: + # Best-effort status reconstruction: signed dispositions map back to + # disposition_approved; everything else stays open. + op.execute( + "UPDATE issue SET status = 'disposition_approved' " + "WHERE status = 'open' AND disposition_type IS NOT NULL AND dispositioned_at IS NOT NULL" + ) + op.execute( + "UPDATE issue SET disposition_type = 'return_to_vendor' " + "WHERE disposition_type = 'return_to_supplier'" + ) + op.execute( + "UPDATE issue SET disposition_type = 'other' " + "WHERE disposition_type IN ('repair', 'no_defect')" + ) + + op.drop_index("ix_issue_raised_step_id", table_name="issue") + + with op.batch_alter_table("issue") as batch_op: + batch_op.drop_constraint("fk_issue_raised_by", type_="foreignkey") + batch_op.drop_constraint("fk_issue_containment_step", type_="foreignkey") + batch_op.alter_column("dispositioned_by_id", new_column_name="disposition_approved_by_id") + batch_op.alter_column("disposition_rationale", new_column_name="disposition_notes") + batch_op.alter_column("actual", new_column_name="is_condition") + batch_op.alter_column("raised_step_id", new_column_name="step_execution_id") + batch_op.drop_column("dispositioned_at") + batch_op.drop_column("raised_by_id") + batch_op.drop_column("containment_step_id") + batch_op.drop_column("containment") + + op.create_index("ix_issue_step_execution_id", "issue", ["step_execution_id"]) diff --git a/src/opal/api/routes/execution.py b/src/opal/api/routes/execution.py index ce98f9d..aaa5c59 100644 --- a/src/opal/api/routes/execution.py +++ b/src/opal/api/routes/execution.py @@ -24,6 +24,7 @@ emit_user_left, ) from opal.core.genealogy import record_assembly_genealogy +from opal.core.holds import get_hold_state, get_holds_payload from opal.core.part_lifecycle import ensure_parts_active from opal.db.models import InventoryRecord, Kit, Part, ProcedureOutput from opal.db.models.execution import ( @@ -40,7 +41,13 @@ SourceType, UsageType, ) -from opal.db.models.issue import Issue, IssuePriority, IssueStatus, IssueType +from opal.db.models.issue import ( + Containment, + Issue, + IssuePriority, + IssueStatus, + IssueType, +) from opal.db.models.procedure import MasterProcedure, ProcedureType, ProcedureVersion router = APIRouter(prefix="/procedure-instances", tags=["execution"]) @@ -143,11 +150,25 @@ class StepComplete(BaseModel): class NonConformanceCreate(BaseModel): - """Log non-conformance during step execution.""" + """Capture an issue at the moment of discovery during step execution. + + The invoking context already knows WO, OP, step, and operator — the + payload only carries what the crew observed. + """ title: str = Field(..., min_length=1, max_length=255) description: str | None = None priority: str = "medium" + should_be: str | None = None + actual: str | None = None + containment: str = "step" + assigned_to_id: int | None = None + + +def _hold_blocker_detail(action: str, blockers: list[Issue]) -> str: + """400 detail naming the holding issue(s) — the blocker line's server twin.""" + names = ", ".join(f"{i.issue_number} {i.title}" for i in blockers) + return f"Cannot {action}: held by {names} (undispositioned)" # ============ Instance CRUD ============ @@ -576,33 +597,19 @@ async def start_step( if step_status != StepStatus.PENDING.value: raise HTTPException(status_code=400, detail="Step already started or completed") + # Containment gate: a bound future step ("resolve by 3.7") cannot START + # while its issue is undispositioned. + start_blockers = get_hold_state(db, instance_id).blockers_for_start(step_exec) + if start_blockers: + raise HTTPException(status_code=400, detail=_hold_blocker_detail("start", start_blockers)) + # Gating. We evaluate against the op-level entry — for sub-steps, that's - # the parent op. A sub-step inside a gated/held op must not start. + # the parent op. gate_op_order: int | None = None if step_exec.level == 0: gate_op_order = step_number elif step_exec.parent_step_order is not None: gate_op_order = step_exec.parent_step_order - # Refuse if the parent op is on hold (NC open on a sibling sub-step). - parent_exec = next( - ( - se - for se in instance.step_executions - if se.step_number == gate_op_order and se.level == 0 - ), - None, - ) - if parent_exec is not None: - parent_status = ( - parent_exec.status.value - if hasattr(parent_exec.status, "value") - else parent_exec.status - ) - if parent_status == StepStatus.ON_HOLD.value: - raise HTTPException( - status_code=400, - detail="Cannot start: parent OP is on hold (open NC)", - ) if gate_op_order is not None: version = ( @@ -735,6 +742,15 @@ async def complete_step( ]: raise HTTPException(status_code=400, detail=f"Cannot complete step in {step_status} status") + # Containment gate: COMPLETE is a commitment moment — it checks the + # undispositioned-issue state of its scope. A signed disposition releases + # this immediately. + complete_blockers = get_hold_state(db, instance_id).blockers_for_complete(step_exec) + if complete_blockers: + raise HTTPException( + status_code=400, detail=_hold_blocker_detail("complete", complete_blockers) + ) + # If step wasn't started, start it now if step_status == StepStatus.PENDING.value: step_exec.started_at = datetime.now(UTC) @@ -792,19 +808,6 @@ async def complete_step( # Check if procedure is complete (considering contingency rules) _check_instance_completion(instance, db) - # If this step is part of a redline op, re-evaluate the held host step's - # auto-resume — it may now be unblocked. - if step_exec.ad_hoc_issue_id is not None: - from opal.api.routes.issues import _maybe_resume_step_after_nc_update - - redline_issue = ( - db.query(Issue) - .filter(Issue.id == step_exec.ad_hoc_issue_id, Issue.deleted_at.is_(None)) - .first() - ) - if redline_issue is not None: - _maybe_resume_step_after_nc_update(db, redline_issue, user_id) - db.commit() db.refresh(step_exec) db.refresh(instance) @@ -919,12 +922,12 @@ def skip_step( step_status = step_exec.status.value if hasattr(step_exec.status, "value") else step_exec.status if step_status == StepStatus.COMPLETED.value: raise HTTPException(status_code=400, detail="Cannot skip completed step") - if step_status == StepStatus.ON_HOLD.value: - raise HTTPException( - status_code=400, - detail="Cannot skip a step that is on hold for an open NC; " - "resolve the NC disposition first.", - ) + + # Containment gate: SKIP is a terminal commitment, same scope check as + # COMPLETE — skipping held work would sweep the hold. + skip_blockers = get_hold_state(db, instance_id).blockers_for_complete(step_exec) + if skip_blockers: + raise HTTPException(status_code=400, detail=_hold_blocker_detail("skip", skip_blockers)) step_exec.status = StepStatus.SKIPPED step_exec.completed_at = datetime.now(UTC) @@ -996,6 +999,14 @@ async def signoff_step( status_code=400, detail=f"Cannot sign off step in {step_status} status" ) + # Containment gate: sign-off is a terminal commitment, same scope check + # as COMPLETE. + signoff_blockers = get_hold_state(db, instance_id).blockers_for_complete(step_exec) + if signoff_blockers: + raise HTTPException( + status_code=400, detail=_hold_blocker_detail("sign off", signoff_blockers) + ) + step_exec.status = StepStatus.SIGNED_OFF step_exec.signed_off_at = datetime.now(UTC) step_exec.signed_off_by_id = user_id @@ -1008,19 +1019,6 @@ async def signoff_step( # Check if procedure is complete _check_instance_completion(instance, db) - # If this signoff terminates a redline step, re-evaluate the held host - # step's auto-resume — it may now be unblocked. - if step_exec.ad_hoc_issue_id is not None: - from opal.api.routes.issues import _maybe_resume_step_after_nc_update - - redline_issue = ( - db.query(Issue) - .filter(Issue.id == step_exec.ad_hoc_issue_id, Issue.deleted_at.is_(None)) - .first() - ) - if redline_issue is not None: - _maybe_resume_step_after_nc_update(db, redline_issue, user_id) - db.commit() db.refresh(step_exec) db.refresh(instance) @@ -1069,6 +1067,10 @@ def _check_instance_completion(instance: ProcedureInstance, db: DbSession) -> No version_steps = {s["order"]: s for s in version.content.get("steps", [])} all_steps = db.query(StepExecution).filter(StepExecution.instance_id == instance.id).all() + # Containment gate: held OPs must not auto-complete, and a held WO must + # not close out, even when every step is terminal. + hold_state = get_hold_state(db, instance.id) + # First pass: check if any parent steps should auto-complete when all children are done for step_exec in all_steps: if step_exec.level == 0: # This is a parent OP @@ -1084,6 +1086,8 @@ def _check_instance_completion(instance: ProcedureInstance, db: DbSession) -> No # If parent is still PENDING or IN_PROGRESS, check if all children are done if step_status in [StepStatus.PENDING.value, StepStatus.IN_PROGRESS.value]: + if hold_state.blockers_for_complete(step_exec): + continue all_children_done = all( (c.status.value if hasattr(c.status, "value") else c.status) in [StepStatus.COMPLETED.value, StepStatus.SKIPPED.value] @@ -1128,6 +1132,10 @@ def _to_aware(dt: datetime) -> datetime: if is_contingency and step_status == StepStatus.IN_PROGRESS.value: return # In-progress contingency blocks completion + # An undispositioned wo-containment issue holds the close-out. + if hold_state.wo_blocked: + return + # All required steps are done instance.status = InstanceStatus.COMPLETED instance.completed_at = datetime.now(UTC) @@ -1141,7 +1149,11 @@ def log_non_conformance( db: DbSession, user_id: CurrentUserId, ) -> dict: - """Log a non-conformance during step execution, creates an Issue.""" + """Capture a non-conformance during step execution, creates an Issue. + + The hold is the issue itself: an undispositioned issue with step + containment makes the step's COMPLETE absent. Step status is not touched. + """ instance = db.query(ProcedureInstance).filter(ProcedureInstance.id == instance_id).first() if not instance: raise HTTPException(status_code=404, detail="Instance not found") @@ -1154,12 +1166,18 @@ def log_non_conformance( if not step_exec: raise HTTPException(status_code=404, detail="Step not found") - # Create issue try: priority = IssuePriority(data.priority) except ValueError: priority = IssuePriority.MEDIUM + try: + containment = Containment(data.containment) + except ValueError as err: + raise HTTPException( + status_code=400, detail=f"Invalid containment: {data.containment}" + ) from err + issue = Issue( issue_number=generate_issue_number(db), title=data.title, @@ -1167,64 +1185,52 @@ def log_non_conformance( issue_type=IssueType.NON_CONFORMANCE, status=IssueStatus.OPEN, priority=priority, + containment=containment, + should_be=data.should_be, + actual=data.actual, procedure_id=instance.procedure_id, procedure_instance_id=instance_id, - step_execution_id=step_exec.id, + raised_step_id=step_exec.id, + raised_by_id=user_id, + assigned_to_id=data.assigned_to_id, ) db.add(issue) db.flush() log_create(db, issue, user_id) - - # Put the step on hold until the NC disposition is approved or the issue closed. - step_status_now = ( - step_exec.status.value if hasattr(step_exec.status, "value") else step_exec.status - ) - if step_status_now != StepStatus.ON_HOLD.value: - step_old = get_model_dict(step_exec) - step_exec.status = StepStatus.ON_HOLD - log_update(db, step_exec, step_old, user_id) - - # Propagate hold to the parent op so the whole operation stalls — not - # just the offending sub-step. Other sub-steps in the same op must not - # be startable while an NC is open anywhere inside it. - if step_exec.level > 0 and step_exec.parent_step_order is not None: - parent_exec = ( - db.query(StepExecution) - .filter( - StepExecution.instance_id == instance_id, - StepExecution.step_number == step_exec.parent_step_order, - StepExecution.level == 0, - ) - .first() - ) - if parent_exec is not None: - parent_status = ( - parent_exec.status.value - if hasattr(parent_exec.status, "value") - else parent_exec.status - ) - if parent_status != StepStatus.ON_HOLD.value: - parent_old = get_model_dict(parent_exec) - parent_exec.status = StepStatus.ON_HOLD - log_update(db, parent_exec, parent_old, user_id) - db.commit() db.refresh(issue) return { "id": issue.id, + "issue_number": issue.issue_number, "title": issue.title, "issue_type": issue.issue_type.value if hasattr(issue.issue_type, "value") else issue.issue_type, "status": issue.status.value if hasattr(issue.status, "value") else issue.status, "priority": issue.priority.value if hasattr(issue.priority, "value") else issue.priority, + "containment": issue.containment.value + if hasattr(issue.containment, "value") + else issue.containment, "procedure_instance_id": instance_id, "step_number": step_number, } +@router.get("/{instance_id}/holds") +def get_instance_holds( + instance_id: int, + db: DbSession, +) -> dict: + """The controller's blocking state: every undispositioned issue holding + this work order and what it blocks.""" + instance = db.query(ProcedureInstance).filter(ProcedureInstance.id == instance_id).first() + if not instance: + raise HTTPException(status_code=404, detail="Instance not found") + return get_holds_payload(db, instance_id) + + @router.get("/{instance_id}/version-content") def get_instance_version_content( instance_id: int, @@ -2226,12 +2232,12 @@ def create_ad_hoc_op( issue_type = issue.issue_type.value if hasattr(issue.issue_type, "value") else issue.issue_type if issue_type != IssueType.NON_CONFORMANCE.value: raise HTTPException(status_code=400, detail="Redlines can only be attached to an NC") - if issue.step_execution_id is None: + if issue.raised_step_id is None: raise HTTPException( status_code=400, detail="NC is not attached to a step; cannot create redline" ) - host_exec = db.get(StepExecution, issue.step_execution_id) + host_exec = db.get(StepExecution, issue.raised_step_id) if not host_exec or host_exec.instance_id != instance.id: raise HTTPException(status_code=400, detail="NC does not belong to this execution") diff --git a/src/opal/api/routes/issues.py b/src/opal/api/routes/issues.py index df97718..e1eca64 100644 --- a/src/opal/api/routes/issues.py +++ b/src/opal/api/routes/issues.py @@ -1,4 +1,10 @@ -"""Issues API routes.""" +"""Issues API routes. + +Two gates, not one: raised → UNDISPOSITIONED → DISPOSITIONED → CLOSED. +The blocking predicate is `undispositioned`, never `open`. Signing a +disposition (POST /{id}/disposition) releases every containment the issue +holds, live; closure is bookkeeping that follows at its own pace. +""" from datetime import UTC, datetime @@ -8,8 +14,11 @@ from opal.api.deps import CurrentUserId, DbSession from opal.core.audit import get_model_dict, log_create, log_delete, log_update from opal.core.designators import generate_issue_number -from opal.db.models.execution import StepExecution, StepStatus +from opal.core.events import emit_issue_dispositioned +from opal.core.holds import holding_readout from opal.db.models.issue import ( + CONTAINMENT_RANK, + Containment, DispositionType, Issue, IssuePriority, @@ -40,12 +49,17 @@ class IssueResponse(BaseModel): issue_type: str status: str priority: str + containment: str + containment_step_id: int | None = None + disp_state: str + dispositioned: bool part_id: int | None = None procedure_id: int | None = None procedure_instance_id: int | None = None - step_execution_id: int | None = None + raised_step_id: int | None = None + raised_by_id: int | None = None should_be: str | None = None - is_condition: str | None = None + actual: str | None = None steps_to_reproduce: str | None = None expected_behavior: str | None = None actual_behavior: str | None = None @@ -53,9 +67,10 @@ class IssueResponse(BaseModel): root_cause: str | None = None corrective_action: str | None = None disposition_type: str | None = None - disposition_notes: str | None = None + disposition_rationale: str | None = None assigned_to_id: int | None = None - disposition_approved_by_id: int | None = None + dispositioned_by_id: int | None = None + dispositioned_at: datetime | None = None created_at: datetime updated_at: datetime @@ -78,8 +93,10 @@ class IssueCreate(BaseModel): description: str | None = None issue_type: str = "task" priority: str = "medium" + containment: str = "advisory" + containment_step_id: int | None = None should_be: str | None = None - is_condition: str | None = None + actual: str | None = None steps_to_reproduce: str | None = None expected_behavior: str | None = None actual_behavior: str | None = None @@ -87,11 +104,13 @@ class IssueCreate(BaseModel): part_id: int | None = None procedure_id: int | None = None procedure_instance_id: int | None = None + raised_step_id: int | None = None assigned_to_id: int | None = None class IssueUpdate(BaseModel): - """Update issue request.""" + """Update issue request. Disposition is signed via POST /{id}/disposition, + not patched; containment changes go through POST /{id}/containment.""" title: str | None = Field(None, min_length=1, max_length=255) description: str | None = None @@ -99,7 +118,7 @@ class IssueUpdate(BaseModel): status: str | None = None priority: str | None = None should_be: str | None = None - is_condition: str | None = None + actual: str | None = None steps_to_reproduce: str | None = None expected_behavior: str | None = None actual_behavior: str | None = None @@ -110,9 +129,24 @@ class IssueUpdate(BaseModel): root_cause: str | None = None corrective_action: str | None = None disposition_type: str | None = None - disposition_notes: str | None = None + disposition_rationale: str | None = None assigned_to_id: int | None = None - disposition_approved_by_id: int | None = None + + +class DispositionSign(BaseModel): + """Sign a disposition — the signature moment.""" + + disposition_type: str + disposition_rationale: str = Field(..., min_length=1) + + +class ContainmentChange(BaseModel): + """Change containment scope. Narrowing releases a hold and is audited + with a note.""" + + containment: str + containment_step_id: int | None = None + note: str | None = None class IssueCommentResponse(BaseModel): @@ -143,12 +177,17 @@ def _issue_to_response(issue: Issue) -> IssueResponse: issue_type=_get_enum_val(issue, "issue_type"), status=_get_enum_val(issue, "status"), priority=_get_enum_val(issue, "priority"), + containment=_get_enum_val(issue, "containment"), + containment_step_id=issue.containment_step_id, + disp_state=issue.disp_state, + dispositioned=issue.dispositioned, part_id=issue.part_id, procedure_id=issue.procedure_id, procedure_instance_id=issue.procedure_instance_id, - step_execution_id=issue.step_execution_id, + raised_step_id=issue.raised_step_id, + raised_by_id=issue.raised_by_id, should_be=issue.should_be, - is_condition=issue.is_condition, + actual=issue.actual, steps_to_reproduce=issue.steps_to_reproduce, expected_behavior=issue.expected_behavior, actual_behavior=issue.actual_behavior, @@ -158,9 +197,10 @@ def _issue_to_response(issue: Issue) -> IssueResponse: disposition_type=_get_enum_val(issue, "disposition_type") if issue.disposition_type else None, - disposition_notes=issue.disposition_notes, + disposition_rationale=issue.disposition_rationale, assigned_to_id=issue.assigned_to_id, - disposition_approved_by_id=issue.disposition_approved_by_id, + dispositioned_by_id=issue.dispositioned_by_id, + dispositioned_at=issue.dispositioned_at, created_at=issue.created_at, updated_at=issue.updated_at, ) @@ -193,6 +233,12 @@ def get_disposition_types() -> list[str]: return [d.value for d in DispositionType] +@router.get("/containments", response_model=list[str]) +def get_containments() -> list[str]: + """Get all containment scopes.""" + return [c.value for c in Containment] + + # ============ Issue CRUD ============ @@ -202,6 +248,7 @@ def list_issues( search: str | None = Query(None), issue_type: str | None = Query(None), status: str | None = Query(None), + disp_state: str | None = Query(None), priority: str | None = Query(None), part_id: int | None = Query(None), procedure_id: int | None = Query(None), @@ -220,6 +267,8 @@ def list_issues( query = query.filter(Issue.issue_type == issue_type) if status: query = query.filter(Issue.status == status) + if disp_state: + query = _filter_disp_state(query, disp_state) if priority: query = query.filter(Issue.priority == priority) if part_id: @@ -241,6 +290,23 @@ def list_issues( ) +def _filter_disp_state(query, disp_state: str): + """Filter by derived disposition state.""" + from sqlalchemy import and_, or_ + + signed = and_(Issue.disposition_type.isnot(None), Issue.dispositioned_at.isnot(None)) + if disp_state == "closed": + return query.filter(Issue.status == IssueStatus.CLOSED) + if disp_state == "dispositioned": + return query.filter(Issue.status != IssueStatus.CLOSED, signed) + if disp_state == "undispositioned": + return query.filter( + Issue.status != IssueStatus.CLOSED, + or_(Issue.disposition_type.is_(None), Issue.dispositioned_at.is_(None)), + ) + raise HTTPException(status_code=400, detail=f"Invalid disp_state: {disp_state}") + + @router.post("", response_model=IssueResponse, status_code=201) def create_issue( data: IssueCreate, @@ -261,6 +327,13 @@ def create_issue( except ValueError as err: raise HTTPException(status_code=400, detail=f"Invalid priority: {data.priority}") from err + try: + containment = Containment(data.containment) + except ValueError as err: + raise HTTPException( + status_code=400, detail=f"Invalid containment: {data.containment}" + ) from err + issue = Issue( issue_number=generate_issue_number(db), title=data.title, @@ -268,8 +341,10 @@ def create_issue( issue_type=issue_type, status=IssueStatus.OPEN, priority=priority, + containment=containment, + containment_step_id=data.containment_step_id, should_be=data.should_be, - is_condition=data.is_condition, + actual=data.actual, steps_to_reproduce=data.steps_to_reproduce, expected_behavior=data.expected_behavior, actual_behavior=data.actual_behavior, @@ -277,6 +352,8 @@ def create_issue( part_id=data.part_id, procedure_id=data.procedure_id, procedure_instance_id=data.procedure_instance_id, + raised_step_id=data.raised_step_id, + raised_by_id=user_id, assigned_to_id=data.assigned_to_id, ) db.add(issue) @@ -302,6 +379,21 @@ def get_issue( return _issue_to_response(issue) +@router.get("/{issue_id}/holding", response_model=list[dict]) +def get_issue_holding( + issue_id: int, + db: DbSession, +) -> list[dict]: + """What this issue is stopping: [{label, href}], empty for advisory or + dispositioned issues. The disposition confirm dialog reads its consequence + sentence from here.""" + issue = db.query(Issue).filter(Issue.id == issue_id, Issue.deleted_at.is_(None)).first() + if not issue: + raise HTTPException(status_code=404, detail="Issue not found") + + return holding_readout(db, issue) + + @router.patch("/{issue_id}", response_model=IssueResponse) def update_issue( issue_id: int, @@ -332,16 +424,8 @@ def update_issue( new_status = IssueStatus(data.status) except ValueError as err: raise HTTPException(status_code=400, detail=f"Invalid status: {data.status}") from err - # Disposition approval requires disposition_type to be set - if new_status == IssueStatus.DISPOSITION_APPROVED: - effective_disposition_type = data.disposition_type or ( - _get_enum_val(issue, "disposition_type") if issue.disposition_type else None - ) - if not effective_disposition_type: - raise HTTPException( - status_code=400, - detail="disposition_type is required when approving disposition", - ) + if new_status == IssueStatus.CLOSED: + _validate_close(issue, data) issue.status = new_status if data.priority is not None: try: @@ -352,8 +436,8 @@ def update_issue( ) from err if data.should_be is not None: issue.should_be = data.should_be - if data.is_condition is not None: - issue.is_condition = data.is_condition + if data.actual is not None: + issue.actual = data.actual if data.steps_to_reproduce is not None: issue.steps_to_reproduce = data.steps_to_reproduce if data.expected_behavior is not None: @@ -372,146 +456,171 @@ def update_issue( issue.root_cause = data.root_cause if data.corrective_action is not None: issue.corrective_action = data.corrective_action - if data.disposition_type is not None: - if data.disposition_type == "": - # Clearing disposition_type — block if status is disposition_approved - current_status = _get_enum_val(issue, "status") if issue.status else None - if current_status == "disposition_approved": - raise HTTPException( - status_code=400, - detail="Cannot clear disposition_type while status is disposition_approved", - ) - issue.disposition_type = None - else: - try: - issue.disposition_type = DispositionType(data.disposition_type) - except ValueError as err: - raise HTTPException( - status_code=400, detail=f"Invalid disposition type: {data.disposition_type}" - ) from err - if data.disposition_notes is not None: - issue.disposition_notes = data.disposition_notes + if data.disposition_type is not None or data.disposition_rationale is not None: + # Drafting the disposition panel is a PATCH; the signature is not. + # A signed disposition is immutable — a different decision is a new + # signature, not an edit. + if issue.dispositioned: + raise HTTPException( + status_code=400, + detail="Disposition already signed; it cannot be edited", + ) + if data.disposition_type is not None: + if data.disposition_type == "": + issue.disposition_type = None + else: + try: + issue.disposition_type = DispositionType(data.disposition_type) + except ValueError as err: + raise HTTPException( + status_code=400, + detail=f"Invalid disposition type: {data.disposition_type}", + ) from err + if data.disposition_rationale is not None: + issue.disposition_rationale = data.disposition_rationale if data.assigned_to_id is not None: issue.assigned_to_id = data.assigned_to_id - if data.disposition_approved_by_id is not None: - issue.disposition_approved_by_id = data.disposition_approved_by_id log_update(db, issue, old_values, user_id) - - # Auto-resume step on hold once all linked NCs reach a terminal disposition. - _maybe_resume_step_after_nc_update(db, issue, user_id) - db.commit() db.refresh(issue) return _issue_to_response(issue) -def _maybe_resume_step_after_nc_update(db, issue: "Issue", user_id: int | None) -> None: - """If this NC just reached a terminal state and no other open NCs remain on - its step, pop the step back to IN_PROGRESS.""" - if issue.step_execution_id is None: - return - issue_type = issue.issue_type.value if hasattr(issue.issue_type, "value") else issue.issue_type - if issue_type != IssueType.NON_CONFORMANCE.value: - return - issue_status = issue.status.value if hasattr(issue.status, "value") else issue.status - if issue_status not in (IssueStatus.DISPOSITION_APPROVED.value, IssueStatus.CLOSED.value): +def _recheck_instance_completion(db, issue: Issue) -> None: + """Releasing a hold may have been the last thing standing between a work + order and completion (e.g. wo/op containment signed after every step + finished) — re-evaluate.""" + if issue.procedure_instance_id is None: return + from opal.api.routes.execution import _check_instance_completion + from opal.db.models.execution import ProcedureInstance + + instance = db.get(ProcedureInstance, issue.procedure_instance_id) + if instance is not None: + _check_instance_completion(instance, db) - step_exec = db.get(StepExecution, issue.step_execution_id) - if step_exec is None: - return - step_status = step_exec.status.value if hasattr(step_exec.status, "value") else step_exec.status - if step_status != StepStatus.ON_HOLD.value: - return - remaining = ( - db.query(Issue) - .filter( - Issue.step_execution_id == step_exec.id, - Issue.issue_type == IssueType.NON_CONFORMANCE, - Issue.id != issue.id, - Issue.status.notin_([IssueStatus.DISPOSITION_APPROVED, IssueStatus.CLOSED]), - Issue.deleted_at.is_(None), +def _validate_close(issue: Issue, data: IssueUpdate | None = None) -> None: + """Closing an undispositioned issue is impossible — disposition first, + always. NC-type issues additionally require corrective action text.""" + if not issue.dispositioned: + raise HTTPException( + status_code=400, + detail=f"{issue.issue_number} is undispositioned; sign a disposition before closing", ) - .count() - ) - if remaining != 0: - return + issue_type = _get_enum_val(issue, "issue_type") + if issue_type == IssueType.NON_CONFORMANCE.value: + corrective = issue.corrective_action + if data is not None and data.corrective_action is not None: + corrective = data.corrective_action + if not (corrective or "").strip(): + raise HTTPException( + status_code=400, + detail=f"{issue.issue_number} is a non-conformance; " + "corrective action is required to close", + ) - # Hold back if any redline op authorized by an NC on this step is still - # outstanding. Approving the disposition isn't enough — the rework itself - # has to be completed before the held step can resume. - terminal_states = ( - StepStatus.COMPLETED, - StepStatus.SIGNED_OFF, - StepStatus.SKIPPED, - ) - open_redlines = ( - db.query(StepExecution) - .join(Issue, Issue.id == StepExecution.ad_hoc_issue_id) - .filter( - StepExecution.instance_id == step_exec.instance_id, - StepExecution.level == 0, - Issue.step_execution_id == step_exec.id, - Issue.deleted_at.is_(None), - StepExecution.status.notin_(terminal_states), + +@router.post("/{issue_id}/disposition", response_model=IssueResponse) +async def sign_disposition( + issue_id: int, + data: DispositionSign, + db: DbSession, + user_id: CurrentUserId, +) -> IssueResponse: + """Sign the disposition — sets the signature and releases every + containment this issue holds, live.""" + issue = db.query(Issue).filter(Issue.id == issue_id, Issue.deleted_at.is_(None)).first() + if not issue: + raise HTTPException(status_code=404, detail="Issue not found") + + if _get_enum_val(issue, "status") == IssueStatus.CLOSED.value: + raise HTTPException(status_code=400, detail="Issue is closed") + if issue.dispositioned: + raise HTTPException(status_code=400, detail="Disposition already signed") + + try: + disposition_type = DispositionType(data.disposition_type) + except ValueError as err: + raise HTTPException( + status_code=400, detail=f"Invalid disposition type: {data.disposition_type}" + ) from err + + old_values = get_model_dict(issue) + issue.disposition_type = disposition_type + issue.disposition_rationale = data.disposition_rationale + issue.dispositioned_by_id = user_id + issue.dispositioned_at = datetime.now(UTC) + + log_update(db, issue, old_values, user_id) + db.flush() + _recheck_instance_completion(db, issue) + db.commit() + db.refresh(issue) + + # Held rows recover their controls without reload. + if issue.procedure_instance_id is not None: + await emit_issue_dispositioned(issue.procedure_instance_id, issue.id, issue.issue_number) + + return _issue_to_response(issue) + + +@router.post("/{issue_id}/containment", response_model=IssueResponse) +def set_containment( + issue_id: int, + data: ContainmentChange, + db: DbSession, + user_id: CurrentUserId, +) -> IssueResponse: + """Change containment scope. Widening is one click; narrowing (or + downgrading to advisory) releases a hold — a decision, not an edit — and + is audited with a note.""" + issue = db.query(Issue).filter(Issue.id == issue_id, Issue.deleted_at.is_(None)).first() + if not issue: + raise HTTPException(status_code=404, detail="Issue not found") + + try: + new_containment = Containment(data.containment) + except ValueError as err: + raise HTTPException( + status_code=400, detail=f"Invalid containment: {data.containment}" + ) from err + + old_containment = _get_enum_val(issue, "containment") + narrowing = CONTAINMENT_RANK[new_containment.value] < CONTAINMENT_RANK[old_containment] + if narrowing and issue.is_blocking and not (data.note or "").strip(): + raise HTTPException( + status_code=400, + detail=f"Narrowing containment {old_containment} → {new_containment.value} " + "releases a hold; a note is required", ) - .count() - ) - if open_redlines: - return - step_old = get_model_dict(step_exec) - step_exec.status = StepStatus.IN_PROGRESS - log_update(db, step_exec, step_old, user_id) - - # Propagate resume up: when an NC was logged on a sub-step we also flipped - # the parent op to ON_HOLD so the whole operation paused. Now that this - # sub-step is clear, resume the parent too — but only if no other - # sub-step of that parent has any open NC remaining. - if step_exec.level > 0 and step_exec.parent_step_order is not None: - parent_exec = ( - db.query(StepExecution) - .filter( - StepExecution.instance_id == step_exec.instance_id, - StepExecution.step_number == step_exec.parent_step_order, - StepExecution.level == 0, - ) - .first() + old_values = get_model_dict(issue) + issue.containment = new_containment + if data.containment_step_id is not None: + issue.containment_step_id = data.containment_step_id + log_update(db, issue, old_values, user_id) + + if narrowing and (data.note or "").strip(): + comment = IssueComment( + issue_id=issue.id, + user_id=user_id, + body=f"Containment narrowed {old_containment} → {new_containment.value}: {data.note}", ) - if parent_exec is not None: - parent_status = ( - parent_exec.status.value - if hasattr(parent_exec.status, "value") - else parent_exec.status - ) - if parent_status == StepStatus.ON_HOLD.value: - sibling_se_ids = [ - se.id - for se in db.query(StepExecution) - .filter( - StepExecution.instance_id == step_exec.instance_id, - StepExecution.parent_step_order == parent_exec.step_number, - ) - .all() - ] - sibling_se_ids.append(parent_exec.id) - siblings_open = ( - db.query(Issue) - .filter( - Issue.step_execution_id.in_(sibling_se_ids), - Issue.issue_type == IssueType.NON_CONFORMANCE, - Issue.status.notin_([IssueStatus.DISPOSITION_APPROVED, IssueStatus.CLOSED]), - Issue.deleted_at.is_(None), - ) - .count() - ) - if siblings_open == 0: - parent_old = get_model_dict(parent_exec) - parent_exec.status = StepStatus.IN_PROGRESS - log_update(db, parent_exec, parent_old, user_id) + db.add(comment) + db.flush() + log_create(db, comment, user_id) + + if narrowing: + db.flush() + _recheck_instance_completion(db, issue) + + db.commit() + db.refresh(issue) + + return _issue_to_response(issue) @router.delete("/{issue_id}", status_code=204) @@ -527,6 +636,8 @@ def delete_issue( issue.deleted_at = datetime.now(UTC) log_delete(db, issue, user_id) + db.flush() + _recheck_instance_completion(db, issue) db.commit() diff --git a/src/opal/api/routes/reports.py b/src/opal/api/routes/reports.py index c12ba41..7ae708f 100644 --- a/src/opal/api/routes/reports.py +++ b/src/opal/api/routes/reports.py @@ -436,9 +436,8 @@ class IssueMetrics(BaseModel): total_issues: int open: int - investigating: int - disposition_pending: int - disposition_approved: int + undispositioned: int + dispositioned: int closed: int by_type: dict[str, int] by_priority: dict[str, int] @@ -462,9 +461,8 @@ def get_issue_metrics( total = len(issues) open_count = sum(1 for i in issues if _get_issue_status(i) == "open") - investigating = sum(1 for i in issues if _get_issue_status(i) == "investigating") - disposition_pending = sum(1 for i in issues if _get_issue_status(i) == "disposition_pending") - disposition_approved = sum(1 for i in issues if _get_issue_status(i) == "disposition_approved") + undispositioned = sum(1 for i in issues if i.disp_state == "undispositioned") + dispositioned = sum(1 for i in issues if i.disp_state == "dispositioned") closed = sum(1 for i in issues if _get_issue_status(i) == "closed") by_type: dict[str, int] = {} @@ -482,9 +480,8 @@ def get_issue_metrics( return IssueMetrics( total_issues=total, open=open_count, - investigating=investigating, - disposition_pending=disposition_pending, - disposition_approved=disposition_approved, + undispositioned=undispositioned, + dispositioned=dispositioned, closed=closed, by_type=by_type, by_priority=by_priority, diff --git a/src/opal/core/events.py b/src/opal/core/events.py index 6ac3223..90f8ae7 100644 --- a/src/opal/core/events.py +++ b/src/opal/core/events.py @@ -26,6 +26,9 @@ class EventType(str, Enum): INSTANCE_STARTED = "instance_started" INSTANCE_COMPLETED = "instance_completed" + # Issue events + ISSUE_DISPOSITIONED = "issue_dispositioned" + # Collaboration events USER_JOINED = "user_joined" USER_LEFT = "user_left" @@ -207,6 +210,24 @@ async def emit_instance_completed( await event_bus.publish(event) +async def emit_issue_dispositioned( + instance_id: int, + issue_id: int, + issue_number: str, +) -> None: + """Emit an issue_dispositioned event — held rows on execution pages + recover their controls without reload.""" + event = Event( + type=EventType.ISSUE_DISPOSITIONED, + data={ + "instance_id": instance_id, + "issue_id": issue_id, + "issue_number": issue_number, + }, + ) + await event_bus.publish_to_instance(instance_id, event) + + async def emit_user_joined( instance_id: int, user_id: int, diff --git a/src/opal/core/holds.py b/src/opal/core/holds.py new file mode 100644 index 0000000..44fd302 --- /dev/null +++ b/src/opal/core/holds.py @@ -0,0 +1,224 @@ +"""Containment-derived hold computation. + +Holds are never stored — they are derived, at the commitment moments +(COMPLETE, START, SKIP, instance close-out), from undispositioned issues +and their containment scope: + + containment while undispositioned… + step the raised/bound step cannot COMPLETE (and its OP cannot + COMPLETE over it); a bound *future* step cannot START + op every step in the OP may run, but the OP cannot COMPLETE + wo the work order cannot COMPLETE/close out + advisory blocks nothing + +Signing a disposition releases every containment the issue holds, live: +there is no stored state to flip back. +""" + +from dataclasses import dataclass, field + +from sqlalchemy import or_ +from sqlalchemy.orm import Session + +from opal.db.models.execution import ProcedureInstance, StepExecution +from opal.db.models.issue import Containment, Issue, IssueStatus + + +def _val(obj: object) -> str | None: + """Unwrap a potentially-enum attribute to its string value.""" + if obj is None: + return None + return obj.value if hasattr(obj, "value") else obj # type: ignore[return-value] + + +def blocking_issues_for_instance(db: Session, instance_id: int) -> list[Issue]: + """Undispositioned, non-advisory issues linked to this work order.""" + return ( + db.query(Issue) + .filter( + Issue.procedure_instance_id == instance_id, + Issue.deleted_at.is_(None), + Issue.status != IssueStatus.CLOSED, + Issue.containment != Containment.ADVISORY, + or_(Issue.disposition_type.is_(None), Issue.dispositioned_at.is_(None)), + ) + .order_by(Issue.id) + .all() + ) + + +@dataclass +class HoldState: + """Derived blocking state for one work order.""" + + # step_execution_id -> issues blocking that step's COMPLETE (and SKIP) + complete_blocked: dict[int, list[Issue]] = field(default_factory=dict) + # step_execution_id -> issues blocking that step's START (bound future step) + start_blocked: dict[int, list[Issue]] = field(default_factory=dict) + # op-level step_execution_id -> issues blocking that OP's COMPLETE + op_complete_blocked: dict[int, list[Issue]] = field(default_factory=dict) + # issues blocking the work order's completion/close-out + wo_blocked: list[Issue] = field(default_factory=list) + + def blockers_for_complete(self, step_exec: StepExecution) -> list[Issue]: + """Issues that make this row's COMPLETE (or SKIP) absent.""" + blockers = list(self.complete_blocked.get(step_exec.id, [])) + if step_exec.level == 0: + seen = {i.id for i in blockers} + blockers += [ + i for i in self.op_complete_blocked.get(step_exec.id, []) if i.id not in seen + ] + return blockers + + def blockers_for_start(self, step_exec: StepExecution) -> list[Issue]: + """Issues that make this row's START absent.""" + return list(self.start_blocked.get(step_exec.id, [])) + + +def get_hold_state(db: Session, instance_id: int) -> HoldState: + """Compute the full blocking state for a work order.""" + state = HoldState() + issues = blocking_issues_for_instance(db, instance_id) + if not issues: + return state + + steps = db.query(StepExecution).filter(StepExecution.instance_id == instance_id).all() + by_id = {se.id: se for se in steps} + op_by_number = {se.step_number: se for se in steps if se.level == 0} + + def op_row_for(step_exec: StepExecution) -> StepExecution | None: + if step_exec.level == 0: + return step_exec + if step_exec.parent_step_order is not None: + return op_by_number.get(step_exec.parent_step_order) + return None + + for issue in issues: + containment = _val(issue.containment) + anchor = by_id.get(issue.containment_step_id) or by_id.get(issue.raised_step_id) + + if containment == Containment.WO.value: + state.wo_blocked.append(issue) + continue + + if anchor is None: + # step/op containment with no resolvable step in this WO holds the + # WO itself rather than silently blocking nothing. + state.wo_blocked.append(issue) + continue + + if containment == Containment.OP.value: + op_row = op_row_for(anchor) + if op_row is not None: + state.op_complete_blocked.setdefault(op_row.id, []).append(issue) + continue + + # containment == step + bound_elsewhere = ( + issue.containment_step_id is not None + and issue.raised_step_id is not None + and issue.containment_step_id != issue.raised_step_id + ) + if bound_elsewhere: + # "resolve by 3.7": work continues up to the boundary, which + # cannot START. + state.start_blocked.setdefault(anchor.id, []).append(issue) + else: + state.complete_blocked.setdefault(anchor.id, []).append(issue) + op_row = op_row_for(anchor) + if op_row is not None and op_row.id != anchor.id: + state.op_complete_blocked.setdefault(op_row.id, []).append(issue) + + return state + + +def _step_label(step_exec: StepExecution) -> str: + return step_exec.step_number_str or str(step_exec.step_number) + + +def _op_label(step_exec: StepExecution) -> str: + return f"OP {_step_label(step_exec)}" + + +def holding_readout(db: Session, issue: Issue) -> list[dict]: + """What this issue is stopping, as [{label, href}] — the HOLDING section, + the disposition confirm sentence, and MCP holds all read from here.""" + if not issue.is_blocking: + return [] + + instance_id = issue.procedure_instance_id + containment = _val(issue.containment) + + instance = ( + db.query(ProcedureInstance).filter(ProcedureInstance.id == instance_id).first() + if instance_id + else None + ) + if instance is None: + return [] + + wo_label = instance.work_order_number or f"WO #{instance.id}" + + def exec_href(op_order: int | None = None) -> str: + base = f"/executions/{instance.id}?tab=operations" + return f"{base}&op={op_order}" if op_order is not None else base + + anchor = None + anchor_id = issue.containment_step_id or issue.raised_step_id + if anchor_id is not None: + anchor = db.query(StepExecution).filter(StepExecution.id == anchor_id).first() + + if containment == Containment.WO.value or anchor is None: + return [{"label": f"{wo_label} COMPLETE", "href": exec_href()}] + + op_row = anchor + if anchor.level != 0 and anchor.parent_step_order is not None: + op_row = ( + db.query(StepExecution) + .filter( + StepExecution.instance_id == instance.id, + StepExecution.step_number == anchor.parent_step_order, + StepExecution.level == 0, + ) + .first() + ) or anchor + + if containment == Containment.OP.value: + return [{"label": f"{_op_label(op_row)} COMPLETE", "href": exec_href(op_row.step_number)}] + + # step containment + bound_elsewhere = ( + issue.containment_step_id is not None + and issue.raised_step_id is not None + and issue.containment_step_id != issue.raised_step_id + ) + if bound_elsewhere: + return [{"label": f"{_step_label(anchor)} START", "href": exec_href(op_row.step_number)}] + + targets = [{"label": f"{_step_label(anchor)} COMPLETE", "href": exec_href(op_row.step_number)}] + if op_row.id != anchor.id: + targets.append( + {"label": f"{_op_label(op_row)} COMPLETE", "href": exec_href(op_row.step_number)} + ) + return targets + + +def get_holds_payload(db: Session, instance_id: int) -> dict: + """The controller's blocking state as JSON: every undispositioned issue on + this work order and what it blocks.""" + issues = blocking_issues_for_instance(db, instance_id) + holds = [] + for issue in issues: + holds.append( + { + "issue_id": issue.id, + "issue_number": issue.issue_number, + "title": issue.title, + "priority": _val(issue.priority), + "containment": _val(issue.containment), + "raised_step_id": issue.raised_step_id, + "containment_step_id": issue.containment_step_id, + "blocks": [t["label"] for t in holding_readout(db, issue)], + } + ) + return {"instance_id": instance_id, "held": bool(holds), "holds": holds} diff --git a/src/opal/db/models/issue.py b/src/opal/db/models/issue.py index f3e6241..2814a43 100644 --- a/src/opal/db/models/issue.py +++ b/src/opal/db/models/issue.py @@ -1,8 +1,20 @@ -"""Issue model.""" +"""Issue model. +Issues are the authorized container for off-nominal reality (AS9100 §8.7 +lineage): when reality departs from the published procedure, work either +stops at the containment boundary or continues inside a ticket under a +signed disposition. Two gates, not one: + + raised → UNDISPOSITIONED → (disposition signed) → DISPOSITIONED → CLOSED + +The blocking predicate is `undispositioned`, never `open`. A signed +disposition releases work immediately; closure is bookkeeping. +""" + +from datetime import datetime from enum import Enum -from sqlalchemy import ForeignKey, String, Text +from sqlalchemy import DateTime, ForeignKey, String, Text from sqlalchemy.orm import Mapped, mapped_column, relationship from opal.db.base import Base, IdMixin, SoftDeleteMixin, TimestampMixin @@ -18,12 +30,10 @@ class IssueType(str, Enum): class IssueStatus(str, Enum): - """Issue status.""" + """Issue status — open or closed; the disposition gate is a separate, + derived state (see Issue.disp_state).""" OPEN = "open" - INVESTIGATING = "investigating" - DISPOSITION_PENDING = "disposition_pending" - DISPOSITION_APPROVED = "disposition_approved" CLOSED = "closed" @@ -37,13 +47,33 @@ class IssuePriority(str, Enum): class DispositionType(str, Enum): - """Disposition type for issue resolution.""" + """Disposition type (MIL-STD-1520 lineage).""" USE_AS_IS = "use_as_is" REWORK = "rework" + REPAIR = "repair" SCRAP = "scrap" - RETURN_TO_VENDOR = "return_to_vendor" - OTHER = "other" + RETURN_TO_SUPPLIER = "return_to_supplier" + NO_DEFECT = "no_defect" + + +class Containment(str, Enum): + """What an undispositioned issue holds.""" + + STEP = "step" + OP = "op" + WO = "wo" + ADVISORY = "advisory" + + +# Widening moves up this ladder freely; narrowing releases a hold and is +# therefore audited with a note. +CONTAINMENT_RANK: dict[str, int] = { + Containment.ADVISORY.value: 0, + Containment.STEP.value: 1, + Containment.OP.value: 2, + Containment.WO.value: 3, +} class Issue(Base, IdMixin, TimestampMixin, SoftDeleteMixin): @@ -72,13 +102,19 @@ class Issue(Base, IdMixin, TimestampMixin, SoftDeleteMixin): String(20), nullable=False, default=IssuePriority.MEDIUM ) - # Type-specific fields - should_be: Mapped[str | None] = mapped_column( - Text, nullable=True, comment="NC: expected condition" + # Containment: what this issue holds while undispositioned + containment: Mapped[Containment] = mapped_column( + String(20), nullable=False, default=Containment.ADVISORY ) - is_condition: Mapped[str | None] = mapped_column( - Text, nullable=True, comment="NC: actual condition" + containment_step_id: Mapped[int | None] = mapped_column( + ForeignKey("step_execution.id", ondelete="SET NULL"), + nullable=True, + comment="Boundary step ('resolve by 3.7'); for op/wo containment the boundary is implicit", ) + + # Capture: the should-be / is pair belongs at the moment of discovery + should_be: Mapped[str | None] = mapped_column(Text, nullable=True, comment="Expected condition") + actual: Mapped[str | None] = mapped_column(Text, nullable=True, comment="Actual condition") steps_to_reproduce: Mapped[str | None] = mapped_column( Text, nullable=True, comment="Bug: repro steps" ) @@ -90,11 +126,15 @@ class Issue(Base, IdMixin, TimestampMixin, SoftDeleteMixin): Text, nullable=True, comment="Improvement: benefit" ) - # Disposition fields + # Disposition — the signature releases containment; root cause and + # corrective action are required at NC closure, not at disposition. root_cause: Mapped[str | None] = mapped_column(Text, nullable=True) corrective_action: Mapped[str | None] = mapped_column(Text, nullable=True) disposition_type: Mapped[DispositionType | None] = mapped_column(String(30), nullable=True) - disposition_notes: Mapped[str | None] = mapped_column(Text, nullable=True) + disposition_rationale: Mapped[str | None] = mapped_column(Text, nullable=True) + dispositioned_at: Mapped[datetime | None] = mapped_column( + DateTime(timezone=True), nullable=True + ) # Optional links - an issue can link to any of these (or none) part_id: Mapped[int | None] = mapped_column( @@ -106,16 +146,19 @@ class Issue(Base, IdMixin, TimestampMixin, SoftDeleteMixin): procedure_instance_id: Mapped[int | None] = mapped_column( ForeignKey("procedure_instance.id", ondelete="SET NULL"), nullable=True, index=True ) - step_execution_id: Mapped[int | None] = mapped_column( + raised_step_id: Mapped[int | None] = mapped_column( ForeignKey("step_execution.id", ondelete="SET NULL"), nullable=True, index=True, - comment="For NC issues created during step execution", + comment="Where it was found (execution-raised issues)", + ) + raised_by_id: Mapped[int | None] = mapped_column( + ForeignKey("user.id", ondelete="SET NULL"), nullable=True ) assigned_to_id: Mapped[int | None] = mapped_column( ForeignKey("user.id", ondelete="SET NULL"), nullable=True, index=True ) - disposition_approved_by_id: Mapped[int | None] = mapped_column( + dispositioned_by_id: Mapped[int | None] = mapped_column( ForeignKey("user.id", ondelete="SET NULL"), nullable=True ) @@ -127,13 +170,20 @@ class Issue(Base, IdMixin, TimestampMixin, SoftDeleteMixin): procedure_instance: Mapped["ProcedureInstance | None"] = relationship( "ProcedureInstance", back_populates="issues" ) + raised_step: Mapped["StepExecution | None"] = relationship( + "StepExecution", foreign_keys=[raised_step_id] + ) + containment_step: Mapped["StepExecution | None"] = relationship( + "StepExecution", foreign_keys=[containment_step_id] + ) risk: Mapped["Risk | None"] = relationship("Risk", back_populates="linked_issue") references: Mapped[list["IssueReference"]] = relationship( "IssueReference", back_populates="issue", cascade="all, delete-orphan" ) + raised_by: Mapped["User | None"] = relationship("User", foreign_keys=[raised_by_id]) assigned_to: Mapped["User | None"] = relationship("User", foreign_keys=[assigned_to_id]) - disposition_approved_by: Mapped["User | None"] = relationship( - "User", foreign_keys=[disposition_approved_by_id] + dispositioned_by: Mapped["User | None"] = relationship( + "User", foreign_keys=[dispositioned_by_id] ) comments: Mapped[list["IssueComment"]] = relationship( "IssueComment", @@ -143,5 +193,41 @@ class Issue(Base, IdMixin, TimestampMixin, SoftDeleteMixin): ) attachments: Mapped[list["Attachment"]] = relationship("Attachment", back_populates="issue") + @property + def dispositioned(self) -> bool: + """Derived: disposition type and signature both set.""" + return self.disposition_type is not None and self.dispositioned_at is not None + + @property + def disp_state(self) -> str: + """undispositioned | dispositioned | closed.""" + status = self.status.value if hasattr(self.status, "value") else self.status + if status == IssueStatus.CLOSED.value: + return "closed" + return "dispositioned" if self.dispositioned else "undispositioned" + + @property + def is_blocking(self) -> bool: + """True when this issue currently holds work: undispositioned with + non-advisory containment.""" + containment = ( + self.containment.value if hasattr(self.containment, "value") else self.containment + ) + return ( + self.deleted_at is None + and containment != Containment.ADVISORY.value + and self.disp_state == "undispositioned" + ) + + @property + def self_dispositioned(self) -> bool: + """Signer == raiser. Visible, not forbidden — a two-person shop cannot + always separate roles; the record can always say so.""" + return ( + self.raised_by_id is not None + and self.dispositioned_by_id is not None + and self.raised_by_id == self.dispositioned_by_id + ) + def __repr__(self) -> str: return f"" diff --git a/src/opal/mcp/server.py b/src/opal/mcp/server.py index 0bed4d0..3bf02fe 100644 --- a/src/opal/mcp/server.py +++ b/src/opal/mcp/server.py @@ -12,6 +12,7 @@ from mcp.types import TextContent, Tool from sqlalchemy import func +from opal.api.routes.issues import _recheck_instance_completion from opal.config import get_active_project, get_active_settings from opal.core.audit import get_model_dict, log_create, log_delete, log_update from opal.core.designators import ( @@ -20,6 +21,7 @@ generate_requirement_number, generate_risk_number, ) +from opal.core.holds import get_holds_payload, holding_readout from opal.core.numbering import ( PartNumberError, next_part_number, @@ -56,7 +58,13 @@ User, Workcenter, ) -from opal.db.models.issue import IssuePriority, IssueStatus, IssueType +from opal.db.models.issue import ( + Containment, + DispositionType, + IssuePriority, + IssueStatus, + IssueType, +) from opal.db.models.part import TrackingType from opal.db.models.procedure import ProcedureStatus, ProcedureType, UsageType from opal.db.models.purchase import PurchaseStatus @@ -712,13 +720,17 @@ async def list_tools() -> list[Tool]: # Issues Tool( name="list_issues", - description="List issues, optionally filtered by status or type", + description="List issues, optionally filtered by status, disposition state, or type", inputSchema={ "type": "object", "properties": { "status": { "type": "string", - "description": "Filter by status: open, investigating, disposition_pending, disposition_approved, closed", + "description": "Filter by status: open, closed", + }, + "disp_state": { + "type": "string", + "description": "Filter by disposition state: undispositioned, dispositioned, closed", }, "issue_type": { "type": "string", @@ -733,8 +745,13 @@ async def list_tools() -> list[Tool]: }, ), Tool( - name="create_issue", - description="Create a new issue to track a problem, task, or improvement", + name="raise_issue", + description=( + "Raise an issue (problem, task, improvement, or non-conformance). " + "Containment declares what the issue holds while undispositioned: " + "step | op | wo | advisory (default advisory; execution-raised " + "issues should use step and set raised_step_id)." + ), inputSchema={ "type": "object", "properties": { @@ -756,10 +773,110 @@ async def list_tools() -> list[Tool]: "description": "Priority: low, medium, high, critical (default: medium)", "default": "medium", }, + "containment": { + "type": "string", + "description": "What this issue holds while undispositioned: step, op, wo, advisory (default: advisory)", + "default": "advisory", + }, + "should_be": { + "type": "string", + "description": "Expected condition (the NC pair, captured at discovery)", + }, + "actual": { + "type": "string", + "description": "Actual condition", + }, + "procedure_instance_id": { + "type": "integer", + "description": "Work order this issue belongs to (required for non-advisory containment)", + }, + "raised_step_id": { + "type": "integer", + "description": "Step execution ID where it was found", + }, + "containment_step_id": { + "type": "integer", + "description": "Boundary step execution ID ('resolve by 3.7'); blocks that step's START", + }, + "part_id": {"type": "integer"}, + "assigned_to_id": {"type": "integer"}, + "user_id": { + "type": "integer", + "description": "Human user raising the issue", + }, }, "required": ["title"], }, ), + Tool( + name="sign_disposition", + description=( + "Sign an issue's disposition — the signature releases every " + "containment the issue holds, immediately. user_id must be the " + "human making the decision." + ), + inputSchema={ + "type": "object", + "properties": { + "issue_id": {"type": "integer"}, + "user_id": { + "type": "integer", + "description": "Human user signing the disposition", + }, + "disposition_type": { + "type": "string", + "description": "use_as_is, rework, repair, scrap, return_to_supplier, no_defect", + }, + "rationale": { + "type": "string", + "description": "Disposition rationale", + }, + }, + "required": ["issue_id", "user_id", "disposition_type", "rationale"], + }, + ), + Tool( + name="set_containment", + description=( + "Change an issue's containment scope. Narrowing (e.g. op → step, " + "or anything → advisory) releases a hold and requires a note, " + "which is recorded on the issue." + ), + inputSchema={ + "type": "object", + "properties": { + "issue_id": {"type": "integer"}, + "containment": { + "type": "string", + "description": "step, op, wo, advisory", + }, + "containment_step_id": {"type": "integer"}, + "note": { + "type": "string", + "description": "Required when narrowing scope", + }, + "user_id": {"type": "integer"}, + }, + "required": ["issue_id", "containment"], + }, + ), + Tool( + name="get_holds", + description=( + "Blocking state for a work order: every undispositioned issue " + "holding it and what each blocks (are we held and why)." + ), + inputSchema={ + "type": "object", + "properties": { + "execution_id": { + "type": "integer", + "description": "Procedure instance (work order) ID", + }, + }, + "required": ["execution_id"], + }, + ), # Risks Tool( name="list_risks", @@ -1646,8 +1763,14 @@ async def call_tool(name: str, arguments: dict) -> list[TextContent]: # Issues elif name == "list_issues": return await _list_issues(db, arguments) - elif name == "create_issue": - return await _create_issue(db, arguments) + elif name in ("raise_issue", "create_issue"): + return await _raise_issue(db, arguments) + elif name == "sign_disposition": + return await _sign_disposition(db, arguments) + elif name == "set_containment": + return await _set_containment(db, arguments) + elif name == "get_holds": + return await _get_holds(db, arguments) # Risks elif name == "list_risks": @@ -2235,6 +2358,23 @@ async def _add_procedure_step(db, args: dict) -> list[TextContent]: ) +def _issue_summary(db, issue: Issue) -> dict: + """Issue facts shared by the MCP issue tools.""" + return { + "id": issue.id, + "issue_number": issue.issue_number, + "title": issue.title, + "type": issue.issue_type.value if hasattr(issue.issue_type, "value") else issue.issue_type, + "status": issue.status.value if hasattr(issue.status, "value") else issue.status, + "disp_state": issue.disp_state, + "priority": issue.priority.value if hasattr(issue.priority, "value") else issue.priority, + "containment": issue.containment.value + if hasattr(issue.containment, "value") + else issue.containment, + "holding": [t["label"] for t in holding_readout(db, issue)], + } + + async def _list_issues(db, args: dict) -> list[TextContent]: """List issues with optional filtering.""" query = db.query(Issue).filter(Issue.deleted_at.is_(None)) @@ -2242,6 +2382,16 @@ async def _list_issues(db, args: dict) -> list[TextContent]: if args.get("status"): query = query.filter(Issue.status == args["status"]) + if args.get("disp_state"): + signed = (Issue.disposition_type.isnot(None)) & (Issue.dispositioned_at.isnot(None)) + disp_state = args["disp_state"] + if disp_state == "closed": + query = query.filter(Issue.status == IssueStatus.CLOSED) + elif disp_state == "dispositioned": + query = query.filter(Issue.status != IssueStatus.CLOSED, signed) + elif disp_state == "undispositioned": + query = query.filter(Issue.status != IssueStatus.CLOSED, ~signed) + if args.get("issue_type"): query = query.filter(Issue.issue_type == args["issue_type"]) @@ -2254,9 +2404,14 @@ async def _list_issues(db, args: dict) -> list[TextContent]: "issues": [ { "id": i.id, + "issue_number": i.issue_number, "title": i.title, "type": i.issue_type.value if hasattr(i.issue_type, "value") else i.issue_type, "status": i.status.value if hasattr(i.status, "value") else i.status, + "disp_state": i.disp_state, + "containment": i.containment.value + if hasattr(i.containment, "value") + else i.containment, "priority": i.priority.value if hasattr(i.priority, "value") else i.priority, } for i in issues @@ -2265,10 +2420,11 @@ async def _list_issues(db, args: dict) -> list[TextContent]: ) -async def _create_issue(db, args: dict) -> list[TextContent]: - """Create a new issue.""" +async def _raise_issue(db, args: dict) -> list[TextContent]: + """Raise a new issue with the capture fields.""" issue_type = args.get("issue_type", "task") priority = args.get("priority", "medium") + containment = args.get("containment", "advisory") issue = Issue( issue_number=generate_issue_number(db), @@ -2277,27 +2433,136 @@ async def _create_issue(db, args: dict) -> list[TextContent]: issue_type=IssueType(issue_type), status=IssueStatus.OPEN, priority=IssuePriority(priority), + containment=Containment(containment), + containment_step_id=args.get("containment_step_id"), + should_be=args.get("should_be"), + actual=args.get("actual"), + procedure_instance_id=args.get("procedure_instance_id"), + raised_step_id=args.get("raised_step_id"), + raised_by_id=args.get("user_id"), + part_id=args.get("part_id"), + assigned_to_id=args.get("assigned_to_id"), ) db.add(issue) db.flush() - log_create(db, issue) + log_create(db, issue, args.get("user_id")) db.commit() db.refresh(issue) return json_response( { "success": True, - "message": f"Created issue '{issue.title}' with ID {issue.id}", - "issue": { - "id": issue.id, - "title": issue.title, - "type": issue_type, - "priority": priority, - }, + "message": f"Raised {issue.issue_number} '{issue.title}'", + "issue": _issue_summary(db, issue), + } + ) + + +async def _sign_disposition(db, args: dict) -> list[TextContent]: + """Sign a disposition — releases every containment the issue holds.""" + issue = db.query(Issue).filter(Issue.id == args["issue_id"], Issue.deleted_at.is_(None)).first() + if not issue: + return json_response({"success": False, "error": "Issue not found"}) + + status = issue.status.value if hasattr(issue.status, "value") else issue.status + if status == IssueStatus.CLOSED.value: + return json_response({"success": False, "error": "Issue is closed"}) + if issue.dispositioned: + return json_response({"success": False, "error": "Disposition already signed"}) + + try: + disposition_type = DispositionType(args["disposition_type"]) + except ValueError: + return json_response( + {"success": False, "error": f"Invalid disposition type: {args['disposition_type']}"} + ) + + released = [t["label"] for t in holding_readout(db, issue)] + + old_values = get_model_dict(issue) + issue.disposition_type = disposition_type + issue.disposition_rationale = args["rationale"] + issue.dispositioned_by_id = args["user_id"] + issue.dispositioned_at = datetime.now(UTC) + log_update(db, issue, old_values, args["user_id"]) + db.flush() + _recheck_instance_completion(db, issue) + db.commit() + db.refresh(issue) + + return json_response( + { + "success": True, + "message": f"Disposition {disposition_type.value} signed on {issue.issue_number}", + "released": released, + "self_dispositioned": issue.self_dispositioned, + "issue": _issue_summary(db, issue), } ) +async def _set_containment(db, args: dict) -> list[TextContent]: + """Change containment scope; narrowing requires a note (audited).""" + from opal.db.models.issue import CONTAINMENT_RANK + from opal.db.models.issue_comment import IssueComment + + issue = db.query(Issue).filter(Issue.id == args["issue_id"], Issue.deleted_at.is_(None)).first() + if not issue: + return json_response({"success": False, "error": "Issue not found"}) + + try: + new_containment = Containment(args["containment"]) + except ValueError: + return json_response( + {"success": False, "error": f"Invalid containment: {args['containment']}"} + ) + + old_containment = ( + issue.containment.value if hasattr(issue.containment, "value") else issue.containment + ) + narrowing = CONTAINMENT_RANK[new_containment.value] < CONTAINMENT_RANK[old_containment] + note = (args.get("note") or "").strip() + if narrowing and issue.is_blocking and not note: + return json_response( + { + "success": False, + "error": f"Narrowing containment {old_containment} → {new_containment.value} " + "releases a hold; a note is required", + } + ) + + user_id = args.get("user_id") + old_values = get_model_dict(issue) + issue.containment = new_containment + if args.get("containment_step_id") is not None: + issue.containment_step_id = args["containment_step_id"] + log_update(db, issue, old_values, user_id) + + if narrowing and note: + comment = IssueComment( + issue_id=issue.id, + user_id=user_id, + body=f"Containment narrowed {old_containment} → {new_containment.value}: {note}", + ) + db.add(comment) + db.flush() + log_create(db, comment, user_id) + + if narrowing: + db.flush() + _recheck_instance_completion(db, issue) + + db.commit() + db.refresh(issue) + + return json_response({"success": True, "issue": _issue_summary(db, issue)}) + + +async def _get_holds(db, args: dict) -> list[TextContent]: + """The controller's blocking state for a work order, as JSON.""" + return json_response(get_holds_payload(db, args["execution_id"])) + + async def _list_risks(db, args: dict) -> list[TextContent]: """List risks with optional filtering.""" query = db.query(Risk).filter(Risk.deleted_at.is_(None)) @@ -2374,7 +2639,7 @@ async def _get_project_info(db, args: dict) -> list[TextContent]: db.query(Issue) .filter( Issue.deleted_at.is_(None), - Issue.status.in_(["open", "investigating"]), + Issue.status == IssueStatus.OPEN, ) .count() ) @@ -2523,8 +2788,7 @@ async def _bulk_activate_parts(db, args: dict) -> list[TextContent]: return json_response( { "success": True, - "message": f"{len(activated)} part(s) activated by {user.name}, " - f"{len(skipped)} skipped", + "message": f"{len(activated)} part(s) activated by {user.name}, {len(skipped)} skipped", "activated": activated, "skipped": skipped, } diff --git a/src/opal/seed.py b/src/opal/seed.py index ff4efc4..8824212 100644 --- a/src/opal/seed.py +++ b/src/opal/seed.py @@ -4062,11 +4062,11 @@ def _seed_issues( title="Injector plate hole pattern out of tolerance", description='During QA inspection, bolt circle measured 0.005" outside tolerance.', issue_type=IssueType.NON_CONFORMANCE, - status=IssueStatus.INVESTIGATING, + status=IssueStatus.OPEN, priority=IssuePriority.HIGH, part_id=p["injector"].id, should_be='32x 0.040" holes on 1.500" bolt circle ±0.002"', - is_condition='Holes measured at 1.505" bolt circle — 0.005" outside tolerance on 3 of 4 quadrants', + actual='Holes measured at 1.505" bolt circle — 0.005" outside tolerance on 3 of 4 quadrants', ) db.add(nc1) db.flush() @@ -4076,11 +4076,11 @@ def _seed_issues( title="LOX tank circumferential weld porosity", description="Radiographic inspection revealed pores in weld at station 14.", issue_type=IssueType.NON_CONFORMANCE, - status=IssueStatus.DISPOSITION_PENDING, + status=IssueStatus.OPEN, priority=IssuePriority.CRITICAL, part_id=p["lox_tank"].id, should_be="Full-penetration weld with no porosity per AWS D17.1 Class A", - is_condition="Three pores detected: 0.8 mm, 0.6 mm, 0.5 mm in circumferential weld at station 14. Total aggregate porosity exceeds Class A limit.", + actual="Three pores detected: 0.8 mm, 0.6 mm, 0.5 mm in circumferential weld at station 14. Total aggregate porosity exceeds Class A limit.", ) db.add(nc2) @@ -4093,7 +4093,7 @@ def _seed_issues( priority=IssuePriority.MEDIUM, part_id=p["bulkhead_aft"].id, should_be="16 µin Ra max per AS568 gland specification for static radial seal", - is_condition="Profilometer measured 32 µin Ra on aft bulkhead bore — 2x allowable roughness", + actual="Profilometer measured 32 µin Ra on aft bulkhead bore — 2x allowable roughness", ) db.add(nc3) @@ -4106,7 +4106,7 @@ def _seed_issues( priority=IssuePriority.HIGH, part_id=p["ground_reg"].id, should_be="Regulate to 450 PSI ±10 PSI with zero external leakage", - is_condition="Bubble test shows steady stream at outlet NPT fitting at 400 PSI. Leak rate ~5 cc/min.", + actual="Bubble test shows steady stream at outlet NPT fitting at 400 PSI. Leak rate ~5 cc/min.", ) db.add(nc4) @@ -4117,7 +4117,7 @@ def _seed_issues( title="Flight computer resets during pyro channel firing", description="FC reboots when e-match is fired on either pyro channel.", issue_type=IssueType.BUG, - status=IssueStatus.INVESTIGATING, + status=IssueStatus.OPEN, priority=IssuePriority.HIGH, part_id=p["fc"].id, steps_to_reproduce="1. Power on flight computer via LiPo\n2. Arm both pyro channels via software command\n3. Fire channel 1 e-match\n4. Observe FC status LED and telemetry stream", diff --git a/src/opal/tui/api_client.py b/src/opal/tui/api_client.py index 47d3a8e..70def1a 100644 --- a/src/opal/tui/api_client.py +++ b/src/opal/tui/api_client.py @@ -464,6 +464,14 @@ def update_issue(self, issue_id: int, data: dict[str, Any]) -> dict[str, Any]: resp.raise_for_status() return resp.json() + def sign_disposition(self, issue_id: int, data: dict[str, Any]) -> dict[str, Any]: + """Sign an issue disposition (releases its holds).""" + resp = self.client.post( + self._url(f"/issues/{issue_id}/disposition"), json=data, headers=self._headers() + ) + resp.raise_for_status() + return resp.json() + def list_comments(self, issue_id: int) -> list[dict[str, Any]]: """List comments on an issue.""" resp = self.client.get(self._url(f"/issues/{issue_id}/comments")) diff --git a/src/opal/tui/screens/issues.py b/src/opal/tui/screens/issues.py index fd708e7..15c2ba0 100644 --- a/src/opal/tui/screens/issues.py +++ b/src/opal/tui/screens/issues.py @@ -60,9 +60,6 @@ def build_form(self) -> ComposeResult: if self.issue: status_options = [ ("Open", "open"), - ("Investigating", "investigating"), - ("Disposition Pending", "disposition_pending"), - ("Disposition Approved", "disposition_approved"), ("Closed", "closed"), ] yield FormGroup( @@ -137,12 +134,18 @@ def build_form(self) -> ComposeResult: ("Repair", "repair"), ("Scrap", "scrap"), ("Return to Supplier", "return_to_supplier"), + ("No Defect", "no_defect"), ] yield FormGroup( "Disposition Type", Select(disp_options, id="field-disposition", prompt="Select..."), required=True, ) + yield FormGroup( + "Rationale", + TextArea(id="field-rationale"), + required=True, + ) yield FormGroup( "Root Cause", TextArea(id="field-root-cause"), @@ -158,12 +161,17 @@ def get_form_data(self) -> dict[str, Any] | None: self.show_error("Disposition type is required") return None + rationale = self.query_one("#field-rationale", TextArea).text.strip() + if not rationale: + self.show_error("Rationale is required") + return None + root_cause = self.query_one("#field-root-cause", TextArea).text.strip() corrective_action = self.query_one("#field-corrective-action", TextArea).text.strip() data: dict[str, Any] = { - "status": "disposition_approved", "disposition_type": disposition, + "disposition_rationale": rationale, } if root_cause: data["root_cause"] = root_cause @@ -291,9 +299,6 @@ def compose(self) -> ComposeResult: Horizontal( Button("All", id="filter-all", variant="primary"), Button("Open", id="filter-open"), - Button("Investigating", id="filter-investigating"), - Button("Disp Pending", id="filter-disposition_pending"), - Button("Disp Approved", id="filter-disposition_approved"), Button("Closed", id="filter-closed"), classes="filter-bar", ), @@ -409,8 +414,18 @@ def _on_disposition(self, data: dict[str, Any] | None) -> None: return client = get_client(self.app.api_url) try: - client.update_issue(detail.issue_data["id"], data) - self.notify("Disposition recorded") + issue_id = detail.issue_data["id"] + extras = {k: v for k, v in data.items() if k in ("root_cause", "corrective_action")} + if extras: + client.update_issue(issue_id, extras) + client.sign_disposition( + issue_id, + { + "disposition_type": data["disposition_type"], + "disposition_rationale": data["disposition_rationale"], + }, + ) + self.notify("Disposition signed") self.run_worker(self.load_issues()) except Exception as e: self.notify(f"Error: {e}", severity="error") diff --git a/src/opal/web/routes.py b/src/opal/web/routes.py index d129ef8..2d9e568 100644 --- a/src/opal/web/routes.py +++ b/src/opal/web/routes.py @@ -27,6 +27,7 @@ validate_password_strength, verify_payload, ) +from opal.core.holds import get_hold_state, holding_readout from opal.db.models import ( InventoryRecord, Kit, @@ -39,7 +40,7 @@ ) from opal.db.models.dataset import DataPoint, Dataset from opal.db.models.execution import InstanceStatus, ProcedureInstance -from opal.db.models.issue import Issue, IssuePriority, IssueStatus, IssueType +from opal.db.models.issue import Containment, Issue, IssuePriority, IssueStatus, IssueType from opal.db.models.procedure import MasterProcedure, ProcedureStatus, ProcedureVersion from opal.db.models.purchase import PurchaseStatus from opal.db.models.requirement import Requirement @@ -557,7 +558,7 @@ def index(request: Request, db: DbSession) -> HTMLResponse: db.query(Issue) .filter( Issue.deleted_at.is_(None), - Issue.status.in_(["open", "investigating", "disposition_pending"]), + Issue.status == IssueStatus.OPEN, ) .count() ) @@ -853,9 +854,7 @@ def parts_import(request: Request, db: DbSession) -> HTMLResponse: @router.get("/parts/new", response_class=HTMLResponse) -def parts_new( - request: Request, db: DbSession, parent_id: int | None = Query(None) -) -> HTMLResponse: +def parts_new(request: Request, db: DbSession, parent_id: int | None = Query(None)) -> HTMLResponse: """Deep link: the parts list with the create overlay open. The form never asks what the invoking context already knows — @@ -2278,7 +2277,7 @@ def _pick_default_order() -> int | None: ) context["can_finalize"] = inst_status == "completed" and has_wip - # Linked issues + # Linked issues — undispositioned holds sort first (rail ISSUES section) linked_issues = ( db.query(Issue) .filter( @@ -2287,24 +2286,35 @@ def _pick_default_order() -> int | None: ) .all() ) + disp_rank = {"undispositioned": 0, "dispositioned": 1, "closed": 2} + linked_issues.sort(key=lambda i: (not i.is_blocking, disp_rank.get(i.disp_state, 3), -i.id)) context["linked_issues"] = linked_issues + context["issue_holding_counts"] = {i.id: len(holding_readout(db, i)) for i in linked_issues} - # Step-hold lookup: open NCs that put their step on hold, keyed by step_execution_id. - holding_ncs_by_step: dict[int, list[Issue]] = {} - for iss in linked_issues: - iss_type = iss.issue_type.value if hasattr(iss.issue_type, "value") else iss.issue_type - iss_status = iss.status.value if hasattr(iss.status, "value") else iss.status - if ( - iss_type == "non_conformance" - and iss.step_execution_id - and iss_status not in ("disposition_approved", "closed") - ): - holding_ncs_by_step.setdefault(iss.step_execution_id, []).append(iss) - context["step_holding_ncs"] = holding_ncs_by_step + # Containment-derived hold state: which controls are absent, and which + # issues replace them. Keyed by step_execution_id. + hold_state = get_hold_state(db, instance.id) + step_complete_blockers: dict[int, list[Issue]] = {} + step_start_blockers: dict[int, list[Issue]] = {} + for se in instance.step_executions: + complete_blockers = hold_state.blockers_for_complete(se) + if complete_blockers: + step_complete_blockers[se.id] = complete_blockers + start_blockers = hold_state.blockers_for_start(se) + if start_blockers: + step_start_blockers[se.id] = start_blockers + context["step_complete_blockers"] = step_complete_blockers + context["step_start_blockers"] = step_start_blockers # Per-op aggregate of open NCs (op-level + any of its sub-steps). Used to # decide when to show the "+ ADD REDLINE OP" button and to populate the # modal's NC dropdown. Keyed by op.order. + open_ncs_by_step_exec: dict[int, list[Issue]] = {} + for iss in linked_issues: + iss_type = iss.issue_type.value if hasattr(iss.issue_type, "value") else iss.issue_type + iss_status = iss.status.value if hasattr(iss.status, "value") else iss.status + if iss_type == "non_conformance" and iss.raised_step_id and iss_status != "closed": + open_ncs_by_step_exec.setdefault(iss.raised_step_id, []).append(iss) open_ncs_by_op_order: dict[int, list[Issue]] = {} for op_data in ops + contingency_ops: op_step = op_data["step"] @@ -2313,15 +2323,22 @@ def _pick_default_order() -> int | None: op_exec = op_step.get("execution") bucket: list[Issue] = [] if op_exec is not None: - bucket.extend(holding_ncs_by_step.get(op_exec.id, [])) + bucket.extend(open_ncs_by_step_exec.get(op_exec.id, [])) for sub in op_data.get("sub_steps", []): sub_exec = sub.get("execution") if sub_exec is not None: - bucket.extend(holding_ncs_by_step.get(sub_exec.id, [])) + bucket.extend(open_ncs_by_step_exec.get(sub_exec.id, [])) if bucket: open_ncs_by_op_order[op_step["order"]] = bucket context["op_open_ncs_by_order"] = open_ncs_by_op_order + # Capture pre-fill: assignable users for the anomaly modal. + from opal.db.models.user import User as _User + + context["users"] = ( + db.query(_User).filter(_User.is_active == True).order_by(_User.name).all() # noqa: E712 + ) + # Gating lookup: top-level ops whose prerequisite ops haven't reached # a terminal status yet. Keyed by op.order → list of blocking step_number_str. terminal_step_statuses = {"completed", "signed_off", "skipped"} @@ -2616,7 +2633,7 @@ def issues_list(request: Request, db: DbSession) -> HTMLResponse: """Issues list page.""" context = get_base_context(request, db, "Issues - OPAL") context["types"] = [t.value for t in IssueType] - context["statuses"] = [s.value for s in IssueStatus] + context["disp_states"] = ["undispositioned", "dispositioned", "closed"] context["priorities"] = [p.value for p in IssuePriority] return templates.TemplateResponse("issues/list.html", context) @@ -2627,7 +2644,7 @@ def issues_table( db: DbSession, search: str | None = Query(None), issue_type: str | None = Query(None), - status: str | None = Query(None), + disp_state: str | None = Query(None), priority: str | None = Query(None), page: int = Query(1, ge=1), ) -> HTMLResponse: @@ -2639,28 +2656,52 @@ def issues_table( query = query.filter(Issue.title.ilike(search_term)) if issue_type: query = query.filter(Issue.issue_type == issue_type) - if status: - query = query.filter(Issue.status == status) if priority: query = query.filter(Issue.priority == priority) - issues, pagination = paginate_query(request, query.order_by(Issue.id.desc()), page, colspan=6) + signed = (Issue.disposition_type.isnot(None)) & (Issue.dispositioned_at.isnot(None)) + if disp_state == "closed": + query = query.filter(Issue.status == IssueStatus.CLOSED) + elif disp_state == "dispositioned": + query = query.filter(Issue.status != IssueStatus.CLOSED, signed) + elif disp_state == "undispositioned": + query = query.filter(Issue.status != IssueStatus.CLOSED, ~signed) + + # Undispositioned-with-containment sorts first — the only warning-weight + # on the page. + blocking = ( + (Issue.status != IssueStatus.CLOSED) & (Issue.containment != Containment.ADVISORY) & ~signed + ) + query = query.order_by(case((blocking, 0), else_=1), Issue.id.desc()) + + issues, pagination = paginate_query(request, query, page, colspan=7) def get_val(obj, attr): val = getattr(obj, attr) return val.value if hasattr(val, "value") else val + def age(dt: datetime) -> str: + """Dense relative age for index rows; full ISO 8601 in the tooltip.""" + aware = dt.replace(tzinfo=UTC) if dt.tzinfo is None else dt + delta = datetime.now(UTC) - aware + if delta.days >= 1: + return f"{delta.days}d" + hours = delta.seconds // 3600 + if hours: + return f"{hours}h" + return f"{max(delta.seconds // 60, 0)}m" + issues_data = [ { "id": i.id, "issue_number": i.issue_number, "title": i.title, "issue_type": get_val(i, "issue_type"), - "status": get_val(i, "status"), + "disp_state": i.disp_state, + "holding": len(holding_readout(db, i)) if i.is_blocking else 0, "priority": get_val(i, "priority"), "created_at": i.created_at, - "procedure_id": i.procedure_id, - "procedure_instance_id": i.procedure_instance_id, + "age": age(i.created_at), } for i in issues ] @@ -2672,11 +2713,15 @@ def get_val(obj, attr): @router.get("/issues/new", response_class=HTMLResponse) -def issues_new(request: Request, db: DbSession) -> HTMLResponse: +def issues_new( + request: Request, db: DbSession, procedure_instance_id: int | None = Query(None) +) -> HTMLResponse: """New issue form page.""" context = get_base_context(request, db, "New Issue - OPAL") context["types"] = [t.value for t in IssueType] context["priorities"] = [p.value for p in IssuePriority] + context["containments"] = [c.value for c in Containment] + context["procedure_instance_id"] = procedure_instance_id # Get procedures and users for linking (parts use the search typeahead) procedures = ( @@ -2705,11 +2750,11 @@ def issues_detail(request: Request, db: DbSession, issue_id: int) -> HTMLRespons status_code=404, ) - context = get_base_context(request, db, f"Issue {issue_id} - OPAL") + context = get_base_context(request, db, f"Issue {issue.issue_number} - OPAL") context["issue"] = issue context["types"] = [t.value for t in IssueType] - context["statuses"] = [s.value for s in IssueStatus] context["priorities"] = [p.value for p in IssuePriority] + context["containments"] = [c.value for c in Containment] from opal.db.models.attachment import Attachment from opal.db.models.issue import DispositionType @@ -2729,6 +2774,15 @@ def issues_detail(request: Request, db: DbSession, issue_id: int) -> HTMLRespons context["attachments"] = attachments context["users"] = users context["disposition_types"] = [d.value for d in DispositionType] + # HOLDING — the consequence readout: what this issue is stopping. + context["holding"] = holding_readout(db, issue) + + # Spawn-redline target: the op (level-0 order) hosting the raised step. + redline_op_order = None + if issue.raised_step is not None and issue.procedure_instance_id is not None: + raised = issue.raised_step + redline_op_order = raised.step_number if raised.level == 0 else raised.parent_step_order + context["redline_op_order"] = redline_op_order return templates.TemplateResponse("issues/detail.html", context) diff --git a/src/opal/web/templates/executions/detail.html b/src/opal/web/templates/executions/detail.html index 098fb73..4efbeb3 100644 --- a/src/opal/web/templates/executions/detail.html +++ b/src/opal/web/templates/executions/detail.html @@ -41,34 +41,70 @@ {% include 'executions/tabs/' ~ tab ~ '.html' %}
- + diff --git a/src/opal/web/templates/layouts/base.html b/src/opal/web/templates/layouts/base.html index 78cf3be..9cada09 100644 --- a/src/opal/web/templates/layouts/base.html +++ b/src/opal/web/templates/layouts/base.html @@ -17,6 +17,7 @@ + {% block head_extra %}{% endblock %} {% if demo_active %} diff --git a/src/opal/web/templates/procedures/detail.html b/src/opal/web/templates/procedures/detail.html index 61db771..f4018d0 100644 --- a/src/opal/web/templates/procedures/detail.html +++ b/src/opal/web/templates/procedures/detail.html @@ -548,6 +548,7 @@ required_data_schema: schema, }; if (form.is_contingency) data.is_contingency = form.is_contingency.checked; + if (form.strict_sequence) data.strict_sequence = form.strict_sequence.checked; try { const r = await fetch(`/api/procedures/${procedureId}/steps/${stepId}`, { method: 'PATCH', headers: getHeaders(), body: JSON.stringify(data) }); @@ -556,6 +557,43 @@ } catch (e) { errorDiv.textContent = 'Network error: ' + e.message; errorDiv.style.display = 'block'; } } +// ============ Step reference images ============ + +async function uploadStepImage(input, stepId) { + if (!input.files.length) return; + const file = input.files[0]; + const fd = new FormData(); + fd.append('file', file); + try { + const r = await fetch(`/api/procedures/${procedureId}/steps/${stepId}/images`, { + method: 'POST', body: fd, + }); + if (r.ok) window.location.reload(); + else { const err = await r.json().catch(() => ({})); alert(formatApiError(err.detail, 'Failed to add image')); } + } catch (e) { alert('Network error: ' + e.message); } + input.value = ''; +} + +async function saveStepImageCaption(stepId, imageId, caption) { + try { + await fetch(`/api/procedures/${procedureId}/steps/${stepId}/images/${imageId}`, { + method: 'PATCH', headers: getHeaders(), + body: JSON.stringify({ caption: caption.trim() || null }), + }); + } catch (e) { console.error('Failed to save caption:', e); } +} + +async function removeStepImage(stepId, imageId) { + if (!confirm('Remove this reference image from the step? Published versions keep their copy.')) return; + try { + const r = await fetch(`/api/procedures/${procedureId}/steps/${stepId}/images/${imageId}`, { method: 'DELETE' }); + if (r.ok) { + const fig = document.querySelector(`#step-images-${stepId} [data-image-id="${imageId}"]`); + if (fig) fig.remove(); + } else { const err = await r.json().catch(() => ({})); alert(formatApiError(err.detail, 'Failed to remove image')); } + } catch (e) { alert('Network error: ' + e.message); } +} + // ============ Step-parts (per step kit) ============ function showAddStepPart(stepId) { diff --git a/src/opal/web/templates/procedures/tabs/_step_editor.html b/src/opal/web/templates/procedures/tabs/_step_editor.html index 3cdeb9a..716a642 100644 --- a/src/opal/web/templates/procedures/tabs/_step_editor.html +++ b/src/opal/web/templates/procedures/tabs/_step_editor.html @@ -37,6 +37,10 @@ Contingency operation + {% endif %}
@@ -79,6 +83,27 @@ +
+
+ REFERENCE IMAGES + +
+
+ {% for img in step.images %} +
+ {{ img.caption or 'Reference image' }} + + +
+ {% else %} + No reference images + {% endfor %} +
+
+
DATA CAPTURE FIELDS diff --git a/tests/unit/test_execution_document_web.py b/tests/unit/test_execution_document_web.py new file mode 100644 index 0000000..ac16c9e --- /dev/null +++ b/tests/unit/test_execution_document_web.py @@ -0,0 +1,325 @@ +"""Web tests for the execution DOCUMENT page and its partials. + +Identity note: API auth resolves the session cookie BEFORE the bearer header +(see opal.api.deps.get_current_user), so API mutations issued through +``web_client`` are attributed to ``test_user`` — the same identity the web +page renders as ``current_user``. The mystep test verifies this explicitly +via /state. +""" + +from datetime import UTC, datetime + +from fastapi.testclient import TestClient +from sqlalchemy.orm import Session + +from opal.db.models.execution import InstanceStatus, ProcedureInstance + +# ============ builders (API-call patterns from tests/unit/test_execution.py) ============ + + +def _create_procedure_with_steps(client: TestClient) -> tuple[int, int]: + """Create a procedure with 3 top-level steps and publish it.""" + proc_response = client.post( + "/api/procedures", + json={"name": "Doc Web Procedure"}, + ) + proc_id = proc_response.json()["id"] + + client.post(f"/api/procedures/{proc_id}/steps", json={"title": "Step 1"}) + client.post(f"/api/procedures/{proc_id}/steps", json={"title": "Step 2"}) + client.post(f"/api/procedures/{proc_id}/steps", json={"title": "Step 3"}) + + version_response = client.post(f"/api/procedures/{proc_id}/publish") + version_id = version_response.json()["id"] + + return proc_id, version_id + + +def _create_instance(client: TestClient) -> int: + """Create a procedure + instance, return instance_id.""" + proc_id, _ = _create_procedure_with_steps(client) + resp = client.post( + "/api/procedure-instances", + json={"procedure_id": proc_id}, + ) + assert resp.status_code == 201, resp.text + return resp.json()["id"] + + +def _create_schema_instance(client: TestClient) -> int: + """Instance whose step 1 carries a required_data_schema (number field).""" + proc_id = client.post("/api/procedures", json={"name": "Schema Doc Proc"}).json()["id"] + client.post( + f"/api/procedures/{proc_id}/steps", + json={ + "title": "Torque bolts", + "required_data_schema": { + "fields": [ + { + "name": "torque", + "label": "TORQUE", + "type": "number", + "required": True, + "unit": "Nm", + } + ] + }, + }, + ) + client.post(f"/api/procedures/{proc_id}/steps", json={"title": "Verify"}) + client.post(f"/api/procedures/{proc_id}/publish") + resp = client.post("/api/procedure-instances", json={"procedure_id": proc_id}) + assert resp.status_code == 201, resp.text + return resp.json()["id"] + + +def _se_id(client: TestClient, instance_id: int, order: int) -> int: + """Step execution id for the snapshot order.""" + inst = client.get(f"/api/procedure-instances/{instance_id}").json() + return next(se["id"] for se in inst["step_executions"] if se["step_number"] == order) + + +def _upload_capture(client: TestClient, se_id: int, note: str) -> dict: + resp = client.post( + "/api/attachments/upload", + files={"file": ("evidence.png", b"\x89PNG\r\n\x1a\nfake", "image/png")}, + data={"step_execution_id": str(se_id), "kind": "capture", "note": note}, + ) + assert resp.status_code == 201, resp.text + return resp.json() + + +def _op_section_class_attr(html: str, order: int) -> str: + """The
opening text immediately preceding id="op-{order}".""" + head = html.split(f'id="op-{order}"')[0] + return head.rsplit(" str: + """Markup between id="op-{order}" and its closing
.""" + return html.split(f'id="op-{order}"')[1].split("")[0] + + +# ============ 1. document page renders ============ + + +def test_document_page_renders(web_client: TestClient): + instance_id = _create_instance(web_client) + + resp = web_client.get(f"/executions/{instance_id}") + assert resp.status_code == 200 + body = resp.text + assert "DOCUMENT" in body + assert "exec-head" in body + assert "op-card" in body + assert "execdoc.js" in body + assert "execdoc.css" in body + # The META and OPERATIONS tabs are gone. + assert ">META<" not in body + assert ">OPERATIONS<" not in body + + +# ============ 2. legacy tab aliases ============ + + +def test_legacy_tab_aliases_land_on_document(web_client: TestClient): + instance_id = _create_instance(web_client) + + for legacy in ("meta", "operations"): + resp = web_client.get(f"/executions/{instance_id}?tab={legacy}") + assert resp.status_code == 200, legacy + assert "exec-doc-layout" in resp.text, legacy + + +# ============ 3. expansion policy ============ + + +def test_completed_op_collapses_pending_op_does_not(web_client: TestClient): + instance_id = _create_instance(web_client) + + # Complete step 1 (a flat top-level step = its own OP card). + resp = web_client.post( + f"/api/procedure-instances/{instance_id}/steps/1/complete", + json={}, + ) + assert resp.status_code == 200, resp.text + + page = web_client.get(f"/executions/{instance_id}") + assert page.status_code == 200 + html = page.text + + # Completed OP: collapsed, body hidden. + assert "is-collapsed" in _op_section_class_attr(html, 1) + assert '
+ {% if part_requirements %} {{ ok.th("REQ", width="110px") }} @@ -31,6 +32,7 @@ {{ ok.th("", width="40px") }} + {% endif %} {% for row in part_requirements %} {% set r = row.req %} @@ -52,7 +54,7 @@ {% else %} - + {% endfor %}
×
—
—
@@ -73,6 +75,7 @@
+ {% if test_templates %} {{ ok.th("NAME") }} @@ -81,6 +84,7 @@ {{ ok.th("", width="40px") }} + {% endif %} {% for tt in test_templates %} @@ -90,7 +94,7 @@ {% else %} - + {% endfor %}
×
—
—
diff --git a/src/opal/web/templates/parts/_section_stock.html b/src/opal/web/templates/parts/_section_stock.html index a900b1c..d4e0113 100644 --- a/src/opal/web/templates/parts/_section_stock.html +++ b/src/opal/web/templates/parts/_section_stock.html @@ -14,6 +14,7 @@ + {% if bom_lines %} {{ ok.th("PN", width="130px") }} @@ -24,6 +25,7 @@ {{ ok.th("", width="64px") }} + {% endif %} {% for line in bom_lines %} @@ -38,7 +40,7 @@ {% else %} - + {% endfor %}
—
—
@@ -64,6 +66,7 @@ + {% if inventory_records %} {{ ok.th("OPAL #", width="120px") }} @@ -72,6 +75,7 @@ {{ ok.th("LOT", width="140px") }} + {% endif %} {% for inv in inventory_records %} @@ -81,7 +85,7 @@ {% else %} - + {% endfor %}
{{ inv.lot_number or '—' }}
—
—
@@ -102,6 +106,7 @@ {{ open_po_lines | length }} + {% if open_po_lines %} {{ ok.th("PO", width="120px") }} @@ -109,6 +114,7 @@ {{ ok.th("RECEIVED", width="100px", align="right") }} + {% endif %} {% for line in open_po_lines %} @@ -117,7 +123,7 @@ {% else %} - + {% endfor %}
{{ '%g' % line.qty_received }}/{{ '%g' % line.qty_ordered }}
—
—
@@ -133,6 +139,7 @@ + {% if supplier_entries %} {{ ok.th("SUPPLIER") }} @@ -141,6 +148,7 @@ {{ ok.th("", width="40px") }} + {% endif %} {% for entry in supplier_entries %} @@ -150,7 +158,7 @@ {% else %} - + {% endfor %}
×
—
—
@@ -163,6 +171,7 @@ {{ where_used | length }} + {% if where_used %} {{ ok.th("PN", width="130px") }} @@ -171,6 +180,7 @@ {{ ok.th("REFDES", width="90px") }} + {% endif %} {% for line in where_used %} @@ -180,7 +190,7 @@ {% else %} - + {% endfor %}
{{ line.reference_designator or '—' }}
—
—
@@ -193,6 +203,7 @@ {{ (kit_usages | length) + (step_kit_usages | length) }} + {% if kit_usages or step_kit_usages %} {{ ok.th("PROCEDURE") }} @@ -202,6 +213,7 @@ {{ ok.th("STATUS", width="110px") }} + {% endif %} {% for kit in kit_usages %} @@ -222,7 +234,7 @@ {% endfor %} {% if not (kit_usages or step_kit_usages) %} - + {% endif %}
—
—
@@ -235,6 +247,7 @@ {{ consumption_history | length }}{{ '+' if consumption_history | length == 50 }} + {% if consumption_history %} {{ ok.th("DATE", width="110px") }} @@ -243,6 +256,7 @@ {{ ok.th("NOTES") }} + {% endif %} {% for c in consumption_history %} @@ -258,7 +272,7 @@ {% else %} - + {% endfor %}
{{ c.notes or '—' }}
—
—
From 2eaca7fa94629fef89e3f02fecf5691a656ffe3f Mon Sep 17 00:00:00 2001 From: CST-100 <42722690+CST-100@users.noreply.github.com> Date: Fri, 12 Jun 2026 11:18:34 -0700 Subject: [PATCH 07/79] Fix permanently-visible lightbox dimming the execution document MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit .execdoc-lightbox sets display:flex, and author display rules override the UA stylesheet's [hidden] default — so the full-viewport rgba(0,0,0,.88) overlay rendered on page load and swallowed every click. Add the standard global guard so the hidden attribute always wins; this also fixes the JOIN/LEAVE buttons rendering before presence is known (.btn sets display). Co-Authored-By: Claude Fable 5 --- src/opal/web/static/css/main.css | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/opal/web/static/css/main.css b/src/opal/web/static/css/main.css index 23fca28..db31b97 100644 --- a/src/opal/web/static/css/main.css +++ b/src/opal/web/static/css/main.css @@ -78,6 +78,12 @@ padding: 0; } +/* The hidden attribute must win over any class that sets display + (author display rules override the UA's [hidden] default). */ +[hidden] { + display: none !important; +} + html { font-size: 14px; } From 7ef96b234865fff2be314f0e835b971260828c22 Mon Sep 17 00:00:00 2001 From: CST-100 <42722690+CST-100@users.noreply.github.com> Date: Fri, 12 Jun 2026 11:18:44 -0700 Subject: [PATCH 08/79] Work order semantics: CUT/IN WORK statuses, batch cut, WO-first list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - InstanceStatus: pending -> CUT (untouched since cut from the master procedure), in_progress -> IN_WORK. Real rename in the enum and data (migration b3d1f0a2c4e6), not a display mapping — one vocabulary for one fact across UI, API, and MCP. Step statuses unchanged. - Executions list: WORK ORDER is the leading, linked column; the database-id column is gone (voice rule: ids never render in lists). - Batch cut: InstanceCreate.quantity (1-100) cuts N identical work orders in one request, each with its own generated WO number; an explicit work_order_number requires quantity 1. The create endpoint loops the full per-instance build (steps, BUILD as-built allocation, audit log); draft-part validation hoisted ahead of the loop. - Cut form: quantity field, CUT verb, redirect to the list for batches. Co-Authored-By: Claude Fable 5 --- ...e6_rename_instance_statuses_cut_in_work.py | 36 +++++++ src/opal/api/routes/execution.py | 94 +++++++++++-------- src/opal/core/execution_flow.py | 6 +- src/opal/db/models/execution.py | 8 +- src/opal/seed.py | 2 +- src/opal/web/routes.py | 4 +- .../templates/executions/_exec_macros.html | 2 +- .../web/templates/executions/_op_card.html | 2 +- src/opal/web/templates/executions/_rail.html | 2 +- .../web/templates/executions/_step_row.html | 4 +- src/opal/web/templates/executions/detail.html | 4 +- src/opal/web/templates/executions/list.html | 7 +- src/opal/web/templates/executions/new.html | 18 +++- .../web/templates/executions/table_rows.html | 5 +- .../templates/executions/tabs/kitting.html | 8 +- src/opal/web/templates/opalkit/_macros.html | 4 +- tests/unit/test_execution.py | 38 +++++++- tests/unit/test_mcp_execution.py | 6 +- tests/unit/test_step_claims.py | 8 +- 19 files changed, 173 insertions(+), 85 deletions(-) create mode 100644 migrations/versions/b3d1f0a2c4e6_rename_instance_statuses_cut_in_work.py diff --git a/migrations/versions/b3d1f0a2c4e6_rename_instance_statuses_cut_in_work.py b/migrations/versions/b3d1f0a2c4e6_rename_instance_statuses_cut_in_work.py new file mode 100644 index 0000000..ce71e5a --- /dev/null +++ b/migrations/versions/b3d1f0a2c4e6_rename_instance_statuses_cut_in_work.py @@ -0,0 +1,36 @@ +"""Rename instance statuses: pending -> cut, in_progress -> in_work + +Work orders are "cut" from a master procedure; untouched instances are +CUT, started instances are IN WORK. Step statuses are unchanged. + +Revision ID: b3d1f0a2c4e6 +Revises: 26297a8e0c76 +Create Date: 2026-06-12 12:00:00.000000 + +""" +from typing import Sequence, Union + +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision: str = 'b3d1f0a2c4e6' +down_revision: Union[str, None] = '26297a8e0c76' +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + +_instance = sa.table('procedure_instance', sa.column('status', sa.String)) + + +def upgrade() -> None: + op.execute(_instance.update().where(_instance.c.status == 'pending').values(status='cut')) + op.execute( + _instance.update().where(_instance.c.status == 'in_progress').values(status='in_work') + ) + + +def downgrade() -> None: + op.execute(_instance.update().where(_instance.c.status == 'cut').values(status='pending')) + op.execute( + _instance.update().where(_instance.c.status == 'in_work').values(status='in_progress') + ) diff --git a/src/opal/api/routes/execution.py b/src/opal/api/routes/execution.py index 95bfd24..09ec384 100644 --- a/src/opal/api/routes/execution.py +++ b/src/opal/api/routes/execution.py @@ -122,6 +122,9 @@ class InstanceCreate(BaseModel): procedure_id: int version_id: int | None = Field(None, description="If not provided, uses current version") work_order_number: str | None = None + quantity: int = Field( + 1, ge=1, le=100, description="Number of identical work orders to cut (batch cut)" + ) scheduled_start_at: datetime | None = None target_completion_at: datetime | None = None priority: int = 0 @@ -264,7 +267,11 @@ def create_instance( db: DbSession, user_id: CurrentUserId, ) -> InstanceResponse: - """Start a new procedure instance.""" + """Cut work order(s) from a procedure version. + + quantity > 1 cuts a batch of identical work orders, each with its own + generated WO number; the response carries the first cut. + """ # Validate procedure exists procedure = ( db.query(MasterProcedure) @@ -289,66 +296,71 @@ def create_instance( .first() ) - # Generate work order number if not provided - work_order_number = data.work_order_number or generate_work_order_number(db) - - # Create instance - instance = ProcedureInstance( - procedure_id=data.procedure_id, - version_id=version.id, - work_order_number=work_order_number, - status=InstanceStatus.PENDING, - started_by_id=user_id, - scheduled_start_at=data.scheduled_start_at, - target_completion_at=data.target_completion_at, - priority=data.priority, - target_entity=data.target_entity, - ) - db.add(instance) - db.flush() + if data.work_order_number and data.quantity > 1: + raise HTTPException( + status_code=400, + detail="Explicit work order number only applies to a single cut", + ) - # Create step executions from version snapshot, preserving hierarchy + # Step hierarchy from the version snapshot — shared by every cut. steps = version.content.get("steps", []) - - # Build a map of step order -> parent step order for hierarchy order_to_parent: dict[int, int | None] = {} for step in steps: parent_id = step.get("parent_step_id") if parent_id: - # Find parent's order parent_step = next((s for s in steps if s.get("id") == parent_id), None) order_to_parent[step["order"]] = parent_step["order"] if parent_step else None else: order_to_parent[step["order"]] = None - for step in steps: - step_exec = StepExecution( - instance_id=instance.id, - step_number=step["order"], - step_number_str=step.get("step_number", str(step["order"])), - level=step.get("level", 0), - parent_step_order=order_to_parent.get(step["order"]), - status=StepStatus.PENDING, - ) - db.add(step_exec) - - # Auto-allocate output assemblies for BUILD procedures + # BUILD procedures auto-allocate output assemblies. An as-built + # allocation is a physical reference — draft output parts block the + # whole cut (raises DraftPartsBlocked -> 409) before anything exists. proc_type = procedure.procedure_type if hasattr(proc_type, "value"): proc_type = proc_type.value + outputs = [] if proc_type == ProcedureType.BUILD.value: outputs = ( db.query(ProcedureOutput) .filter(ProcedureOutput.procedure_id == data.procedure_id) .all() ) - # An as-built allocation is a physical reference — draft output - # parts block it (raises DraftPartsBlocked -> 409) ensure_parts_active( db, [output.part_id for output in outputs], - f"work order {work_order_number} as-built allocation", + f"{procedure.name} as-built allocation", + ) + + first_instance: ProcedureInstance | None = None + for _ in range(data.quantity): + work_order_number = data.work_order_number or generate_work_order_number(db) + + instance = ProcedureInstance( + procedure_id=data.procedure_id, + version_id=version.id, + work_order_number=work_order_number, + status=InstanceStatus.CUT, + started_by_id=user_id, + scheduled_start_at=data.scheduled_start_at, + target_completion_at=data.target_completion_at, + priority=data.priority, + target_entity=data.target_entity, ) + db.add(instance) + db.flush() + + for step in steps: + step_exec = StepExecution( + instance_id=instance.id, + step_number=step["order"], + step_number_str=step.get("step_number", str(step["order"])), + level=step.get("level", 0), + parent_step_order=order_to_parent.get(step["order"]), + status=StepStatus.PENDING, + ) + db.add(step_exec) + for output in outputs: output_part = db.query(Part).filter(Part.id == output.part_id).first() if not output_part: @@ -385,7 +397,11 @@ def create_instance( # Link inventory record to its production inv_record.source_production_id = production.id - log_create(db, instance, user_id) + log_create(db, instance, user_id) + if first_instance is None: + first_instance = instance + + instance = first_instance db.commit() db.refresh(instance) @@ -497,7 +513,7 @@ def update_instance( instance.status = new_status # Set timestamps based on status - if new_status == InstanceStatus.IN_PROGRESS and not instance.started_at: + if new_status == InstanceStatus.IN_WORK and not instance.started_at: instance.started_at = datetime.now(UTC) elif new_status in [InstanceStatus.COMPLETED, InstanceStatus.ABORTED]: instance.completed_at = datetime.now(UTC) diff --git a/src/opal/core/execution_flow.py b/src/opal/core/execution_flow.py index 0f2bfa8..345e3be 100644 --- a/src/opal/core/execution_flow.py +++ b/src/opal/core/execution_flow.py @@ -407,7 +407,7 @@ def claim_step( """Claim = START. One user per step; one active step per user (claiming another supersedes the first). Checks the start gates.""" inst_status = _status_value(instance.status) - if inst_status not in (InstanceStatus.PENDING.value, InstanceStatus.IN_PROGRESS.value): + if inst_status not in (InstanceStatus.CUT.value, InstanceStatus.IN_WORK.value): raise FlowError("Instance is not active") existing = active_claim_for_step(db, step_exec.id) @@ -431,8 +431,8 @@ def claim_step( release_user_claims(db, instance.id, user.id, ClaimReleaseReason.SUPERSEDED) instance_started = False - if inst_status == InstanceStatus.PENDING.value: - instance.status = InstanceStatus.IN_PROGRESS + if inst_status == InstanceStatus.CUT.value: + instance.status = InstanceStatus.IN_WORK instance.started_at = datetime.now(UTC) db.query(InventoryProduction).filter( InventoryProduction.procedure_instance_id == instance.id, diff --git a/src/opal/db/models/execution.py b/src/opal/db/models/execution.py index e81ef10..e51e194 100644 --- a/src/opal/db/models/execution.py +++ b/src/opal/db/models/execution.py @@ -11,10 +11,10 @@ class InstanceStatus(str, Enum): - """Procedure instance status.""" + """Procedure instance (work order) status.""" - PENDING = "pending" - IN_PROGRESS = "in_progress" + CUT = "cut" # Untouched since cut from the master procedure + IN_WORK = "in_work" COMPLETED = "completed" ABORTED = "aborted" @@ -47,7 +47,7 @@ class ProcedureInstance(Base, IdMixin, TimestampMixin): String(100), nullable=True, index=True, comment="For grouping related instances" ) status: Mapped[InstanceStatus] = mapped_column( - String(20), nullable=False, default=InstanceStatus.PENDING + String(20), nullable=False, default=InstanceStatus.CUT ) started_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True), nullable=True) completed_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True), nullable=True) diff --git a/src/opal/seed.py b/src/opal/seed.py index 4365efd..ddf603e 100644 --- a/src/opal/seed.py +++ b/src/opal/seed.py @@ -3816,7 +3816,7 @@ def _seed_versions_and_executions( procedure_id=proc.id, version_id=version.id, work_order_number=generate_work_order_number(db), - status=InstanceStatus.IN_PROGRESS, + status=InstanceStatus.IN_WORK, started_at=now - timedelta(hours=2), priority=1, ) diff --git a/src/opal/web/routes.py b/src/opal/web/routes.py index e8ad710..625cf1b 100644 --- a/src/opal/web/routes.py +++ b/src/opal/web/routes.py @@ -565,7 +565,7 @@ def index(request: Request, db: DbSession) -> HTMLResponse: .count() ) context["in_progress_count"] = ( - db.query(ProcedureInstance).filter(ProcedureInstance.status == "in_progress").count() + db.query(ProcedureInstance).filter(ProcedureInstance.status == "in_work").count() ) context["risks_count"] = ( db.query(Risk) @@ -1935,7 +1935,7 @@ def executions_table( query = query.filter(ProcedureInstance.status == status) rows, pagination = paginate_query( - request, query.order_by(ProcedureInstance.id.desc()), page, colspan=7 + request, query.order_by(ProcedureInstance.id.desc()), page, colspan=6 ) instances_data = [] diff --git a/src/opal/web/templates/executions/_exec_macros.html b/src/opal/web/templates/executions/_exec_macros.html index c21a883..183052c 100644 --- a/src/opal/web/templates/executions/_exec_macros.html +++ b/src/opal/web/templates/executions/_exec_macros.html @@ -11,7 +11,7 @@ {% set claim_info = claims_by_se.get(se_id) if se_id else none %} {% set bound = bound_holds_by_se.get(se_id, []) if se_id else [] %} {% set raised = step_holding_ncs.get(se_id, []) if se_id else [] %} -{% set active_inst = inst_status in ['pending', 'in_progress'] %} +{% set active_inst = inst_status in ['cut', 'in_work'] %} {% if step.status in ['completed', 'signed_off'] %} ✓ {% if exec and exec.completed_by_user %}{{ exec.completed_by_user.name | initials }}{% endif %} {{ hm(exec.completed_at) if exec else '' }} {% elif step.status == 'skipped' %} diff --git a/src/opal/web/templates/executions/_op_card.html b/src/opal/web/templates/executions/_op_card.html index df9052c..1f21cb4 100644 --- a/src/opal/web/templates/executions/_op_card.html +++ b/src/opal/web/templates/executions/_op_card.html @@ -22,7 +22,7 @@ {% if op_vs.get('caution') %}⚠{% endif %} {{ op_data.completed_steps }}/{{ op_data.total_steps }}{% if op_terminal %} ✓{% endif %} - {% if inst_status in ['pending', 'in_progress'] %} + {% if inst_status in ['cut', 'in_work'] %} {% if op_holds %} ⛔ {% for i in op_holds %}{{ i.issue_number }}{% if not loop.last %} {% endif %}{% endfor %} {% elif op_gated %} diff --git a/src/opal/web/templates/executions/_rail.html b/src/opal/web/templates/executions/_rail.html index 1a5c53e..9eb4d62 100644 --- a/src/opal/web/templates/executions/_rail.html +++ b/src/opal/web/templates/executions/_rail.html @@ -89,7 +89,7 @@ {% else %} {{ ok.empty_line("Attachments", "attach", onclick="showAttachModal(null, 'WO')") }} {% endif %} - {% if (capture_attachments or wo_attachments) and inst_status in ['pending', 'in_progress', 'completed'] %} + {% if (capture_attachments or wo_attachments) and inst_status in ['cut', 'in_work', 'completed'] %}
diff --git a/src/opal/web/templates/executions/_step_row.html b/src/opal/web/templates/executions/_step_row.html index 81bce5c..b82ba41 100644 --- a/src/opal/web/templates/executions/_step_row.html +++ b/src/opal/web/templates/executions/_step_row.html @@ -72,7 +72,7 @@ {% endfor %} - {% if step.status == 'in_progress' and inst_status == 'in_progress' %} + {% if step.status == 'in_progress' and inst_status == 'in_work' %}
@@ -97,7 +97,7 @@ {% endif %} - {% if inst_status in ['pending', 'in_progress'] %} + {% if inst_status in ['cut', 'in_work'] %}
{% if step.status in ['pending', 'in_progress'] %} diff --git a/src/opal/web/templates/executions/detail.html b/src/opal/web/templates/executions/detail.html index 614e33b..3ca372f 100644 --- a/src/opal/web/templates/executions/detail.html +++ b/src/opal/web/templates/executions/detail.html @@ -28,14 +28,14 @@ - {% if inst_status in ['pending', 'in_progress'] %} + {% if inst_status in ['cut', 'in_work'] %} {% endif %} {% if inst_status == 'completed' %} {{ ok.btn("REPORT", href="/executions/" ~ instance.id ~ "/report", size="sm", attrs='target="_blank" rel="noopener"') }} {% endif %} - {% if inst_status in ['pending', 'in_progress'] %} + {% if inst_status in ['cut', 'in_work'] %} {% endif %} diff --git a/src/opal/web/templates/executions/list.html b/src/opal/web/templates/executions/list.html index 5d50f45..813fd95 100644 --- a/src/opal/web/templates/executions/list.html +++ b/src/opal/web/templates/executions/list.html @@ -7,7 +7,7 @@ {% endblock %} {% block content %} -{% call ok.panel("EXECUTIONS", actions=ok.btn("+ NEW EXECUTION", href="/executions/new", variant="primary")) %} +{% call ok.panel("EXECUTIONS", actions=ok.btn("+ CUT WO", href="/executions/new", variant="primary")) %}
- {{ ok.field("work_order_number", "WORK ORDER NUMBER", placeholder="Optional - for grouping related executions") }} + {{ ok.field("work_order_number", "WORK ORDER NUMBER", placeholder="Optional - generated if blank") }} + +
+ + +
@@ -38,7 +44,7 @@ {% endif %} {{ ok.form_messages() }} - {{ ok.form_actions(cancel_href="/executions", submit_text="START") }} + {{ ok.form_actions(cancel_href="/executions", submit_text="CUT") }} {% endcall %} @@ -50,9 +56,11 @@ const errorDiv = document.getElementById('form-error'); errorDiv.style.display = 'none'; + const quantity = parseInt(form.quantity.value, 10) || 1; const data = { procedure_id: parseInt(form.procedure_id.value), work_order_number: form.work_order_number.value || null, + quantity: quantity, }; if (form.entity_type.value || form.entity_id.value || form.entity_label.value) { @@ -77,7 +85,7 @@ if (response.ok) { const instance = await response.json(); - window.location.href = '/executions/' + instance.id; + window.location.href = quantity > 1 ? '/executions' : '/executions/' + instance.id; } else { const error = await response.json(); errorDiv.textContent = formatApiError(error.detail, 'Failed to start execution'); diff --git a/src/opal/web/templates/executions/table_rows.html b/src/opal/web/templates/executions/table_rows.html index f5c4895..b9b9c7f 100644 --- a/src/opal/web/templates/executions/table_rows.html +++ b/src/opal/web/templates/executions/table_rows.html @@ -2,10 +2,9 @@ {% if instances %} {% for inst in instances %} - {{ ok.id_cell(inst.id, "/executions/" ~ inst.id) }} + {{ inst.work_order }} {{ inst.procedure_name }} {{ ok.mono("v" ~ inst.version_number) }} - {{ ok.mono(inst.work_order) }} {{ ok.status(inst.status | upper | replace('_', ' ')) }} {{ ok.mono(inst.completed_steps ~ "/" ~ inst.total_steps) }} {% if inst.started_at %}{{ ok.timestamp(inst.started_at) }}{% else %}{{ ok.timestamp(inst.created_at) }}{% endif %} @@ -13,7 +12,7 @@ {% endfor %} {% else %} - + {{ ok.empty("No executions found") }} diff --git a/src/opal/web/templates/executions/tabs/kitting.html b/src/opal/web/templates/executions/tabs/kitting.html index d10c50e..d867b54 100644 --- a/src/opal/web/templates/executions/tabs/kitting.html +++ b/src/opal/web/templates/executions/tabs/kitting.html @@ -66,12 +66,12 @@ {% endfor %} - {% if inst_status in ['in_progress', 'completed'] %} + {% if inst_status in ['in_work', 'completed'] %}
- {% elif inst_status == 'pending' %} + {% elif inst_status == 'cut' %}

Parts can be consumed after the execution is started.

{% endif %} {% endif %} @@ -158,12 +158,12 @@ {% endfor %} - {% if inst_status in ['in_progress', 'completed'] %} + {% if inst_status in ['in_work', 'completed'] %}
- {% elif inst_status == 'pending' %} + {% elif inst_status == 'cut' %}

Output can be recorded after the execution is started.

{% endif %}
diff --git a/src/opal/web/templates/opalkit/_macros.html b/src/opal/web/templates/opalkit/_macros.html index 4fca3fa..4078759 100644 --- a/src/opal/web/templates/opalkit/_macros.html +++ b/src/opal/web/templates/opalkit/_macros.html @@ -34,11 +34,11 @@ {%- set v = value | lower if value else "" -%} {%- if v in ["active", "completed", "ok", "received", "published"] -%} {%- set variant_class = "status-ok" -%} - {%- elif v in ["pending", "draft", "warn", "ordered", "partial"] -%} + {%- elif v in ["pending", "cut", "draft", "warn", "ordered", "partial"] -%} {%- set variant_class = "status-warn" -%} {%- elif v in ["error", "aborted", "critical", "cancelled", "closed"] -%} {%- set variant_class = "status-error" -%} - {%- elif v in ["in_progress", "in-progress", "info"] -%} + {%- elif v in ["in_progress", "in-progress", "in work", "in_work", "info"] -%} {%- set variant_class = "status-info" -%} {%- elif v in ["inactive", "deprecated"] -%} {%- set variant_class = "status-inactive" -%} diff --git a/tests/unit/test_execution.py b/tests/unit/test_execution.py index 977ba0a..c819b52 100644 --- a/tests/unit/test_execution.py +++ b/tests/unit/test_execution.py @@ -141,10 +141,40 @@ def test_create_instance(client): assert data["procedure_id"] == proc_id assert data["version_id"] == version_id assert data["work_order_number"] == "WO-001" - assert data["status"] == "pending" + assert data["status"] == "cut" assert len(data["step_executions"]) == 3 +def test_batch_cut_creates_n_instances(client): + """quantity > 1 cuts a batch of identical work orders with distinct WO numbers.""" + proc_id, _ = _create_procedure_with_steps(client) + + response = client.post( + "/api/procedure-instances", + json={"procedure_id": proc_id, "quantity": 3}, + ) + assert response.status_code == 201 + assert response.json()["status"] == "cut" + + resp = client.get(f"/api/procedure-instances?procedure_id={proc_id}") + items = resp.json()["items"] + assert len(items) == 3 + assert len({i["work_order_number"] for i in items}) == 3 + assert all(i["status"] == "cut" for i in items) + assert all(len(i["step_executions"]) == 3 for i in items) + + +def test_batch_cut_rejects_explicit_work_order_number(client): + """An explicit WO number identifies a single cut; batches must generate.""" + proc_id, _ = _create_procedure_with_steps(client) + + response = client.post( + "/api/procedure-instances", + json={"procedure_id": proc_id, "quantity": 2, "work_order_number": "WO-X"}, + ) + assert response.status_code == 400 + + def test_list_instances(client): """Test listing instances.""" proc_id, _ = _create_procedure_with_steps(client) @@ -192,7 +222,7 @@ def test_start_step(client): # Instance should now be in_progress instance = client.get(f"/api/procedure-instances/{instance_id}").json() - assert instance["status"] == "in_progress" + assert instance["status"] == "in_work" def test_complete_step(client): @@ -394,11 +424,11 @@ def test_list_instances_filter_by_status(client): # Start the instance client.post(f"/api/procedure-instances/{instance_id}/steps/1/start") - resp = client.get("/api/procedure-instances?status=in_progress") + resp = client.get("/api/procedure-instances?status=in_work") assert resp.status_code == 200 data = resp.json() for item in data["items"]: - assert item["status"] == "in_progress" + assert item["status"] == "in_work" # ============ New tests — Kit & Consumption ============ diff --git a/tests/unit/test_mcp_execution.py b/tests/unit/test_mcp_execution.py index 3e2a6b8..26632aa 100644 --- a/tests/unit/test_mcp_execution.py +++ b/tests/unit/test_mcp_execution.py @@ -54,7 +54,7 @@ def test_get_execution_state_by_execution_id(client, db_session): data = _call(server._get_execution_state, db_session, {"execution_id": instance_id}) assert data["instance"]["id"] == instance_id assert data["instance"]["work_order"] == work_order - assert data["instance"]["status"] == "pending" + assert data["instance"]["status"] == "cut" assert data["instance"]["procedure_name"] == "Test Procedure" assert data["instance"]["progress"] == {"done": 0, "total": 3} assert [s["order"] for s in data["steps"]] == [1, 2, 3] @@ -142,13 +142,13 @@ def test_claim_step_happy(client, db_session, test_user): assert "claimed step" in data["message"] assert test_user.name in data["message"] assert data["status"] == "in_progress" - assert data["instance_started"] is True # first claim starts a pending instance + assert data["instance_started"] is True # first claim starts a cut instance state = _call(server._get_execution_state, db_session, {"execution_id": instance_id}) step1 = next(s for s in state["steps"] if s["order"] == 1) assert step1["status"] == "in_progress" assert step1["claim"]["user_id"] == test_user.id - assert state["instance"]["status"] == "in_progress" + assert state["instance"]["status"] == "in_work" def test_claim_step_conflict_second_user(client, db_session, test_user, admin_user): diff --git a/tests/unit/test_step_claims.py b/tests/unit/test_step_claims.py index 52e8346..89deae9 100644 --- a/tests/unit/test_step_claims.py +++ b/tests/unit/test_step_claims.py @@ -300,11 +300,11 @@ def test_complete_pending_unclaimed_step_still_works(client): # ============ 9. first claim flips instance ============ -def test_first_claim_flips_instance_to_in_progress(client, auth_headers): +def test_first_claim_flips_instance_to_in_work(client, auth_headers): instance_id = _create_instance(client) before = client.get(f"/api/procedure-instances/{instance_id}").json() - assert before["status"] == "pending" + assert before["status"] == "cut" client.post( f"/api/procedure-instances/{instance_id}/steps/1/start", @@ -312,10 +312,10 @@ def test_first_claim_flips_instance_to_in_progress(client, auth_headers): ) after = client.get(f"/api/procedure-instances/{instance_id}").json() - assert after["status"] == "in_progress" + assert after["status"] == "in_work" state = _state(client, instance_id) - assert state["instance"]["status"] == "in_progress" + assert state["instance"]["status"] == "in_work" # ============ 10. progress counts ============ From 056185513e10e7b82138745437b7a42b8e93114f Mon Sep 17 00:00:00 2001 From: CST-100 <42722690+CST-100@users.noreply.github.com> Date: Fri, 12 Jun 2026 11:30:48 -0700 Subject: [PATCH 09/79] Cache-bust static assets with mtime versions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stylesheet/script links carried no version, so browsers kept serving stale CSS/JS on normal reloads — shop-floor pages stay open for days, which made the lightbox fix invisible to an already-open browser. static_url() appends the file's mtime; URLs change exactly when files do. Co-Authored-By: Claude Fable 5 --- src/opal/web/routes.py | 18 ++++++++++++++++++ src/opal/web/templates/errors/404.html | 2 +- src/opal/web/templates/executions/detail.html | 4 ++-- src/opal/web/templates/layouts/base.html | 12 ++++++------ src/opal/web/templates/login.html | 4 ++-- src/opal/web/templates/login_set_password.html | 2 +- src/opal/web/templates/parts/label.html | 2 +- .../web/templates/requirements/detail.html | 2 +- src/opal/web/templates/requirements/tree.html | 2 +- src/opal/web/templates/risks/detail.html | 4 ++-- src/opal/web/templates/risks/list.html | 2 +- src/opal/web/templates/risks/new.html | 4 ++-- src/opal/web/templates/setup.html | 2 +- src/opal/web/templates/setup_profile.html | 2 +- src/opal/web/templates/users/detail.html | 2 +- 15 files changed, 41 insertions(+), 23 deletions(-) diff --git a/src/opal/web/routes.py b/src/opal/web/routes.py index 625cf1b..7028be4 100644 --- a/src/opal/web/routes.py +++ b/src/opal/web/routes.py @@ -52,6 +52,24 @@ TEMPLATES_DIR = Path(__file__).parent / "templates" templates = Jinja2Templates(directory=str(TEMPLATES_DIR)) +STATIC_DIR = Path(__file__).parent / "static" + + +def static_url(path: str) -> str: + """/static URL with an mtime cache-buster. + + Pages stay open for days on shop-floor tablets; without a version + in the URL a normal reload can keep serving stale CSS/JS forever. + """ + try: + version = int((STATIC_DIR / path).stat().st_mtime) + except OSError: + return f"/static/{path}" + return f"/static/{path}?v={version}" + + +templates.env.globals["static_url"] = static_url + def status_value(status) -> str: """Get string value from status (handles both enum and string).""" diff --git a/src/opal/web/templates/errors/404.html b/src/opal/web/templates/errors/404.html index d664fc5..fd26c8e 100644 --- a/src/opal/web/templates/errors/404.html +++ b/src/opal/web/templates/errors/404.html @@ -4,7 +4,7 @@ 404 - Not Found - OPAL - +
diff --git a/src/opal/web/templates/executions/detail.html b/src/opal/web/templates/executions/detail.html index 3ca372f..beaf292 100644 --- a/src/opal/web/templates/executions/detail.html +++ b/src/opal/web/templates/executions/detail.html @@ -4,7 +4,7 @@ {% block body_class %}exec-detail{% endblock %} {% block head_extra %} - + {% endblock %} {% block breadcrumbs %} @@ -268,5 +268,5 @@ status: {{ inst_status | tojson }}, }; - + {% endblock %} diff --git a/src/opal/web/templates/layouts/base.html b/src/opal/web/templates/layouts/base.html index 9cada09..0533a4c 100644 --- a/src/opal/web/templates/layouts/base.html +++ b/src/opal/web/templates/layouts/base.html @@ -11,12 +11,12 @@ if (theme === 'light') document.documentElement.setAttribute('data-theme', 'light'); })(); - - - - - - + + + + + + {% block head_extra %}{% endblock %} diff --git a/src/opal/web/templates/login.html b/src/opal/web/templates/login.html index 51f7ddb..7dbc5fd 100644 --- a/src/opal/web/templates/login.html +++ b/src/opal/web/templates/login.html @@ -10,7 +10,7 @@ if (theme === 'light') document.documentElement.setAttribute('data-theme', 'light'); })(); - + - + diff --git a/src/opal/web/templates/executions/tabs/document.html b/src/opal/web/templates/executions/tabs/document.html index 9748a6b..e4b8d58 100644 --- a/src/opal/web/templates/executions/tabs/document.html +++ b/src/opal/web/templates/executions/tabs/document.html @@ -1,9 +1,32 @@ -{# The document: one vertical scroll of OP cards in procedure order, plus the - rail. Single wrapper div (display:contents) — .exec-doc-layout becomes a - direct flex child of .exec-tab-content. #} +{# The document: a centered readable column of OP cards, with rails floating + in the side gutters (page-level scroll, no inner scrollbar). LEFT rail: + online roster + OP minimap. RIGHT rail: issues/holds, attachments, + reference docs. Rails hide below ~1280px — content is king; the drawer + keeps the right rail reachable and "N ONLINE" serves the roster. #}
+
{% for op_data in ops %} {% include 'executions/_op_card.html' %} diff --git a/src/opal/web/templates/index.html b/src/opal/web/templates/index.html index d430dcd..791545c 100644 --- a/src/opal/web/templates/index.html +++ b/src/opal/web/templates/index.html @@ -21,18 +21,18 @@
{% if stale_draft_parts %}
- ✗ stale draft parts (>30d, unreferenced): {{ stale_draft_parts | length }} + stale draft parts (>30d, unreferenced): {{ stale_draft_parts | length }}
{% endif %} {% for item in overdue_tbrs %}
- ✗ {{ item.req.req_number }} + {{ item.req.req_number }} TBR overdue {{ item.days_over }}d — {{ item.owner_name }}
{% endfor %} {% for req in stuck_drafts %}
- ✗ {{ req.req_number }} + {{ req.req_number }} draft >7d with block-severity lint
{% endfor %} @@ -44,7 +44,7 @@ {% endfor %} {% if ready_to_baseline %}
- ✓ ready to baseline: {{ ready_to_baseline }} → + ready to baseline: {{ ready_to_baseline }} →
{% endif %}
diff --git a/src/opal/web/templates/layouts/base.html b/src/opal/web/templates/layouts/base.html index 0533a4c..80a076d 100644 --- a/src/opal/web/templates/layouts/base.html +++ b/src/opal/web/templates/layouts/base.html @@ -494,7 +494,7 @@ panel.className = 'draft-blocker mono'; const title = document.createElement('div'); title.className = 'draft-blocker-title'; - title.textContent = '✗ draft parts block: ' + detail.action; + title.textContent = 'draft parts block: ' + detail.action; panel.appendChild(title); detail.draft_parts.forEach(p => { const row = document.createElement('div'); diff --git a/src/opal/web/templates/parts/_ledger_is.html b/src/opal/web/templates/parts/_ledger_is.html index d361b84..1b2ff7d 100644 --- a/src/opal/web/templates/parts/_ledger_is.html +++ b/src/opal/web/templates/parts/_ledger_is.html @@ -17,7 +17,7 @@ {{ line.component.name }} ×{{ line.quantity }}{% if line.reference_designator %} [{{ line.reference_designator }}]{% endif %} - ✎ + EDIT ×
diff --git a/src/opal/web/templates/requirements/queue.html b/src/opal/web/templates/requirements/queue.html index 738a898..f58fb1d 100644 --- a/src/opal/web/templates/requirements/queue.html +++ b/src/opal/web/templates/requirements/queue.html @@ -124,7 +124,7 @@ document.getElementById('queue-item-screen').style.display = 'none'; document.getElementById('queue-commit-screen').style.display = 'block'; document.getElementById('queue-staged-list').innerHTML = list.map(it => - '
✓ ' + escapeHtml(it.req_number) + ' rev ' + it.revision + + '
' + escapeHtml(it.req_number) + ' rev ' + it.revision + ' — ' + escapeHtml(it.title) + '
').join(''); document.getElementById('queue-sign-time').textContent = new Date().toISOString().substring(0, 19) + 'Z'; diff --git a/src/opal/web/templates/risks/_disposition_panel.html b/src/opal/web/templates/risks/_disposition_panel.html index 0086968..2f3fd5c 100644 --- a/src/opal/web/templates/risks/_disposition_panel.html +++ b/src/opal/web/templates/risks/_disposition_panel.html @@ -50,7 +50,7 @@ {% if blockers %}
{% for blocker in blockers %} -
✗ {{ blocker }}
+
{{ blocker }}
{% endfor %}
{% endif %} diff --git a/src/opal/web/templates/risks/list.html b/src/opal/web/templates/risks/list.html index e1b72f3..9588446 100644 --- a/src/opal/web/templates/risks/list.html +++ b/src/opal/web/templates/risks/list.html @@ -7,7 +7,7 @@ {% endblock %} {% block content %} -{% set actions %}{{ ok.btn("REVIEWED ✓", attrs='onclick="stampReviewed()" title="stamp last reviewed on all listed risks"') }} {{ ok.btn("MATRIX VIEW", href="/risks/matrix") }} {{ ok.btn("+ NEW RISK", href="/risks/new", variant="primary") }}{% endset %} +{% set actions %}{{ ok.btn("REVIEWED", attrs='onclick="stampReviewed()" title="stamp last reviewed on all listed risks"') }} {{ ok.btn("MATRIX VIEW", href="/risks/matrix") }} {{ ok.btn("+ NEW RISK", href="/risks/new", variant="primary") }}{% endset %} {% call ok.panel("RISK REGISTER", actions=actions) %}
{{ ok.toolbar_search(name="search", id="search", placeholder="Search risks...", hx_get="/risks/table", hx_target="#risks-table", hx_include="[name='disposition'], [name='severity']") }} diff --git a/tests/unit/test_risks_web.py b/tests/unit/test_risks_web.py index 2db52dd..9b67896 100644 --- a/tests/unit/test_risks_web.py +++ b/tests/unit/test_risks_web.py @@ -41,7 +41,7 @@ def test_register_columns_no_id_column(web_client): assert column in page.text # No ID column; the bulk review stamp is in the header assert ">ID<" not in page.text - assert "REVIEWED ✓" in page.text + assert "REVIEWED" in page.text assert "stampReviewed()" in page.text rows = web_client.get("/risks/table") From 698bd60f311276e45fb61cedf8d536bb57beeea4 Mon Sep 17 00:00:00 2001 From: CST-100 <42722690+CST-100@users.noreply.github.com> Date: Fri, 12 Jun 2026 12:38:06 -0700 Subject: [PATCH 15/79] Rewrite execution tests for the focus model MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claim-era tests map to their focus equivalents: focus places/moves a cursor without touching step status, two users share a step without conflict, /leave clears the cursor, COMPLETE acts directly on pending steps by any user, bound holds and strict_sequence and OP dependencies gate COMPLETE (not start), NC-resume pops steps to PENDING, and the first COMPLETE or SKIP flips the WO out of CUT. Document-web tests now carry the Amendment 1 exit criteria: zero buttons in the exec-doc column, dockbar serves ?step=N and follows the cursor, indicators and holds render as words. Also fixes a real bug the new dockbar tests caught: the partial routes (/dockbar, /step-row, /rail) rendered with inst_status undefined — every dockbar refetch dropped the ISSUE/COMPLETE/SKIP controls. Co-Authored-By: Claude Fable 5 --- src/opal/web/routes.py | 3 + tests/unit/test_execution.py | 112 +++---- tests/unit/test_execution_document_web.py | 86 +++--- tests/unit/test_execution_holds.py | 104 ++++--- tests/unit/test_mcp_execution.py | 147 ++++----- tests/unit/test_procedures.py | 18 +- tests/unit/test_step_claims.py | 349 ---------------------- tests/unit/test_step_focus.py | 324 ++++++++++++++++++++ 8 files changed, 559 insertions(+), 584 deletions(-) delete mode 100644 tests/unit/test_step_claims.py create mode 100644 tests/unit/test_step_focus.py diff --git a/src/opal/web/routes.py b/src/opal/web/routes.py index 4d3e53f..b99cee5 100644 --- a/src/opal/web/routes.py +++ b/src/opal/web/routes.py @@ -2272,6 +2272,9 @@ def _se_status(se): # Can finalize: instance completed + has WIP productions inst_status = instance.status.value if hasattr(instance.status, "value") else instance.status + # Partials (_dockbar, _op_card, _rail) read inst_status from context — + # only detail.html re-derives it with {% set %}. + context["inst_status"] = inst_status has_wip = any( (p.status.value if hasattr(p.status, "value") else p.status) == "wip" for p in productions ) diff --git a/tests/unit/test_execution.py b/tests/unit/test_execution.py index c819b52..e51d411 100644 --- a/tests/unit/test_execution.py +++ b/tests/unit/test_execution.py @@ -204,8 +204,8 @@ def test_get_instance(client): assert response.json()["id"] == instance_id -def test_start_step(client): - """Test starting a step.""" +def test_complete_from_pending_sets_started_at_and_flips_instance(client): + """Completing a pending step records started_at and moves the WO to in_work.""" proc_id, _ = _create_procedure_with_steps(client) instance_response = client.post( @@ -214,13 +214,12 @@ def test_start_step(client): ) instance_id = instance_response.json()["id"] - # Start step 1 - response = client.post(f"/api/procedure-instances/{instance_id}/steps/1/start") + # Complete step 1 directly — there is no separate start moment. + response = client.post(f"/api/procedure-instances/{instance_id}/steps/1/complete", json={}) assert response.status_code == 200 - assert response.json()["status"] == "in_progress" + assert response.json()["status"] == "completed" assert response.json()["started_at"] is not None - # Instance should now be in_progress instance = client.get(f"/api/procedure-instances/{instance_id}").json() assert instance["status"] == "in_work" @@ -235,8 +234,6 @@ def test_complete_step(client): ) instance_id = instance_response.json()["id"] - # Start and complete step 1 - client.post(f"/api/procedure-instances/{instance_id}/steps/1/start") response = client.post( f"/api/procedure-instances/{instance_id}/steps/1/complete", json={"data_captured": {"notes": "Done"}}, @@ -258,7 +255,6 @@ def test_complete_all_steps_completes_instance(client): # Complete all steps for step in [1, 2, 3]: - client.post(f"/api/procedure-instances/{instance_id}/steps/{step}/start") client.post(f"/api/procedure-instances/{instance_id}/steps/{step}/complete", json={}) # Instance should be completed @@ -277,8 +273,6 @@ def test_log_non_conformance(client): ) instance_id = instance_response.json()["id"] - # Start step and log NC - client.post(f"/api/procedure-instances/{instance_id}/steps/1/start") response = client.post( f"/api/procedure-instances/{instance_id}/steps/1/nc", json={ @@ -306,8 +300,8 @@ def test_abort_instance(client): ) instance_id = instance_response.json()["id"] - # Start instance - client.post(f"/api/procedure-instances/{instance_id}/steps/1/start") + # Put the instance in work (first complete flips cut -> in_work). + client.post(f"/api/procedure-instances/{instance_id}/steps/1/complete", json={}) # Abort response = client.patch( @@ -319,8 +313,8 @@ def test_abort_instance(client): assert response.json()["completed_at"] is not None -def test_cannot_start_completed_step(client): - """Test cannot start a step that's already completed.""" +def test_cannot_complete_completed_step(client): + """Test cannot complete a step that's already completed.""" proc_id, _ = _create_procedure_with_steps(client) instance_response = client.post( @@ -329,12 +323,10 @@ def test_cannot_start_completed_step(client): ) instance_id = instance_response.json()["id"] - # Start and complete step 1 - client.post(f"/api/procedure-instances/{instance_id}/steps/1/start") client.post(f"/api/procedure-instances/{instance_id}/steps/1/complete", json={}) - # Try to start again - response = client.post(f"/api/procedure-instances/{instance_id}/steps/1/start") + # Try to complete again + response = client.post(f"/api/procedure-instances/{instance_id}/steps/1/complete", json={}) assert response.status_code == 400 @@ -345,7 +337,6 @@ def test_update_step_notes(client): """Test updating notes on a step.""" instance_id = _create_instance(client) - client.post(f"/api/procedure-instances/{instance_id}/steps/1/start") resp = client.patch( f"/api/procedure-instances/{instance_id}/steps/1/notes", json={"notes": "Check torque value"}, @@ -373,7 +364,6 @@ def test_skip_step_completes_instance(client): # Complete steps 1 and 2, skip step 3 for step in [1, 2]: - client.post(f"/api/procedure-instances/{instance_id}/steps/{step}/start") client.post(f"/api/procedure-instances/{instance_id}/steps/{step}/complete", json={}) client.post( @@ -421,12 +411,14 @@ def test_list_instances_filter_by_status(client): """Test filtering instances by status.""" instance_id = _create_instance(client) - # Start the instance - client.post(f"/api/procedure-instances/{instance_id}/steps/1/start") + # First complete flips the instance to in_work (one of three steps — + # completing all of them would flip it to completed). + client.post(f"/api/procedure-instances/{instance_id}/steps/1/complete", json={}) resp = client.get("/api/procedure-instances?status=in_work") assert resp.status_code == 200 data = resp.json() + assert instance_id in {item["id"] for item in data["items"]} for item in data["items"]: assert item["status"] == "in_work" @@ -470,9 +462,6 @@ def test_consume_step_parts(client, auth_headers): """Test consuming parts at a specific step.""" instance_id, _, inv_record_id = _create_procedure_with_kit(client, auth_headers) - # Start step 1 - client.post(f"/api/procedure-instances/{instance_id}/steps/1/start") - resp = client.post( f"/api/procedure-instances/{instance_id}/steps/1/consume", json={ @@ -507,8 +496,7 @@ def test_get_step_consumptions(client, auth_headers): """Test getting consumptions for a specific step.""" instance_id, _, inv_record_id = _create_procedure_with_kit(client, auth_headers) - # Start step 1 and consume at that step - client.post(f"/api/procedure-instances/{instance_id}/steps/1/start") + # Consume at step 1 — consumption does not require any step status. client.post( f"/api/procedure-instances/{instance_id}/steps/1/consume", json={"items": [{"inventory_record_id": inv_record_id, "quantity": 1}]}, @@ -612,9 +600,8 @@ def test_leave_execution(client, auth_headers): # ============ NC step-hold tests ============ -def _start_step_and_log_nc(client, instance_id, step_number=1, title="NC A"): - """Start a step and log an NC against it. Returns (issue_id, step_status_after).""" - client.post(f"/api/procedure-instances/{instance_id}/steps/{step_number}/start") +def _log_nc(client, instance_id, step_number=1, title="NC A"): + """Log an NC against a step. Returns (issue_id, step_status_after).""" resp = client.post( f"/api/procedure-instances/{instance_id}/steps/{step_number}/nc", json={"title": title, "description": "x", "priority": "medium"}, @@ -629,16 +616,16 @@ def _start_step_and_log_nc(client, instance_id, step_number=1, title="NC A"): def test_log_nc_puts_step_on_hold(client): - """Logging an NC against an in-progress step transitions it to on_hold.""" + """Logging an NC against a step transitions it to on_hold.""" instance_id = _create_instance(client) - _, step_status = _start_step_and_log_nc(client, instance_id) + _, step_status = _log_nc(client, instance_id) assert step_status == "on_hold" def test_approving_nc_disposition_resumes_step(client): - """Approving the sole NC's disposition pops the step back to in_progress.""" + """Approving the sole NC's disposition pops the step back to pending.""" instance_id = _create_instance(client) - issue_id, _ = _start_step_and_log_nc(client, instance_id) + issue_id, _ = _log_nc(client, instance_id) resp = client.patch( f"/api/issues/{issue_id}", @@ -648,14 +635,14 @@ def test_approving_nc_disposition_resumes_step(client): inst = client.get(f"/api/procedure-instances/{instance_id}").json() step_status = next(s["status"] for s in inst["step_executions"] if s["step_number"] == 1) - assert step_status == "in_progress" + assert step_status == "pending" def test_two_open_ncs_keep_step_on_hold_until_all_resolved(client): """With two open NCs on a step, the step stays held until both reach a terminal disposition state.""" instance_id = _create_instance(client) - issue_a, _ = _start_step_and_log_nc(client, instance_id, title="NC A") + issue_a, _ = _log_nc(client, instance_id, title="NC A") # Log a second NC on the same step via the API (UI hides the button while held). resp_b = client.post( @@ -673,21 +660,19 @@ def test_two_open_ncs_keep_step_on_hold_until_all_resolved(client): inst = client.get(f"/api/procedure-instances/{instance_id}").json() assert next(s["status"] for s in inst["step_executions"] if s["step_number"] == 1) == "on_hold" - # Approve B — step now resumes. + # Approve B — step now resumes to pending. client.patch( f"/api/issues/{issue_b}", json={"status": "disposition_approved", "disposition_type": "use_as_is"}, ) inst = client.get(f"/api/procedure-instances/{instance_id}").json() - assert ( - next(s["status"] for s in inst["step_executions"] if s["step_number"] == 1) == "in_progress" - ) + assert next(s["status"] for s in inst["step_executions"] if s["step_number"] == 1) == "pending" def test_cannot_skip_on_hold_step(client): """The /skip endpoint refuses a step that is on hold for an open NC.""" instance_id = _create_instance(client) - _start_step_and_log_nc(client, instance_id) + _log_nc(client, instance_id) resp = client.post( f"/api/procedure-instances/{instance_id}/steps/1/skip", @@ -703,7 +688,7 @@ def test_cannot_skip_on_hold_step(client): def _create_redline_setup(client): """Create an instance, log an NC on step 1, return (instance_id, issue_id, host_step_exec_id).""" instance_id = _create_instance(client) - issue_id, _ = _start_step_and_log_nc(client, instance_id) + issue_id, _ = _log_nc(client, instance_id) inst = client.get(f"/api/procedure-instances/{instance_id}").json() host_se_id = next(s["id"] for s in inst["step_executions"] if s["step_number"] == 1) return instance_id, issue_id, host_se_id @@ -775,17 +760,14 @@ def test_redline_completion_releases_held_step(client): assert next(s["status"] for s in inst["step_executions"] if s["step_number"] == 1) == "on_hold" # Run the redline sub-step to completion. - client.post(f"/api/procedure-instances/{instance_id}/steps/{sub_step_number}/start") client.post( f"/api/procedure-instances/{instance_id}/steps/{sub_step_number}/complete", json={}, ) - # Host step should now have auto-resumed. + # Host step should now have auto-resumed to pending. inst = client.get(f"/api/procedure-instances/{instance_id}").json() - assert ( - next(s["status"] for s in inst["step_executions"] if s["step_number"] == 1) == "in_progress" - ) + assert next(s["status"] for s in inst["step_executions"] if s["step_number"] == 1) == "pending" def test_redline_orphan_when_nc_soft_deleted(client): @@ -819,8 +801,9 @@ def test_redline_orphan_when_nc_soft_deleted(client): assert len(list_resp.json()) == 1 -def test_redline_delete_unstarted(client): - """An unstarted redline can be deleted; once any sub-step is started, deletion is rejected.""" +def test_redline_delete_rejected_once_worked(client): + """A pending redline can be deleted; once any sub-step has been worked + (no longer pending), deletion is rejected.""" instance_id, issue_id, _ = _create_redline_setup(client) op_resp = client.post( f"/api/procedure-instances/{instance_id}/ad-hoc-ops", @@ -833,8 +816,8 @@ def test_redline_delete_unstarted(client): op_id = op_resp["id"] sub_step_number = op_resp["sub_steps"][0]["step_number"] - # Start the sub-step → delete should now fail. - client.post(f"/api/procedure-instances/{instance_id}/steps/{sub_step_number}/start") + # Complete the sub-step → delete should now fail. + client.post(f"/api/procedure-instances/{instance_id}/steps/{sub_step_number}/complete", json={}) fail = client.delete(f"/api/procedure-instances/{instance_id}/ad-hoc-ops/{op_id}") assert fail.status_code == 400 @@ -905,7 +888,6 @@ def _create_instance_with_sub_steps(client): def test_nc_on_sub_step_puts_parent_op_on_hold(client): """Logging an NC on a sub-step also marks the parent op ON_HOLD.""" instance_id, op_a, a1, _a2, _op_b = _create_instance_with_sub_steps(client) - client.post(f"/api/procedure-instances/{instance_id}/steps/{a1}/start") client.post( f"/api/procedure-instances/{instance_id}/steps/{a1}/nc", json={"title": "NC on A.1", "description": "x", "priority": "medium"}, @@ -916,23 +898,21 @@ def test_nc_on_sub_step_puts_parent_op_on_hold(client): assert by_num[op_a] == "on_hold" -def test_sibling_sub_step_cannot_start_while_op_on_hold(client): - """A sibling sub-step of an NC'd step cannot be started — the whole op is held.""" +def test_sibling_sub_step_cannot_complete_while_op_on_hold(client): + """A sibling sub-step of an NC'd step cannot be completed — the whole op is held.""" instance_id, _op_a, a1, a2, _op_b = _create_instance_with_sub_steps(client) - client.post(f"/api/procedure-instances/{instance_id}/steps/{a1}/start") client.post( f"/api/procedure-instances/{instance_id}/steps/{a1}/nc", json={"title": "NC", "description": "x", "priority": "medium"}, ) - resp = client.post(f"/api/procedure-instances/{instance_id}/steps/{a2}/start") + resp = client.post(f"/api/procedure-instances/{instance_id}/steps/{a2}/complete", json={}) assert resp.status_code == 400 assert "on hold" in resp.json()["detail"].lower() def test_nc_resolution_resumes_both_sub_step_and_parent_op(client): - """Disposing the NC pops both the sub-step and the parent op back to in_progress.""" + """Disposing the NC pops both the sub-step and the parent op back to pending.""" instance_id, op_a, a1, _a2, _op_b = _create_instance_with_sub_steps(client) - client.post(f"/api/procedure-instances/{instance_id}/steps/{a1}/start") nc_resp = client.post( f"/api/procedure-instances/{instance_id}/steps/{a1}/nc", json={"title": "NC", "description": "x", "priority": "medium"}, @@ -944,15 +924,15 @@ def test_nc_resolution_resumes_both_sub_step_and_parent_op(client): ) inst = client.get(f"/api/procedure-instances/{instance_id}").json() by_num = {s["step_number"]: s["status"] for s in inst["step_executions"]} - assert by_num[a1] == "in_progress" - assert by_num[op_a] == "in_progress" + assert by_num[a1] == "pending" + assert by_num[op_a] == "pending" -# ============ Sub-steps on pending (gated) ops are not startable (issue #2) ============ +# ============ Sub-steps on gated ops are not completable (issue #2) ============ -def test_sub_step_cannot_start_when_parent_op_has_unmet_prereqs(client): - """OP B has OP A as a dependency. A sub-step of OP B must not be startable +def test_sub_step_cannot_complete_when_parent_op_has_unmet_prereqs(client): + """OP B has OP A as a dependency. A sub-step of OP B must not be completable until OP A is terminal.""" # Build a procedure: OP A, OP B (depends on A) with one sub-step. proc_resp = client.post("/api/procedures", json={"name": "Gated Sub Procedure"}) @@ -975,8 +955,8 @@ def test_sub_step_cannot_start_when_parent_op_has_unmet_prereqs(client): by_label = {s["step_number_str"]: s["step_number"] for s in inst["step_executions"]} b1_order = by_label["2.1"] - # OP A not started yet; B.1 must refuse. - resp = client.post(f"/api/procedure-instances/{instance_id}/steps/{b1_order}/start") + # OP A not complete yet; B.1's COMPLETE must refuse. + resp = client.post(f"/api/procedure-instances/{instance_id}/steps/{b1_order}/complete", json={}) assert resp.status_code == 400 assert "waiting on" in resp.json()["detail"].lower() # B.1 unused but kept as a reference to confirm setup correctness. diff --git a/tests/unit/test_execution_document_web.py b/tests/unit/test_execution_document_web.py index ac16c9e..9655f84 100644 --- a/tests/unit/test_execution_document_web.py +++ b/tests/unit/test_execution_document_web.py @@ -3,8 +3,8 @@ Identity note: API auth resolves the session cookie BEFORE the bearer header (see opal.api.deps.get_current_user), so API mutations issued through ``web_client`` are attributed to ``test_user`` — the same identity the web -page renders as ``current_user``. The mystep test verifies this explicitly -via /state. +page renders as ``current_user``. The dockbar cursor test verifies this +explicitly via /state. """ from datetime import UTC, datetime @@ -100,6 +100,11 @@ def _op_section_body(html: str, order: int) -> str: return html.split(f'id="op-{order}"')[1].split("")[0] +def _doc_column(html: str) -> str: + """Markup between id="exec-doc" and the right rail's
+
+ {# Declares which sections expect content (make: BOM; + buy: suppliers/POs); untouched it follows the + external-ref heuristic, the server's one home #} + + +
+
+
-
-
+
+
+{% else %} +{{ ok.empty_line("REQUIREMENTS", onclick="showAssignRequirement()") }} +{% endif %}
@@ -66,6 +66,7 @@
{# --- Test templates --- #} +{% if test_templates %}
TESTS @@ -75,7 +76,6 @@
- {% if test_templates %} {{ ok.th("NAME") }} @@ -84,7 +84,6 @@ {{ ok.th("", width="40px") }} - {% endif %} {% for tt in test_templates %} @@ -93,9 +92,10 @@ - {% else %} - {% endfor %}
{% if tt.test_type == 'numeric' %}{{ '%g' % tt.min_value if tt.min_value is not none else '' }}–{{ '%g' % tt.max_value if tt.max_value is not none else '' }}{{ ' ' ~ tt.unit if tt.unit else '' }}{% else %}—{% endif %} ×
—
+{% else %} +{{ ok.empty_line("TESTS", onclick="showAddTest()") }} +{% endif %} diff --git a/src/opal/web/templates/parts/_section_stock.html b/src/opal/web/templates/parts/_section_stock.html index d4e0113..ea39e88 100644 --- a/src/opal/web/templates/parts/_section_stock.html +++ b/src/opal/web/templates/parts/_section_stock.html @@ -1,10 +1,15 @@ {% import 'opalkit/_macros.html' as ok %} {# STOCK & USE — composition, stock, purchasing, suppliers, usage, and the - consumption log (last: it is the one unbounded section). Sections always - render their structure: panel header (label · count · add) + headed - table; an empty body is one muted dash row, never a box or narration. #} + consumption log (last: it is the one unbounded section). Relevance is + declared by procurement: BOM expects content on make|both, SUPPLIERS and + PO LINES on buy|both; the rest always. An irrelevant empty section is + absent; DATA WINS — any section with rows renders regardless. An empty + relevant section is one line. #} +{% set bom_relevant = part.procurement in ('make', 'both') %} +{% set buy_relevant = part.procurement in ('buy', 'both') %} {# --- BOM --- #} +{% if bom_lines %}
BOM @@ -14,7 +19,6 @@
- {% if bom_lines %} {{ ok.th("PN", width="130px") }} @@ -25,7 +29,6 @@ {{ ok.th("", width="64px") }} - {% endif %} {% for line in bom_lines %} @@ -39,34 +42,34 @@ × - {% else %} - {% endfor %}
—
- {% if bom_lines %} {# Tree renders only when it carries depth the direct lines don't #} - {% endif %}
+{% elif bom_relevant %} +{{ ok.empty_line("BOM", onclick="showAddComponent()") }} +{% endif %} -{# --- Stock — the on-hand figure is the section's value; LOW derives from - the reorder point the same way the parts list derives it #} +{# --- Stock — always relevant; the on-hand figure is the section's value; + LOW derives from the reorder point as on the parts list #} {% set on_hand = part.total_quantity or 0 %} +{% set stock_low = part.reorder_point is not none and on_hand < part.reorder_point %} +{% if inventory_records %}
STOCK {{ '%g' % on_hand }} {{ (part.unit_of_measure or 'EA') | upper }} · {{ inventory_records | length }} REC - {%- if part.reorder_point is not none and on_hand < part.reorder_point %} · LOW{% endif %} + {%- if stock_low %} · LOW{% endif %}
+ ADD
- {% if inventory_records %} {{ ok.th("OPAL #", width="120px") }} @@ -75,7 +78,6 @@ {{ ok.th("LOT", width="140px") }} - {% endif %} {% for inv in inventory_records %} @@ -84,12 +86,13 @@ - {% else %} - {% endfor %}
{{ '%g' % inv.quantity }} {{ inv.lot_number or '—' }}
—
+{% else %} +{{ ok.empty_line("STOCK", onclick="toggleReceiveStock()", flag="LOW" if stock_low else none) }} +{% endif %}
@@ -100,13 +103,13 @@
{# --- Open PO lines --- #} +{% if open_po_lines %}
PO LINES {{ open_po_lines | length }}
- {% if open_po_lines %} {{ ok.th("PO", width="120px") }} @@ -114,7 +117,6 @@ {{ ok.th("RECEIVED", width="100px", align="right") }} - {% endif %} {% for line in open_po_lines %} @@ -122,14 +124,16 @@ - {% else %} - {% endfor %}
{{ (line.purchase.status.value if line.purchase.status.value is defined else line.purchase.status) | upper }} {{ '%g' % line.qty_received }}/{{ '%g' % line.qty_ordered }}
—
+{% elif buy_relevant %} +{{ ok.empty_line("PO LINES") }} +{% endif %} {# --- Suppliers --- #} +{% if supplier_entries %}
SUPPLIERS @@ -139,7 +143,6 @@
- {% if supplier_entries %} {{ ok.th("SUPPLIER") }} @@ -148,7 +151,6 @@ {{ ok.th("", width="40px") }} - {% endif %} {% for entry in supplier_entries %} @@ -157,21 +159,22 @@ - {% else %} - {% endfor %}
{{ '✓' if entry.is_preferred else '—' }} ×
—
+{% elif buy_relevant %} +{{ ok.empty_line("SUPPLIERS", onclick="showAddSupplier()") }} +{% endif %} {# --- Where used --- #} +{% if where_used %}
WHERE USED {{ where_used | length }}
- {% if where_used %} {{ ok.th("PN", width="130px") }} @@ -180,7 +183,6 @@ {{ ok.th("REFDES", width="90px") }} - {% endif %} {% for line in where_used %} @@ -189,21 +191,22 @@ - {% else %} - {% endfor %}
{{ line.quantity }} {{ line.reference_designator or '—' }}
—
+{% else %} +{{ ok.empty_line("WHERE USED") }} +{% endif %} {# --- Procedure usage (kits reference the design, not the stock) --- #} +{% if kit_usages or step_kit_usages %}
PROCEDURE USE {{ (kit_usages | length) + (step_kit_usages | length) }}
- {% if kit_usages or step_kit_usages %} {{ ok.th("PROCEDURE") }} @@ -213,7 +216,6 @@ {{ ok.th("STATUS", width="110px") }} - {% endif %} {% for kit in kit_usages %} @@ -233,21 +235,21 @@ {% endfor %} - {% if not (kit_usages or step_kit_usages) %} - - {% endif %}
{{ (sk.step.procedure.status.value if sk.step.procedure.status.value is defined else sk.step.procedure.status) | upper }}
—
+{% else %} +{{ ok.empty_line("PROCEDURE USE") }} +{% endif %} {# --- Consumption history --- #} +{% if consumption_history %}
CONSUMED {{ consumption_history | length }}{{ '+' if consumption_history | length == 50 }}
- {% if consumption_history %} {{ ok.th("DATE", width="110px") }} @@ -256,7 +258,6 @@ {{ ok.th("NOTES") }} - {% endif %} {% for c in consumption_history %} @@ -271,9 +272,10 @@ - {% else %} - {% endfor %}
{{ c.notes or '—' }}
—
+{% else %} +{{ ok.empty_line("CONSUMED") }} +{% endif %} diff --git a/src/opal/web/templates/parts/detail.html b/src/opal/web/templates/parts/detail.html index ba9beeb..b821683 100644 --- a/src/opal/web/templates/parts/detail.html +++ b/src/opal/web/templates/parts/detail.html @@ -75,6 +75,7 @@ CATEGORY{% if part.category %}{{ part.category | upper }}{% else %}—{% endif %} UOM{{ (part.unit_of_measure or 'EA') | upper }} TRACKING{{ 'SERIALIZED' if part.tracking_type and part.tracking_type.value == 'serialized' else 'BULK' }} + PROCUREMENT{{ (part.procurement.value if part.procurement.value is defined else part.procurement) | upper }} REORDER{% if part.reorder_point is not none %}{{ '%g' % part.reorder_point }}{% else %}—{% endif %} TOOLING{% if part.is_tooling %}{{ 'CAL ' ~ part.calibration_interval_days ~ 'D' if part.calibration_interval_days else 'YES' }}{% else %}—{% endif %} PARENT{% if part.parent_id %}{{ part.parent.internal_pn }}{% else %}—{% endif %} diff --git a/tests/unit/test_part_lifecycle.py b/tests/unit/test_part_lifecycle.py index 73778d5..6f3ac3c 100644 --- a/tests/unit/test_part_lifecycle.py +++ b/tests/unit/test_part_lifecycle.py @@ -348,8 +348,12 @@ def test_draft_part_detail_page_renders_activate_confirm(web_client): # (activate confirm, delete confirm, tier-change confirm in the form JS) assert "Locks PN and tier permanently." in page.text assert page.text.count("permanently") <= 3 - # Empty sections keep their table structure — a dash row, no narration - assert page.text.count('class="panel panel-ledger"') == 9 + # Empty relevant sections are one line each, no panels, no narration; + # a make part (no external ref) has no SUPPLIERS or PO LINES sections + assert page.text.count('class="empty-line"') == 7 + assert page.text.count('class="panel panel-ledger"') == 0 + assert ">SUPPLIERS<" not in page.text + assert ">PO LINES<" not in page.text assert "No BOM components" not in page.text assert "nothing physical yet" not in page.text # Nothing hides behind disclosure: the identity table is always visible @@ -403,13 +407,14 @@ def test_part_page_two_column_layout(web_client): assert page.text.count('class="part-col"') == 2 # Order is fixed and global: DESIGN (REQUIREMENTS, TESTS) in the left # column, then STOCK & USE (BOM first, CONSUMED last) in the right — - # in both lifecycle states + # in both lifecycle states. SUPPLIERS/PO LINES are absent on this make + # part (section relevance), so the order check covers what renders. assert page.text.index(">DESIGN<") < page.text.index(">STOCK & USE<") second_col = page.text[page.text.index(">STOCK & USE<") :] assert page.text.index(">REQUIREMENTS<") < page.text.index(">TESTS<") < page.text.index( ">STOCK & USE<" ) - order = [">BOM<", ">STOCK<", ">PO LINES<", ">SUPPLIERS<", ">WHERE USED<", ">PROCEDURE USE<", ">CONSUMED<"] + order = [">BOM<", ">STOCK<", ">WHERE USED<", ">PROCEDURE USE<", ">CONSUMED<"] positions = [second_col.index(label) for label in order] assert positions == sorted(positions) web_client.post(f"/api/parts/{part['id']}/activate", json={}) diff --git a/tests/unit/test_part_procurement.py b/tests/unit/test_part_procurement.py new file mode 100644 index 0000000..2643ae0 --- /dev/null +++ b/tests/unit/test_part_procurement.py @@ -0,0 +1,122 @@ +"""Amendment 6: procurement declaration and section relevance. + +The declaration (make | buy | both) governs which part-page sections +expect content. Irrelevant empty sections are absent; data always +renders; an empty relevant section is exactly one line. +""" + + +def _create(client, **kwargs): + payload = {"name": "Procurement Part", "tier": 1, **kwargs} + resp = client.post("/api/parts", json=payload) + assert resp.status_code == 201, resp.text + return resp.json() + + +def test_create_defaults_buy_when_external_pn_given(web_client): + part = _create(web_client, name="Lock Washer", external_pn="91255A101") + assert part["procurement"] == "buy" + + +def test_create_defaults_make_without_external_pn(web_client): + part = _create(web_client, name="Engine Assembly") + assert part["procurement"] == "make" + + +def test_explicit_procurement_overrides_heuristic(web_client): + part = _create(web_client, name="Machined Casting", external_pn="CAST-44", procurement="both") + assert part["procurement"] == "both" + + +def test_patch_procurement(web_client): + part = _create(web_client, name="Patched Part") + resp = web_client.patch(f"/api/parts/{part['id']}", json={"procurement": "buy"}) + assert resp.status_code == 200 + assert resp.json()["procurement"] == "buy" + + bad = web_client.patch(f"/api/parts/{part['id']}", json={"procurement": "steal"}) + assert bad.status_code == 422 + + +def test_buy_part_page_has_no_bom_but_one_line_suppliers(web_client): + """Exit criterion 6: the lock washer.""" + part = _create(web_client, name="Lock Washer", external_pn="91255A101") + page = web_client.get(f"/parts/{part['id']}") + assert page.status_code == 200 + assert ">BOM<" not in page.text + assert ">SUPPLIERS<" in page.text + assert ">PO LINES<" in page.text + # One line with the add affordance, never a header bar over a box + suppliers_line = page.text[page.text.index(">SUPPLIERS<") - 200 : page.text.index(">SUPPLIERS<") + 300] + assert "empty-line" in suppliers_line + assert "+ ADD" in suppliers_line + assert page.text.count('class="panel panel-ledger"') == 0 + + +def test_make_part_page_has_no_supplier_sections(web_client): + """Exit criterion 7: the engine assembly.""" + part = _create(web_client, name="Engine Assembly") + page = web_client.get(f"/parts/{part['id']}") + assert page.status_code == 200 + assert ">BOM<" in page.text + assert ">SUPPLIERS<" not in page.text + assert ">PO LINES<" not in page.text + + +def test_data_wins_bom_on_buy_part(web_client): + """Exit criterion 8: a BOM edge forced onto a buy part renders.""" + washer = _create(web_client, name="Washer Kit", external_pn="91255A102") + component = _create(web_client, name="Split Washer", external_pn="91255A103") + resp = web_client.post( + f"/api/bom/assemblies/{washer['id']}", + json={"component_id": component["id"], "quantity": 4}, + ) + assert resp.status_code in (200, 201), resp.text + + page = web_client.get(f"/parts/{washer['id']}") + assert ">BOM<" in page.text + # Populated: the section is a panel with a headed table, not a line + bom_region = page.text[page.text.index(">BOM<") - 300 : page.text.index(">BOM<") + 200] + assert "panel-ledger" in bom_region + assert page.text.count("Split Washer") >= 1 + + +def test_every_empty_section_is_exactly_one_line(web_client): + """Exit criterion 9: empty sections are one line each, on any part.""" + make_part = _create(web_client, name="Bare Make Part") + page = web_client.get(f"/parts/{make_part['id']}") + # make: BOM, STOCK, WHERE USED, PROCEDURE USE, CONSUMED, REQUIREMENTS, TESTS + assert page.text.count('class="empty-line"') == 7 + + buy_part = _create(web_client, name="Bare Buy Part", external_pn="MC-1") + page2 = web_client.get(f"/parts/{buy_part['id']}") + # buy: SUPPLIERS, PO LINES, STOCK, WHERE USED, PROCEDURE USE, REQUIREMENTS, + # TESTS, CONSUMED — BOM absent + assert page2.text.count('class="empty-line"') == 8 + assert page2.text.count('class="panel panel-ledger"') == 0 + + +def test_import_honors_explicit_procurement(web_client): + resp = web_client.post( + "/api/parts/import", + json={ + "rows": [ + {"name": "Imported Declared", "external_pn": "IMP-1", "tier": 1, "procurement": "both"}, + {"name": "Imported Heuristic", "external_pn": "IMP-2", "tier": 1}, + ] + }, + ) + assert resp.status_code in (200, 201), resp.text + + listing = web_client.get("/api/parts", params={"search": "Imported"}).json() + by_name = {p["name"]: p["procurement"] for p in listing["items"]} + assert by_name["Imported Declared"] == "both" + assert by_name["Imported Heuristic"] == "buy" + + +def test_both_renders_all_relevant_sections(web_client): + part = _create(web_client, name="Make And Buy", procurement="both") + page = web_client.get(f"/parts/{part['id']}") + assert ">BOM<" in page.text + assert ">SUPPLIERS<" in page.text + assert ">PO LINES<" in page.text From b4ad36a21f8d56a59f3e56602053989a2e2048b1 Mon Sep 17 00:00:00 2001 From: CST-100 <42722690+CST-100@users.noreply.github.com> Date: Fri, 12 Jun 2026 12:58:03 -0700 Subject: [PATCH 18/79] Rails in boxes, dockbar above the footer, smaller scroll reserve Both rails get the bordered-panel treatment; the docked bar sits above the status footer (offset measured from the footer's real height, set as --opal-footer-h on init/resize); the bottom scroll reserve drops 7rem -> 5.5rem, which was painting a phantom band under the document. Co-Authored-By: Claude Fable 5 --- src/opal/web/static/css/execdoc.css | 12 ++++++++---- src/opal/web/static/js/execdoc.js | 9 +++++++++ 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/opal/web/static/css/execdoc.css b/src/opal/web/static/css/execdoc.css index d97c335..2cc7438 100644 --- a/src/opal/web/static/css/execdoc.css +++ b/src/opal/web/static/css/execdoc.css @@ -134,10 +134,14 @@ top: var(--space-sm); max-height: calc(100vh - 9rem); overflow-y: auto; - background: var(--bg-primary); + border: 1px solid var(--border-color); + background: var(--bg-secondary); } -.exec-rail-left { width: 230px; } +.exec-rail-left { + width: 230px; + padding: var(--space-sm); +} @media (max-width: 1279px) { .exec-rail-left { display: none; } @@ -446,14 +450,14 @@ #dockbar { position: fixed; - bottom: 0; + bottom: var(--opal-footer-h, 0px); /* docked above the status footer */ left: 50%; transform: translateX(-50%); width: min(900px, 100vw); z-index: var(--z-dropdown); } -body.has-dockbar .main { padding-bottom: 7rem; } +body.has-dockbar .main { padding-bottom: 5.5rem; } .dockbar { display: flex; diff --git a/src/opal/web/static/js/execdoc.js b/src/opal/web/static/js/execdoc.js index 6943d21..49d9f15 100644 --- a/src/opal/web/static/js/execdoc.js +++ b/src/opal/web/static/js/execdoc.js @@ -1115,7 +1115,16 @@ } }); + function setFooterOffset() { + const footer = document.querySelector('.footer'); + document.documentElement.style.setProperty( + '--opal-footer-h', footer ? `${footer.offsetHeight}px` : '0px' + ); + } + window.addEventListener('resize', setFooterOffset); + function initExecDoc() { + setFooterOffset(); applyStoredToggles(); loadKitAvailability(); loadStepKitAvailability(); From 33ba966a32cf61ffe91a9ab944c5bfddbd006d50 Mon Sep 17 00:00:00 2001 From: CST-100 <42722690+CST-100@users.noreply.github.com> Date: Fri, 12 Jun 2026 13:00:36 -0700 Subject: [PATCH 19/79] Enter expands the focused step Keyboard parity with row click: Enter toggles the focused step's body (instructions, kit, data readout) without leaving the keyboard. Co-Authored-By: Claude Fable 5 --- src/opal/web/static/js/execdoc.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/opal/web/static/js/execdoc.js b/src/opal/web/static/js/execdoc.js index 49d9f15..0e87fa6 100644 --- a/src/opal/web/static/js/execdoc.js +++ b/src/opal/web/static/js/execdoc.js @@ -431,6 +431,11 @@ document.addEventListener('keydown', (e) => { if (e.target && (['INPUT', 'TEXTAREA', 'SELECT'].includes(e.target.tagName) || e.target.isContentEditable)) return; if (document.querySelector('.execdoc-modal:not([hidden])')) return; + if (e.key === 'Enter' && focusedOrder !== null) { + e.preventDefault(); + toggleStepBody(focusedOrder); + return; + } const down = e.key === 'j' || e.key === 'ArrowDown'; const up = e.key === 'k' || e.key === 'ArrowUp'; if (!down && !up) return; From cb260dcf0f1a7a943edb35904098842fd983da77 Mon Sep 17 00:00:00 2001 From: CST-100 <42722690+CST-100@users.noreply.github.com> Date: Fri, 12 Jun 2026 13:07:45 -0700 Subject: [PATCH 20/79] Step bodies are the control surface; docked bar opt-in and column-aligned MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Expanding a step (click/Enter) reveals its full control surface: editable data fields, notes, ATTACH / ISSUE / SKIP / CONSUME PARTS / COMPLETE (SIGN OFF when awaiting). Collapsed rows stay state-only — the zero-buttons criterion now reads "no controls outside step bodies" and the web test parses the column to enforce it. - The docked bar is hidden by default; the header BAR button toggles it (persisted). completeStep reads capture fields from whichever surface invoked it (step body or bar). - The bar aligns to the document column instead of the viewport center and rides a step above the status footer. Co-Authored-By: Claude Fable 5 --- src/opal/web/static/css/execdoc.css | 21 ++++++- src/opal/web/static/js/execdoc.js | 57 ++++++++++++++++--- .../web/templates/executions/_dockbar.html | 2 +- .../web/templates/executions/_step_row.html | 33 +++++++++++ src/opal/web/templates/executions/detail.html | 1 + tests/unit/test_execution_document_web.py | 39 ++++++++++++- 6 files changed, 139 insertions(+), 14 deletions(-) diff --git a/src/opal/web/static/css/execdoc.css b/src/opal/web/static/css/execdoc.css index 2cc7438..1b1ddea 100644 --- a/src/opal/web/static/css/execdoc.css +++ b/src/opal/web/static/css/execdoc.css @@ -396,6 +396,23 @@ } .step-kit-panel { margin: var(--space-xs) 0; } + +/* Expanded-step control surface: fields, notes, actions. */ +.step-controls { margin-top: var(--space-sm); } + +.step-controls-fields { + display: flex; + gap: var(--space-md); + align-items: flex-end; + flex-wrap: wrap; + margin-bottom: var(--space-xs); +} + +.step-actions { + display: flex; + gap: var(--space-xs); + justify-content: flex-end; +} .step-kit-actions { display: flex; justify-content: flex-end; margin-top: var(--space-xs); } .step-kit-consumed { margin-top: var(--space-xs); font-size: 0.75rem; } @@ -450,7 +467,7 @@ #dockbar { position: fixed; - bottom: var(--opal-footer-h, 0px); /* docked above the status footer */ + bottom: calc(var(--opal-footer-h, 0px) + var(--space-sm)); /* clear of the footer */ left: 50%; transform: translateX(-50%); width: min(900px, 100vw); @@ -458,6 +475,8 @@ } body.has-dockbar .main { padding-bottom: 5.5rem; } +body.dockbar-off #dockbar { display: none; } +body.dockbar-off.has-dockbar .main { padding-bottom: var(--space-md); } .dockbar { display: flex; diff --git a/src/opal/web/static/js/execdoc.js b/src/opal/web/static/js/execdoc.js index 0e87fa6..b27c465 100644 --- a/src/opal/web/static/js/execdoc.js +++ b/src/opal/web/static/js/execdoc.js @@ -144,7 +144,40 @@ } catch (e) { console.error('execdoc: row refresh failed', e); } } + function dockbarVisible() { + return localStorage.getItem('opal_dockbar') === '1'; + } + + function applyDockbarPref() { + document.body.classList.toggle('dockbar-off', !dockbarVisible()); + } + + window.toggleDockbar = function () { + localStorage.setItem('opal_dockbar', dockbarVisible() ? '0' : '1'); + applyDockbarPref(); + if (dockbarVisible()) refreshDockbar(); + }; + + function positionDockbar() { + const bar = document.getElementById('dockbar'); + if (!bar) return; + const doc = document.getElementById('exec-doc'); + if (doc) { + // Align the bar with the document column, not the viewport. + const rect = doc.getBoundingClientRect(); + bar.style.left = `${rect.left}px`; + bar.style.width = `${rect.width}px`; + bar.style.transform = 'none'; + } else { + bar.style.left = ''; + bar.style.width = ''; + bar.style.transform = ''; + } + } + window.addEventListener('resize', positionDockbar); + async function refreshDockbar() { + if (!dockbarVisible()) return; try { const url = pageUrl('/dockbar') + (focusedOrder !== null ? `?step=${focusedOrder}` : ''); const resp = await fetch(url); @@ -160,6 +193,7 @@ 'has-dockbar', !!document.querySelector('#dockbar .dockbar') ); + positionDockbar(); } catch (e) { console.error('execdoc: dockbar refresh failed', e); } } @@ -450,10 +484,9 @@ // ---------- step actions ---------- - function collectBarData() { - const bar = document.getElementById('dockbar-fields'); - if (!bar) return { data: null, errors: [] }; - const fields = bar.querySelectorAll('[data-capture-field]'); + function collectCaptureData(container) { + if (!container) return { data: null, errors: [] }; + const fields = container.querySelectorAll('[data-capture-field]'); if (!fields.length) return { data: null, errors: [] }; const data = {}; const errors = []; @@ -486,17 +519,21 @@ return { data, errors }; } - window.completeStep = async function (order) { + window.completeStep = async function (order, btn) { const body = {}; - const bar = document.getElementById('dockbar'); - if (bar && bar.dataset.barOrder === String(order)) { - const { data, errors } = collectBarData(); + let container = btn ? btn.closest('.doc-step-body, .dockbar') : null; + if (!container) { + const bar = document.getElementById('dockbar'); + if (bar && bar.dataset.barOrder === String(order)) container = bar.querySelector('.dockbar'); + } + if (container) { + const { data, errors } = collectCaptureData(container); if (errors.length) { toastError(errors, 'Cannot complete'); return; } if (data) body.data_captured = data; - const notesEl = document.querySelector('#dockbar .step-notes-input'); + const notesEl = container.querySelector('.step-notes-input'); if (notesEl && notesEl.value.trim()) body.notes = notesEl.value.trim(); } try { @@ -1130,6 +1167,8 @@ function initExecDoc() { setFooterOffset(); + applyDockbarPref(); + positionDockbar(); applyStoredToggles(); loadKitAvailability(); loadStepKitAvailability(); diff --git a/src/opal/web/templates/executions/_dockbar.html b/src/opal/web/templates/executions/_dockbar.html index aba35c4..9c27c77 100644 --- a/src/opal/web/templates/executions/_dockbar.html +++ b/src/opal/web/templates/executions/_dockbar.html @@ -58,7 +58,7 @@ {% if actionable and bs == 'awaiting_signoff' %} {% elif workable and not bar_step.has_children %} - + {% endif %} {% endif %}
diff --git a/src/opal/web/templates/executions/_step_row.html b/src/opal/web/templates/executions/_step_row.html index c9ab45b..6577729 100644 --- a/src/opal/web/templates/executions/_step_row.html +++ b/src/opal/web/templates/executions/_step_row.html @@ -83,6 +83,38 @@
{% endif %} + {% set body_schema = step.required_data_schema or vs.get('required_data_schema') %} + {% set workable = inst_status in ['cut', 'in_work'] and step.status in ['pending', 'in_progress'] %} + {% if workable and exec %} + {# The expanded step is a control surface: fields, notes, actions. #} +
+
+ {{ ex.data_fields_editable({'exec': exec, 'schema': body_schema}) }} +
+ + +
+
+
+ + + + {% if step_kit %} + + {% endif %} + +
+
+ {% elif inst_status in ['cut', 'in_work'] and step.status == 'awaiting_signoff' %} +
+
+ {% if exec %}{% endif %} + + +
+
+ {% else %} {{ ex.data_readout(step) }} {% if exec and exec.notes %} @@ -91,5 +123,6 @@
{{ exec.notes }}
{% endif %} + {% endif %} diff --git a/src/opal/web/templates/executions/detail.html b/src/opal/web/templates/executions/detail.html index 6afce74..a6a90a7 100644 --- a/src/opal/web/templates/executions/detail.html +++ b/src/opal/web/templates/executions/detail.html @@ -34,6 +34,7 @@ {% if inst_status in ['cut', 'in_work'] %} {% endif %} + diff --git a/tests/unit/test_execution_document_web.py b/tests/unit/test_execution_document_web.py index 9655f84..29c3c9a 100644 --- a/tests/unit/test_execution_document_web.py +++ b/tests/unit/test_execution_document_web.py @@ -105,6 +105,37 @@ def _doc_column(html: str) -> str: return html.split('id="exec-doc"')[1].split(" int: + """Count - {% if workable or (bar_step.op_open_ncs and not bar_step.op_is_ad_hoc) or bar_step.step_kit %} + {% if menu_skip or menu_consume or menu_redline %}
@@ -58,9 +61,13 @@ {% if actionable and bs == 'awaiting_signoff' %} {% elif workable and not bar_step.has_children %} + {% if bar_step.raised_holds %} + COMPLETE — held by {% for nc in bar_step.raised_holds %}{{ nc.issue_number }}{% if not loop.last %}, {% endif %}{% endfor %} + {% else %} {% endif %} {% endif %} + {% endif %}
{% endif %} diff --git a/src/opal/web/templates/executions/_step_row.html b/src/opal/web/templates/executions/_step_row.html index e4a89a4..d5a4370 100644 --- a/src/opal/web/templates/executions/_step_row.html +++ b/src/opal/web/templates/executions/_step_row.html @@ -99,7 +99,7 @@
- + {% if not step_raised_holds %} {% endif %} @@ -117,7 +117,7 @@
{% if exec %}{% endif %} - +
diff --git a/src/opal/web/templates/executions/tabs/bom.html b/src/opal/web/templates/executions/tabs/bom.html index bc63d65..ad70527 100644 --- a/src/opal/web/templates/executions/tabs/bom.html +++ b/src/opal/web/templates/executions/tabs/bom.html @@ -1,3 +1,4 @@ +{% import 'opalkit/_macros.html' as ok %}
BOM RECONCILIATION
@@ -17,7 +18,7 @@ {% else %} -

No BOM items defined for this procedure.

+ {{ ok.empty_line("BOM") }} {% endif %} {% if unplanned_consumptions %} diff --git a/src/opal/web/templates/executions/tabs/data.html b/src/opal/web/templates/executions/tabs/data.html index 3568255..58b282f 100644 --- a/src/opal/web/templates/executions/tabs/data.html +++ b/src/opal/web/templates/executions/tabs/data.html @@ -1,3 +1,4 @@ +{% import 'opalkit/_macros.html' as ok %}
@@ -31,7 +32,7 @@ {% else %} -

No structured data has been captured yet for this execution.

+ {{ ok.empty_line("Captured data") }} {% endif %}
diff --git a/src/opal/web/templates/executions/tabs/kitting.html b/src/opal/web/templates/executions/tabs/kitting.html index d867b54..d048ff9 100644 --- a/src/opal/web/templates/executions/tabs/kitting.html +++ b/src/opal/web/templates/executions/tabs/kitting.html @@ -1,3 +1,4 @@ +{% import 'opalkit/_macros.html' as ok %}
{% if kit_items %}
@@ -173,7 +174,7 @@ {% if not kit_items and not productions and not output_items %}
KITTING
-

No kit, output, or production records for this procedure.

+
{{ ok.empty_line("Kit") }}
{% endif %}
From 7b8b232dccb684dfe59daf8da5d78190d2d5bf43 Mon Sep 17 00:00:00 2001 From: CST-100 <42722690+CST-100@users.noreply.github.com> Date: Sun, 5 Jul 2026 13:19:40 -0700 Subject: [PATCH 25/79] Fix review findings: XSS attribute quoting, step-image 500, stale link, SSE From the PR #56 review: - Stored XSS (#1): step titles, image captions, attachment notes/filenames were piped through | tojson inside DOUBLE-quoted onclick attributes, which tojson does not escape (it escapes < > & ' but not "). A title containing a double quote could inject a new event-handler attribute. Switched the seven execution-template onclick handlers to single-quoted attributes (with inner URL literals double-quoted), matching the confirm_btn/_ledger fix. Added a hostile-title regression test that HTML-parses the page and asserts no onmouseover attribute is injected. - Step-image upload 500 (#6): add_step_image calls upload_attachment as a plain function; the omitted Form(...) params defaulted to truthy sentinel objects that reached the entity-validation queries. Pass explicit None. Added the missing endpoint test. - Stale dashboard link (#9): EXECUTIONS IN PROGRESS linked ?status=in_progress, which no longer exists after the CUT/IN_WORK rename; now ?status=in_work. - issue_dispositioned SSE (#10): execdoc.js now subscribes to the event, so held rows recover controls on disposition without waiting for the 5s poll. Co-Authored-By: Claude Fable 5 --- src/opal/api/routes/procedures.py | 15 +++++---- src/opal/web/static/js/execdoc.js | 2 +- .../web/templates/executions/_dockbar.html | 2 +- .../templates/executions/_exec_macros.html | 6 ++-- src/opal/web/templates/executions/_rail.html | 4 +-- .../web/templates/executions/_step_row.html | 2 +- src/opal/web/templates/index.html | 2 +- tests/unit/test_execution_document_web.py | 32 +++++++++++++++++++ tests/unit/test_procedures.py | 26 +++++++++++++++ 9 files changed, 76 insertions(+), 15 deletions(-) diff --git a/src/opal/api/routes/procedures.py b/src/opal/api/routes/procedures.py index e81a162..a3e449a 100644 --- a/src/opal/api/routes/procedures.py +++ b/src/opal/api/routes/procedures.py @@ -655,12 +655,19 @@ async def add_step_image( from opal.api.routes.attachments import upload_attachment + # Called as a plain function, not through FastAPI: the omitted Form(...) + # parameters would otherwise default to truthy sentinel objects, so pass + # them explicitly as None. attachment = await upload_attachment( db=db, user_id=user_id, file=file, + procedure_instance_id=None, + step_execution_id=None, + issue_id=None, procedure_id=procedure_id, kind="step_image", + note=None, ) max_position = ( @@ -695,9 +702,7 @@ def update_step_image( """Update a step image's caption.""" _get_procedure_step(db, procedure_id, step_id) image = ( - db.query(StepImage) - .filter(StepImage.id == image_id, StepImage.step_id == step_id) - .first() + db.query(StepImage).filter(StepImage.id == image_id, StepImage.step_id == step_id).first() ) if not image: raise HTTPException(status_code=404, detail="Step image not found") @@ -726,9 +731,7 @@ def delete_step_image( """ _get_procedure_step(db, procedure_id, step_id) image = ( - db.query(StepImage) - .filter(StepImage.id == image_id, StepImage.step_id == step_id) - .first() + db.query(StepImage).filter(StepImage.id == image_id, StepImage.step_id == step_id).first() ) if not image: raise HTTPException(status_code=404, detail="Step image not found") diff --git a/src/opal/web/static/js/execdoc.js b/src/opal/web/static/js/execdoc.js index 28cf183..69bf435 100644 --- a/src/opal/web/static/js/execdoc.js +++ b/src/opal/web/static/js/execdoc.js @@ -1230,7 +1230,7 @@ // SSE accelerates the poll; the poll remains the source of truth. if (window.opalEvents) { - ['cursor_moved', 'step_completed', 'user_joined', 'user_left'].forEach((type) => { + ['cursor_moved', 'step_completed', 'user_joined', 'user_left', 'issue_dispositioned'].forEach((type) => { window.opalEvents.on(type, (data) => { if (data && data.instance_id === instanceId) pollNow(); }); diff --git a/src/opal/web/templates/executions/_dockbar.html b/src/opal/web/templates/executions/_dockbar.html index 347919a..2661403 100644 --- a/src/opal/web/templates/executions/_dockbar.html +++ b/src/opal/web/templates/executions/_dockbar.html @@ -47,7 +47,7 @@