Skip to content

Commit 931a0ab

Browse files
dwolfsonclaude
andauthored
fix(tests): stale shop_for_data assertion; chore(release): bump version to 6.1.12 (#351)
* ISSUE-93: declassify_metadata_element raises when its Optional body is omitted `MetadataExpert.declassify_metadata_element(guid, classification_name, body=None)` declares `body` as Optional but calls `.model_dump()` on it unconditionally, so omitting it raises `AttributeError: 'NoneType' object has no attribute 'model_dump'` and the classification is left in place. Worse than an ordinary signature bug because the failure is silent at the caller. A caller that wraps the call defensively — reasonable, since removing a classification that is already absent should not be fatal — swallows the AttributeError and believes the removal happened. Measured live: kinds before ['Task'], declassify raises, kinds after ['Task']. In our case the element then carried TWO kind classifications at once (['Task', 'PersonalProject']) and a check that only asked "is the new classification present?" reported success. The wrong-but-plausible state was invisible until the element was read back for BOTH names. Workaround verified: passing `{"class": "MetadataSourceRequestBody"}` explicitly makes it work (kinds after: []). Candidate fix is to default the body when None, matching the declaration. `classify_metadata_element` and `reclassify_metadata_element` sit beside it with the same Optional body declaration and are worth checking — we pass a body to both so we have not hit them. Logged, not fixed, per the standing rule for this repo. Found in trellis/packages/resource-explorer's investigation reclassifier, which now passes the body and additionally asserts the OLD classification is gone rather than only that the new one arrived. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Dan Wolfson <dan.wolfson@pdr-associates.com> * fix(tests): update stale shop_for_data catalog-search assertion; chore(release): bump version to 6.1.12 test_handle_shop_for_data_option_success asserted the Digital-Product-Catalog-MyE search call's params as {"search_string": "*"} only. PR #346 added a metadata_element_subtypes: ["DigitalProduct", "DigitalProductFamily"] filter to that call; the test was never updated to match, so it failed deterministically (not flaky) on every run since -- this is what blocked the v6.1.11 release workflow's PyPI publish step (build never ran; see release run 34252940497). Also bumps to 6.1.12: PyPI already had a 6.1.11 published at 2026-09-08T02:15 UTC, ~14.5h before this repo's v6.1.11 tag/PR merged, with different (older) content -- a pre-existing version collision unrelated to this fix. 6.1.12 is the first clean, unused version number. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KYNxABE3SRUT5cXbpmrfU2 Signed-off-by: Dan Wolfson <dan.wolfson@pdr-associates.com> --------- Signed-off-by: Dan Wolfson <dan.wolfson@pdr-associates.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 48d7319 commit 931a0ab

3 files changed

Lines changed: 65 additions & 3 deletions

File tree

‎PYEGERIA_ISSUES.md‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,63 @@ security context changes; (2) quickstart content: give `generalnpa` read access
199199
elements its engine actions anchor to, or anchor those actions to elements the engine-host identity can
200200
read. Full draft: trellis session scratch `egeria-issue-engine-host-403-loop.md`.
201201

202+
### ISSUE-93: `declassify_metadata_element` raises `AttributeError` when `body` is omitted, though the parameter is Optional — and the classification is silently left in place
203+
204+
**Layer:** Pyegeria · **Status:** open · **Found:** 2026-09-08 (Resource Explorer, swapping a Project's kind classification during investigation reclassification).
205+
206+
`MetadataExpert.declassify_metadata_element(metadata_element_guid, classification_name, body=None)`
207+
declares `body` as `Optional[dict | MetadataSourceRequestBody] = None`, but the
208+
implementation calls `.model_dump()` on it unconditionally. Omitting it raises:
209+
210+
```
211+
AttributeError: 'NoneType' object has no attribute 'model_dump'
212+
```
213+
214+
**Why this is worse than an ordinary signature bug:** the exception arrives from
215+
deep inside the client, so a caller that wraps the call defensively — which is
216+
reasonable, since a classification that is already absent should not be fatal —
217+
swallows it and *believes the classification was removed*. It was not. Measured
218+
live:
219+
220+
```python
221+
kinds before: ['Task']
222+
declassify_metadata_element(guid, "Task") # raises AttributeError
223+
kinds after: ['Task'] # unchanged
224+
```
225+
226+
In our case the element then carried **two** kind classifications at once
227+
(`['Task', 'PersonalProject']`) — the new one added, the old one never removed —
228+
and a check that only asked "is the new classification present?" reported
229+
success. The wrong-but-plausible state was invisible until the element was read
230+
back for *both* names.
231+
232+
**Working around it** — pass an explicit body:
233+
234+
```python
235+
me.declassify_metadata_element(guid, "Task", {"class": "MetadataSourceRequestBody"})
236+
# kinds after: []
237+
```
238+
239+
**Candidate fix:** default the body when `None`, matching the Optional
240+
declaration — the same shape the workaround passes:
241+
242+
```python
243+
body = body or {"class": "MetadataSourceRequestBody"}
244+
```
245+
246+
**Worth checking while in there:** `classify_metadata_element` and
247+
`reclassify_metadata_element` sit beside it with the same `Optional` body
248+
declaration. We pass a body to both so we have not hit them, but if they share
249+
the unconditional `.model_dump()` they have the same defect — and the
250+
declassify case shows the failure mode is silent at the caller, not loud.
251+
252+
**Where seen:** `trellis/packages/resource-explorer/resource_explorer/surveyors/
253+
investigation_reclassifier.py::_move_kind_classification`, which now passes the
254+
body explicitly and additionally verifies that the OLD classification is gone
255+
rather than only that the new one is present.
256+
257+
---
258+
202259
### ISSUE-92: `Project Type`'s description in `commands_project_compact.json` lists only 4 of its 6 `valid_values` — omits `Project` and `Experiment`, and the stale text is baked into 24 generated files
203260

204261
**Layer:** Pyegeria · **Status:** open · **Found:** 2026-09-07 (Resource

‎pyproject.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
[project]
55
name = "pyegeria"
6-
version = "6.1.11"
6+
version = "6.1.12"
77
license = 'Apache-2.0'
88
license-files = ["LICENSE"]
99
authors = [{name = "Dan Wolfson", email = "dan.wolfson@pdr-associates.com"},]

‎tests/micro-tests/my_profile/test_shop_for_data_handler.py‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,13 +97,18 @@ async def test_handle_shop_for_data_option_success(self, mock_exec, mock_table_c
9797
assert isinstance(screen, ShopForDataScreen)
9898
assert cb == app.shop_for_data_callback
9999

100-
# Verify that Digital-Product-Catalog-MyE was called with params={"search_string": "*"}
100+
# Verify that Digital-Product-Catalog-MyE was called with the search string plus the
101+
# DigitalProduct/DigitalProductFamily subtype filter (added #346, this assertion was
102+
# stale until fixed alongside the 6.1.12 release).
101103
catalog_calls = [
102104
call for call in mock_exec.call_args_list
103105
if call.kwargs.get("format_set_name") == "Digital-Product-Catalog-MyE"
104106
]
105107
assert len(catalog_calls) == 1
106-
assert catalog_calls[0].kwargs.get("params") == {"search_string": "*"}
108+
assert catalog_calls[0].kwargs.get("params") == {
109+
"search_string": "*",
110+
"metadata_element_subtypes": ["DigitalProduct", "DigitalProductFamily"],
111+
}
107112

108113
@pytest.mark.asyncio
109114
@patch("shop_for_data_handler.DataTable")

0 commit comments

Comments
 (0)