From 0e835253058d3b0f73490462d8b1d4f04bd1aac9 Mon Sep 17 00:00:00 2001 From: Anto01 Date: Wed, 29 Apr 2026 13:00:56 -0400 Subject: [PATCH] fix(engineering): close Codex V1-A audit conditions (spec key + extra coverage) Codex's audit of b575773 returned GO WITH CONDITIONS, all small. P2 - Q-001 child shape now exposes the catalog-spec key. engineering-query-catalog.md Q-001 specifies {id, type, name, status}. Implementation now emits both `type` and `entity_type` with the same value: spec compliance for new consumers, parity with the rest of the queries module (which uses entity_type) for existing ones. P2 - V1-A acceptance test now exercises Q-009 + Q-011 against the already-seeded data. The seed had a flagged-assumption decision and an unsupported validation claim but neither was being asserted; new test pins risky_decisions() and unsupported_claims() against the expected names. P3 - /v1 alias coverage upgraded from OpenAPI presence to a real GET /v1/entities/Subsystem/{id}?expand=contains assertion. Catches route-copy or ordering regressions, not just spec drift. P3 - new test pins two intended Q-001 behaviors that aren't obvious from the data shape: child *Subsystems* surface in `contains` (the impl walks part_of irrespective of child entity_type), and inactive children are filtered out (uses invalidate_active_entity to seed). Tests: 602 -> 605 (+3). New count on branch: 9 V1-A tests. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/atocore/engineering/queries.py | 4 ++ tests/test_v1_a_pillar_queries.py | 74 ++++++++++++++++++++++++++++-- 2 files changed, 75 insertions(+), 3 deletions(-) diff --git a/src/atocore/engineering/queries.py b/src/atocore/engineering/queries.py index fb1b37f..40cce07 100644 --- a/src/atocore/engineering/queries.py +++ b/src/atocore/engineering/queries.py @@ -169,6 +169,10 @@ def subsystem_contents(subsystem_id: str) -> dict | None: "contains": [ { "id": r["id"], + # V1-A spec uses `type` per engineering-query-catalog.md Q-001; + # `entity_type` is duplicated for parity with the rest of + # this module's response shape (see `_entity_dict`). + "type": r["entity_type"], "entity_type": r["entity_type"], "name": r["name"], "status": r["status"], diff --git a/tests/test_v1_a_pillar_queries.py b/tests/test_v1_a_pillar_queries.py index b2ae194..71a872e 100644 --- a/tests/test_v1_a_pillar_queries.py +++ b/tests/test_v1_a_pillar_queries.py @@ -20,12 +20,15 @@ from atocore.engineering.queries import ( evidence_chain, orphan_requirements, requirements_for, + risky_decisions, subsystem_contents, + unsupported_claims, ) from atocore.engineering.service import ( create_entity, create_relationship, init_engineering_schema, + invalidate_active_entity, ) from atocore.main import app from atocore.models.database import init_db @@ -105,8 +108,11 @@ def v1a_seed(tmp_data_dir): def test_subsystem_contents_returns_spec_shape(v1a_seed): - """The shape must be exactly what the catalog promises: - {subsystem, contains: [{id, entity_type, name, status}]}.""" + """The shape must match the catalog spec + (engineering-query-catalog.md Q-001): + ``{subsystem, contains: [{id, type, name, status}]}``. + The implementation also emits ``entity_type`` for parity with the + rest of this module's response style — both must be present.""" result = subsystem_contents(v1a_seed["subsystem"].id) assert result is not None @@ -120,7 +126,11 @@ def test_subsystem_contents_returns_spec_shape(v1a_seed): contained = {c["name"] for c in result["contains"]} assert contained == {"Primary Mirror", "Diverger Lens"} for child in result["contains"]: - assert set(child.keys()) == {"id", "entity_type", "name", "status"} + # Spec requires `type`; we also include `entity_type` for parity. + assert "type" in child + assert "entity_type" in child + assert child["type"] == child["entity_type"] + assert set(child.keys()) >= {"id", "type", "entity_type", "name", "status"} assert child["entity_type"] == "component" assert child["status"] == "active" @@ -172,6 +182,47 @@ def test_subsystem_contents_v1_alias_is_present(): assert "/v1/entities/Subsystem/{subsystem_id}" in spec["paths"] +def test_subsystem_contents_v1_alias_serves_real_response(v1a_seed): + """Behavior, not just OpenAPI presence — catches a future + route-copy or ordering regression (Codex V1-A P3).""" + client = TestClient(app) + sid = v1a_seed["subsystem"].id + + r = client.get(f"/v1/entities/Subsystem/{sid}?expand=contains") + assert r.status_code == 200 + body = r.json() + assert body["subsystem"]["id"] == sid + assert {c["name"] for c in body["contains"]} == {"Primary Mirror", "Diverger Lens"} + + +def test_subsystem_contents_includes_child_subsystems_and_excludes_inactive(tmp_data_dir): + """Codex V1-A P3: lock in two intended behaviors that aren't + obvious from the data shape: + 1. Child *Subsystems* (nested) surface in `contains` — the impl + walks part_of irrespective of child entity_type. + 2. Children whose status is not 'active' are filtered out.""" + init_db() + init_engineering_schema() + + parent = create_entity("subsystem", "Parent System", project="p-nested") + child_ss = create_entity("subsystem", "Child Subsystem", project="p-nested") + child_comp = create_entity("component", "Child Component", project="p-nested") + invalid_comp = create_entity("component", "Invalidated Component", project="p-nested") + create_relationship(child_ss.id, parent.id, "part_of") + create_relationship(child_comp.id, parent.id, "part_of") + create_relationship(invalid_comp.id, parent.id, "part_of") + + invalidate_active_entity(invalid_comp.id, reason="v1a regression seed") + + result = subsystem_contents(parent.id) + names_by_type = {(c["entity_type"], c["name"]) for c in result["contains"]} + assert ("subsystem", "Child Subsystem") in names_by_type + assert ("component", "Child Component") in names_by_type + # Inactive child must not appear + assert all(c["name"] != "Invalidated Component" for c in result["contains"]) + assert all(c["status"] == "active" for c in result["contains"]) + + # --------------------------------------------------------------------------- # V1-A acceptance — the four pillar queries against the single seed graph # --------------------------------------------------------------------------- @@ -208,3 +259,20 @@ def test_v1a_pillar_queries_run_end_to_end_against_single_seed(v1a_seed): # And the unsupported claim must have an empty supports chain q17_unsup = evidence_chain(v1a_seed["claim_unsupported"].id) assert not any(edge["via"] == "supports" for edge in q17_unsup["evidence_chain"]) + + +def test_v1a_seed_also_exercises_q009_and_q011_killers(v1a_seed): + """Per Codex V1-A P2: the seed already includes Q-009 (decision on + flagged assumption) and Q-011 (unsupported validation claim) data, + so the V1-A integration test should assert those killer queries + surface them. Otherwise the seed entities are dead weight.""" + project = "p05-interferometer" + + q9 = risky_decisions(project) + risky_names = {row["decision_name"] for row in q9["gaps"]} + assert "Pre-order CGH from external vendor" in risky_names + + q11 = unsupported_claims(project) + unsupported_names = {row["name"] for row in q11["gaps"]} + assert "Vibration isolation passes spec" in unsupported_names + assert "Thermal margin is adequate" not in unsupported_names