From 3eda056149442db0388cded982a264ffed1e7eb1 Mon Sep 17 00:00:00 2001 From: Thomas Chopitea Date: Sat, 8 Aug 2026 16:47:10 +0000 Subject: [PATCH 1/2] Remove redundant dfiq_type request field from DFIQ create/validate/patch Every DFIQ create/validate/patch request already carries the object's type embedded in its dfiq_yaml body (`type: scenario|facet|question`). The separate `dfiq_type` request field duplicated that: - On /from_yaml and /validate, it only selected which pydantic subclass parsed the YAML, and that subclass already re-checks its own `type:` field and raises ValueError on mismatch -- so the parameter could only ever agree with the YAML or cause a 400. DFIQBase.from_yaml (used by the batch/feed ingestion path) already dispatches on the YAML's own embedded type with no separate parameter, proving one isn't needed here either. - On PATCH /{id}, it was outright dead: the handler dispatches on db_dfiq.type (the object's existing type from the database), never reads request.dfiq_type, yet every caller was required to send it. Switches /from_yaml and /validate to the existing generic DFIQBase.from_yaml dispatcher, and drops dfiq_type from all three request schemas (extra="forbid" means callers must stop sending it, hence the test updates -- no test exercised type-mismatch behavior, so this is a mechanical payload cleanup, not a coverage change). Also tightens DFIQBase.from_yaml itself to parse via parse_yaml (which every subclass's own from_yaml already does) instead of a bare yaml.safe_load -- needed so a malformed/missing 'type' now raises the same ValueError every other path already guarantees, rather than an uncaught KeyError/YAMLError this generic dispatcher previously let through (relevant now that /from_yaml and /validate lean on it more). Paired with a frontend fix (yeti-feeds-frontend) that stops sending dfiq_type and fixes a real UI inconsistency the same redundancy was masking (Approaches tab and Parents field disagreeing about an object's type when its raw YAML is hand-edited). Verified: full tests/apiv2/dfiq.py (18/18) and tests/apiv2/timeline.py pass; tests/schemas, tests/apiv2, tests/core_tests show zero new failures vs a clean origin/main baseline (pre-existing environmental failures only: bloomcheck config, missing system config, missing default group seed data); ruff clean on all touched lines; confirmed via generated OpenAPI schema that dfiq_type no longer appears on NewDFIQRequest/DFIQValidateRequest/PatchDFIQRequest. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_011tXLywVq32tGPE7jCrdZVJ --- core/schemas/dfiq.py | 6 +++++- core/web/apiv2/dfiq.py | 9 ++------- tests/apiv2/dfiq.py | 21 ++++----------------- tests/apiv2/timeline.py | 3 --- 4 files changed, 11 insertions(+), 28 deletions(-) diff --git a/core/schemas/dfiq.py b/core/schemas/dfiq.py index 5357ac631..acaf9dd13 100644 --- a/core/schemas/dfiq.py +++ b/core/schemas/dfiq.py @@ -240,7 +240,11 @@ def parse_yaml(cls, yaml_string: str) -> dict[str, Any]: @classmethod def from_yaml(cls, yaml_string: str) -> "DFIQBase": - yaml_data = yaml.safe_load(yaml_string) + # parse_yaml (not a bare yaml.safe_load) so a malformed/missing/ + # unrecognized 'type' raises the same ValueError every subclass's own + # from_yaml already guarantees, instead of a KeyError/YAMLError this + # generic dispatcher used to let through uncaught. + yaml_data = cls.parse_yaml(yaml_string) return TYPE_MAPPING[yaml_data["type"]].from_yaml(yaml_string) def to_yaml(self, sort_keys=False) -> str: diff --git a/core/web/apiv2/dfiq.py b/core/web/apiv2/dfiq.py index 5452f10fb..a46b722fe 100644 --- a/core/web/apiv2/dfiq.py +++ b/core/web/apiv2/dfiq.py @@ -18,7 +18,6 @@ class NewDFIQRequest(BaseModel): model_config = ConfigDict(extra="forbid") dfiq_yaml: str - dfiq_type: dfiq.DFIQType update_indicators: bool = False @@ -40,7 +39,6 @@ class PatchDFIQRequest(BaseModel): dfiq_yaml: str | None = None dfiq_object: dfiq.DFIQTypes | None = None - dfiq_type: dfiq.DFIQType update_indicators: bool = False @@ -131,10 +129,7 @@ def from_archive(httpreq: Request, archive: UploadFile) -> dict[str, int]: def new_from_yaml(httpreq: Request, request: NewDFIQRequest) -> dfiq.DFIQTypes: """Creates a new DFIQ object in the database.""" try: - new = cast( - "dfiq.DFIQTypes", - dfiq.TYPE_MAPPING[request.dfiq_type].from_yaml(request.dfiq_yaml), - ) + new = cast("dfiq.DFIQTypes", dfiq.DFIQBase.from_yaml(request.dfiq_yaml)) except ValueError as error: raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=str(error)) @@ -276,7 +271,7 @@ def to_archive(httpreq: Request, request: DFIQSearchRequest) -> FileResponse: def validate_dfiq_yaml(request: DFIQValidateRequest) -> DFIQValidateResponse: """Validates a DFIQ YAML string.""" try: - obj = dfiq.TYPE_MAPPING[request.dfiq_type].from_yaml(request.dfiq_yaml) + obj = dfiq.DFIQBase.from_yaml(request.dfiq_yaml) except ValidationError as error: error_objs: list[dict] = [] for pydantic_error in error.errors(): diff --git a/tests/apiv2/dfiq.py b/tests/apiv2/dfiq.py index ecd7a26f6..4524ef834 100644 --- a/tests/apiv2/dfiq.py +++ b/tests/apiv2/dfiq.py @@ -46,7 +46,6 @@ def test_config(self) -> None: "/api/v2/dfiq/from_yaml", json={ "dfiq_yaml": yaml_string, - "dfiq_type": dfiq.DFIQType.question, }, ) data = response.json() @@ -70,7 +69,6 @@ def test_new_dfiq_scenario(self) -> None: "/api/v2/dfiq/from_yaml", json={ "dfiq_yaml": yaml_string, - "dfiq_type": dfiq.DFIQType.scenario, }, ) data = response.json() @@ -92,7 +90,6 @@ def test_get_dfiq_by_name(self) -> None: "/api/v2/dfiq/from_yaml", json={ "dfiq_yaml": yaml_string, - "dfiq_type": dfiq.DFIQType.scenario, }, ) data = response.json() @@ -121,7 +118,6 @@ def test_new_dfiq_facet(self) -> None: "/api/v2/dfiq/from_yaml", json={ "dfiq_yaml": yaml_string, - "dfiq_type": dfiq.DFIQType.facet, }, ) data = response.json() @@ -161,7 +157,6 @@ def test_new_dfiq_question(self) -> None: "/api/v2/dfiq/from_yaml", json={ "dfiq_yaml": yaml_string, - "dfiq_type": dfiq.DFIQType.question, }, ) data = response.json() @@ -256,7 +251,7 @@ def test_dfiq_patch_yaml(self) -> None: response = client.patch( f"/api/v2/dfiq/{scenario.id}", - json={"dfiq_yaml": yaml_string, "dfiq_type": scenario.type}, + json={"dfiq_yaml": yaml_string}, ) data = response.json() self.assertEqual(response.status_code, 200, data) @@ -323,7 +318,7 @@ def test_dfiq_patch_object(self): question_json = json.loads(question.model_dump_json()) response = client.patch( f"/api/v2/dfiq/{question.id}", - json={"dfiq_object": question_json, "dfiq_type": question.type}, + json={"dfiq_object": question_json}, ) data = response.json() self.assertEqual(response.status_code, 200, data) @@ -370,7 +365,7 @@ def test_dfiq_patch_updates_parents(self) -> None: response = client.patch( f"/api/v2/dfiq/{facet.id}", - json={"dfiq_yaml": facet.to_yaml(), "dfiq_type": facet.type}, + json={"dfiq_yaml": facet.to_yaml()}, ) data = response.json() self.assertEqual(response.status_code, 200, data) @@ -428,7 +423,7 @@ def test_dfiq_patch_prunes_parents(self) -> None: response = client.patch( f"/api/v2/dfiq/{facet.id}", - json={"dfiq_yaml": facet.to_yaml(), "dfiq_type": facet.type}, + json={"dfiq_yaml": facet.to_yaml()}, ) data = response.json() self.assertEqual(response.status_code, 200, data) @@ -490,7 +485,6 @@ def test_dfiq_patch_question_updates_indicators(self) -> None: f"/api/v2/dfiq/{question.id}", json={ "dfiq_yaml": yaml_string, - "dfiq_type": question.type, "update_indicators": False, }, ) @@ -504,7 +498,6 @@ def test_dfiq_patch_question_updates_indicators(self) -> None: f"/api/v2/dfiq/{question.id}", json={ "dfiq_yaml": yaml_string, - "dfiq_type": question.type, "update_indicators": True, }, ) @@ -522,7 +515,6 @@ def test_wrong_parent(self) -> None: "/api/v2/dfiq/from_yaml", json={ "dfiq_yaml": yaml_string, - "dfiq_type": dfiq.DFIQType.facet, }, ) data = response.json() @@ -539,7 +531,6 @@ def test_valid_dfiq_yaml(self) -> None: "/api/v2/dfiq/validate", json={ "dfiq_yaml": yaml_string, - "dfiq_type": dfiq.DFIQType.scenario, "check_id": True, }, ) @@ -554,7 +545,6 @@ def test_valid_dfiq_yaml(self) -> None: "/api/v2/dfiq/validate", json={ "dfiq_yaml": yaml_string, - "dfiq_type": dfiq.DFIQType.facet, "check_id": True, }, ) @@ -569,7 +559,6 @@ def test_valid_dfiq_yaml(self) -> None: "/api/v2/dfiq/validate", json={ "dfiq_yaml": yaml_string, - "dfiq_type": dfiq.DFIQType.question, "check_id": True, }, ) @@ -585,7 +574,6 @@ def test_standalone_question_creation(self): "/api/v2/dfiq/from_yaml", json={ "dfiq_yaml": yaml_string, - "dfiq_type": dfiq.DFIQType.question, }, ) data = response.json() @@ -701,7 +689,6 @@ def test_get_multiple(self): "/api/v2/dfiq/from_yaml", json={ "dfiq_yaml": yaml_string, - "dfiq_type": dfiq.DFIQType.scenario, }, ) data = response.json() diff --git a/tests/apiv2/timeline.py b/tests/apiv2/timeline.py index 6f2797fd8..f14bfd68b 100644 --- a/tests/apiv2/timeline.py +++ b/tests/apiv2/timeline.py @@ -330,7 +330,6 @@ def test_new_dfiq_makes_timeline_log(self): "/api/v2/dfiq/from_yaml", json={ "dfiq_yaml": yaml_string, - "dfiq_type": "question", "update_indicators": True, }, ) @@ -430,7 +429,6 @@ def test_delete_dfiq_makes_timeline_log(self): "/api/v2/dfiq/from_yaml", json={ "dfiq_yaml": yaml_string, - "dfiq_type": "facet", }, ) @@ -445,7 +443,6 @@ def test_delete_dfiq_makes_timeline_log(self): "/api/v2/dfiq/from_yaml", json={ "dfiq_yaml": yaml_string, - "dfiq_type": "question", }, ) data = response.json() From b234f37b0eb4fa6db9453065430242eab8a69b2d Mon Sep 17 00:00:00 2001 From: Thomas Chopitea Date: Sat, 8 Aug 2026 17:46:31 +0000 Subject: [PATCH 2/2] Drop fix-narration comment on DFIQBase.from_yaml Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_011tXLywVq32tGPE7jCrdZVJ --- core/schemas/dfiq.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/core/schemas/dfiq.py b/core/schemas/dfiq.py index acaf9dd13..777af8792 100644 --- a/core/schemas/dfiq.py +++ b/core/schemas/dfiq.py @@ -240,10 +240,6 @@ def parse_yaml(cls, yaml_string: str) -> dict[str, Any]: @classmethod def from_yaml(cls, yaml_string: str) -> "DFIQBase": - # parse_yaml (not a bare yaml.safe_load) so a malformed/missing/ - # unrecognized 'type' raises the same ValueError every subclass's own - # from_yaml already guarantees, instead of a KeyError/YAMLError this - # generic dispatcher used to let through uncaught. yaml_data = cls.parse_yaml(yaml_string) return TYPE_MAPPING[yaml_data["type"]].from_yaml(yaml_string)