fix(pyegeria): resolve OMVS audit mismatches -- broken helper calls, wrong body classes, tool hardening - #281
Merged
dwolfson merged 8 commits intoAug 18, 2026
Conversation
…ody, arity, classification helper These three fixes were made and verified in an isolated Agent worktree (egeria-python-omvs-audit-followup) that the harness auto-cleaned once its commits were pushed and PR odpi#277 merged. Because these three fixes were still uncommitted at that point, they were lost with the directory rather than merged. Reapplied here from scratch on top of PR odpi#277's tip (confirmed absent via egeria-python-73, a peer session, before redoing). 1. governance_officer.get_governance_action_process_graph sent GetRequestBody where Egeria-api-governance-officer.http documents ResultsRequestBody. Added an opt-in body_model param to _async_get_guid_request (default unchanged: GetRequestBody) so this one call can send the documented body without touching the other 24 callers of that helper. Kept on _async_get_guid_request rather than switching to _async_get_results_body_request, because only this helper reads the singular "elementGraph" response key the graph endpoint returns. 2. Six *_from_template methods (actor_manager x3, data_discovery, subject_area, time_keeper) called _async_create_element_body_request instead of _async_create_element_from_template -- silently dropping the 8 fields only TemplateRequestBody has (template_guid, placeholder_property_values, replacement_properties, deep_copy, ...) via PyegeriaModel's extra='ignore'. The element was created with no template ever applied, no error raised. Also fixed 4 call sites (actor_manager x3, location_arena) passing an extra "POST" positional argument to _async_create_element_from_template, which only takes (url, body) -- confirmed this raises TypeError before any request is sent. 3. classification_explorer's clear_known_duplicate_classification and clear_consolidated_duplicate_classification annotated DeleteClassificationRequestBody but called _async_delete_relationship_request, whose validator only accepts DeleteRelationshipRequestBody or dict -- passing the annotated type hits the validator's else branch, which returns None, so the guarded call is skipped and the clear silently never happens. Switched both to the dedicated _async_delete_classification_request helper (the other 12 clear_* methods in this file already used it correctly). Verified: full pyegeria/ compiles, all touched modules import, zero _async_create_element_from_template call sites now exceed its 2-param arity, micro-tests pass, functional tests collect. Audit: 612 OK / 47 mismatch (was 605/54 on this base) -- governance-officer and the six from-template methods now report 0 mismatches; classification-explorer down to the 6 unrelated findings that predate this pass. Signed-off-by: Dan Wolfson <dan.wolfson@pdr-associates.com>
…ix broken get_metadata_element_history
metadata-expert reported 16 mismatches; 14 were a new variant of the tool
artifact already fixed for connection-maker/governance-officer.
Tooling (scripts/omvs_audit.py):
- Resolve module-level helper functions that just return the service root.
metadata_expert.py (and governance_officer.py, solution_architect.py) builds
URLs as f"{base_path(self, self.view_server)}/..." -- a plain function call,
not a self.<attr> reference, so neither the by-name nor by-value root
detection matched it. Added resolve_module_root_funcs(): any top-level
function whose single return statement contains "/open-metadata/" is
registered by name, and _flatten's Call handling checks that registry before
falling through to the URL-wrapper/opaque-expression cases. The function's
own arguments are irrelevant (they just interpolate platform_url/view_server,
already collapsed to placeholders), so this resolves by name, not by call
shape.
Endpoint fixes:
- get_metadata_element_history routed through _async_get_guid_request, whose
default validates against GetRequestBody -- a Literal['GetRequestBody']
discriminator on the "class" field. The method hand-builds a body with
"class": "HistoryRequestBody" (matching Egeria-api-metadata-expert.http;
no HistoryRequestBody model exists in pyegeria.models). Every call raised a
pydantic ValidationError before any request was sent. Rewritten to send the
raw dict via _async_make_request directly, mirroring
_async_get_classification_history -- the sibling endpoint in this same file
that already does this correctly.
- get_metadata_guid_by_unique_name annotated body as dict | FilterRequestBody,
but actually sends "class": "UniqueNameRequestBody" (matching ground truth;
again no backing pyegeria.models class exists). A caller following the
annotation and passing an actual FilterRequestBody object would crash in
body_slimmer(), whose body.items() call assumes a dict. Corrected the
annotation to Optional[dict] with a comment, rather than importing a class
that doesn't exist.
Audit now reports 628 OK / 31 mismatch (was 612 / 47).
Not fixed here (missing, not mismatch -- different bucket, deferred):
getAllRelatedMetadataElements and findRelationshipsBetweenMetadataElements
have no SDK method at all.
Signed-off-by: Dan Wolfson <dan.wolfson@pdr-associates.com>
…l, detach helper direction
actor-manager reported 7 mismatches; 4 were real, fixed; 2 are a .http
ground-truth typo (see below); 1 is a stray .http parse artifact.
Fixes:
- add_security_group_membership / update_security_group_membership built their
URL as .../security-group-membership/classify|reclassify (singular).
Egeria-api-actor-manager.http documents .../security-group-memberships/
(plural) consistently across classify, reclassify, and declassify;
remove_all_security_group_memberships in this same file already used the
plural form correctly. Also dropped a stray "url = url = (...)" double
assignment in add_security_group_membership while touching that line.
- detach_asset_from_profile / detach_person_role_from_profile annotated their
body as DeleteRelationshipRequestBody (correct, matches ground truth) but
called _async_delete_element_request -- the inverse of the bug class fixed
earlier in governance_officer/classification_explorer. Its validator treats
a DeleteRelationshipRequestBody instance as "not provided" (isinstance check
against DeleteElementRequestBody fails, falls to the else branch), silently
discarding the caller's relationship-detach properties and sending a default
DeleteElementRequestBody body instead. Switched both to
_async_delete_relationship_request to match the annotation and the ground
truth's /detach path shape.
Not changed -- ground-truth data-quality issues, not SDK bugs:
- updateActorRole / deleteActorRole: Egeria-api-actor-manager.http's URLs for
these two are missing the {{actorRoleGUID}} path segment entirely
(.../actor-roles/update, .../actor-roles/delete) while every sibling type in
the same file (actor-profiles, user-identities) correctly uses
.../{guid}/update and .../{guid}/delete. The SDK's {guid}/update /
{guid}/delete shape is the one consistent with the rest of the API and is
presumed correct; changing it to match would introduce a real bug to
satisfy a typo in the ground truth.
- getActorRoleByGUID: the .http line for this endpoint ends in a stray
`"})` copy/paste artifact (`.../retrieve"})`), which canon_path's cleanup
can't fully strip without risking corruption of legitimately
parameter-terminated paths elsewhere. Cosmetic parser noise, not a real
finding.
Audit now reports 631 OK / 27 mismatch (was 628 / 31).
Signed-off-by: Dan Wolfson <dan.wolfson@pdr-associates.com>
…path segment Six methods (get_semantic_assignees, get_source_elements, get_elements_sourced_from, get_licensed_elements, get_licenses, get_certified_elements) built URLs as .../classification-explorer/glossaries/elements/... . Every one of these operates on a generic Referenceable element (source/license/certification relationships), not a glossary term, and Egeria-api-classification-explorer.http confirms none of them include "glossaries" in the path -- only get_meanings (glossaries/terms/...) actually does, and was correctly left untouched. Audit now reports 0 mismatches for classification-explorer (was 6). Signed-off-by: Dan Wolfson <dan.wolfson@pdr-associates.com>
…ffix, resolve query-string helper
solution-architect reported 5 mismatches: 1 real duplicate-name bug, 3 tool
artifacts from a second variant of the base_path()-style false positive, 1
left flagged rather than force-fixed.
Tooling (scripts/omvs_audit.py):
- _flatten's ast.Name branch never consulted `roots` at all -- only the
Attribute and Call branches did. A bare local variable referenced in an
f-string (`f"{url}{possible_query_params}"`) always rendered as an opaque
placeholder no matter what it held. Now checks `roots` first.
- Added QUERY_STRING_FUNCS + per-function local-variable resolution: methods
assign a `query_string(...)`-built value to a local (e.g.
`possible_query_params = query_string([("startFrom", ...), ...])`) before
interpolating it into the URL. query_string() always returns "" or a
"?key=value&..." suffix (solution_architect.py / collection_manager.py),
safe to elide entirely since canon_path already splits on "?". Without this,
every by-name/paginated endpoint using this pattern reported a false PATH
mismatch (get_solution_roles_by_name, get_solution_components_by_name,
get_solution_component_implementations all cleared by this alone).
Endpoint fix:
- get_design_patterns_by_name appended the search name directly onto the URL
path (.../design-patterns/by-name/{name}) *and* sent it again in the request
body via _async_get_name_request's FilterRequestBody -- the name was
effectively sent twice, and the URL didn't match
Egeria-api-solution-architect.http's undecorated .../by-name path. Removed
the URL segment; the body-carried filter is Egeria's actual query mechanism
for this endpoint (confirmed against ground truth).
Left flagged, not changed:
- detach_solution_linking_wire(component1_guid, component2_guid) builds
.../wired-to/{}/detach, a path Egeria-api-solution-architect.http does not
document at all -- only .../wires/{relationshipGUID}/detach exists, which
detach_solution_linking_wire_by_guid (a sibling method, already correct)
already implements. The two-component-GUID method has no relationship GUID
to build the documented URL from, so this isn't a mechanical path fix;
flagging for a follow-up decision (deprecate in favor of the _by_guid
sibling, or confirm live whether the two-GUID form has a real backing
endpoint under another path).
Audit now reports 642 OK / 17 mismatch (was 638 / 21).
Signed-off-by: Dan Wolfson <dan.wolfson@pdr-associates.com>
…nteed AttributeError on every call
schema_maker.delete_schema_type, schema_maker.delete_schema_attribute,
subject_area.delete_subject_area, and time_keeper.delete_context_event all
called self._async_delete_element_body_request(url, body). That method does
not exist anywhere in the codebase -- not on ServerClient, not on any base
class. Every call to any of these four methods raised AttributeError before
any request was sent. Found while investigating a "BODY sends
DeleteElementRequestBody != MetadataSourceRequestBody" audit finding for
delete_schema_type; the audit's body-class check doesn't (and can't) detect a
call to a name that isn't defined at all, since it only compares declared
types, so this was more broken than the audit reported.
Fix depends on what each method's ground truth (.http file) actually
documents as its request body:
- schema_maker.delete_schema_type / delete_schema_attribute: Egeria documents
"class": "MetadataSourceRequestBody" for both (confirmed against
Egeria-api-schema-maker.http). Routed to the existing, correct
_async_metadata_source_body_request helper (validates MetadataSourceRequestBody
via _metadata_source_request_adapter) and corrected the misleading
DeleteElementRequestBody annotations on all 4 signatures (both async/sync
pairs) to match.
- subject_area.delete_subject_area / time_keeper.delete_context_event: Egeria
documents "class": "DeleteElementRequestBody" for both (confirmed against
Egeria-api-subject-area.http / Egeria-api-time-keeper.http) -- these two
already had the correct annotation, just the wrong helper name. Routed to
the existing _async_delete_element_request helper instead.
Verified: full pyegeria/ compiles, all four touched modules import, zero
remaining references to the nonexistent helper name anywhere in the tree,
micro-tests pass, functional tests collect. Audit: schema-maker now 11 OK / 3
mismatch (was 10 / 4) -- the 3 remaining are Egeria-api-schema-maker.http
itself missing the {{schemaAttributeGUID}} path segment on
updateSchemaAttribute/deleteSchemaAttribute (every sibling type in the same
file correctly includes it) plus one stray '"}' copy-paste artifact on
getSchemaAttributeByGUID's line -- ground-truth data-quality issues, not SDK
bugs, left unchanged.
Signed-off-by: Dan Wolfson <dan.wolfson@pdr-associates.com>
…the wrong body class remove_is_abstract_concept, remove_is_context_definition, and remove_activity_description all called _async_delete_classification_request with a DeleteClassificationRequestBody annotation, but Egeria-api-glossary-manager.http documents different classes per endpoint: - clearTermAsAbstractConcept -> DeleteElementRequestBody - clearTermAsActivity -> DeleteRelationshipRequestBody - clearTermAsContext -> DeleteRelationshipRequestBody Egeria's own API is inconsistent here -- other clear_* siblings in this same file (glossary_as_taxonomy, is_data_value, term_as_question, is_prime_word, is_modifier, is_class_word) genuinely do use DeleteClassificationRequestBody and were correctly left untouched; the audit didn't flag them because they already match ground truth. Fixed the 3 that don't, matching each to its own documented class and helper (_async_delete_element_request / _async_delete_relationship_request), both async and sync signatures. Audit now reports glossary-manager at 0 mismatches (was 3). Overall: 646 OK / 13 mismatch. Signed-off-by: Dan Wolfson <dan.wolfson@pdr-associates.com>
…audit resolver picks up local URL-root variables
Fixed the remaining audit mismatches down to 7 (were 13), all confirmed
against ground truth individually -- this batch skews toward more severe,
functionally-wrong-operation bugs than earlier ones.
Endpoint fixes:
- project_manager.clear_project_classification called
_async_new_classification_request (the ADD-classification helper) instead
of _async_delete_classification_request, despite its own URL
(.../declassify) and docstring both documenting a remove operation.
Calling "clear" would have added a ProjectClassification instead of
removing one -- the opposite of what the method claims to do.
- valid_metadata.link_specification_property called
_async_create_element_body_request (validates NewElementRequestBody)
instead of _async_new_relationship_request (NewRelationshipRequestBody,
confirmed via Egeria-api-valid-metadata.http) for an /attach relationship
endpoint.
- my_profile.get_my_profile hand-rolled a POST to a GET-only, bodyless
endpoint (Egeria-api-my-profile.http: plain GET, no request body -- the
profile is derived from the bearer token). Every call would have hit the
wrong HTTP method.
- actor_manager.delete_actor_role annotated + called the DeleteElementRequestBody
path, but both ground truth and the method's own docstring sample document
DeleteRelationshipRequestBody. Switched to _async_delete_relationship_request.
- data_designer.detach_specialized_data_value_specification used
".../specialized-data-value-specification-definition/..." (singular,
"-definition" suffix); its attach sibling in the same file already uses the
correct ".../specialized-data-value-specifications/..." (plural).
- lineage_linker.link_lineage used a made-up
".../elements/{}/{}/{}/attach" path with no model validation at all
(raw dict passed straight to _async_make_request). A correct sibling,
link_data_flow, already builds the real
".../from-elements/{}/via/{}/to-elements/{}/attach" shape -- matched it and
routed through _async_new_relationship_request instead of an unvalidated
raw POST.
Tooling (scripts/omvs_audit.py):
- Generalized the per-function local-variable root resolution (added for
query_string() in the previous commit) to also catch a plain local
assignment building a URL prefix inside a method body, e.g.
`base = f"{self.platform_url}/servers/.../api/open-metadata/<service>"`
(collection_manager.py). resolve_roots only sees self.<attr> assignments
in __init__; this covers the same pattern scoped to a local variable.
Cleared 1 more false mismatch (detach_associated_skill_set).
Not changed -- flagged as ambiguous, not mechanically fixable:
- detachSolutionLinkingWire (MISMATCH) and detachAllSolutionLinkingWire
(MISSING) are two distinct, correctly-implemented Egeria endpoints
(detach_solution_linking_wire and detach_solution_linking_wire_by_guid)
whose ground-truth names are too similar for the audit's exact-name
matching to disambiguate -- adding a NAME_OVERRIDES entry would just move
the false mismatch from one ground-truth row to the other rather than
resolve it. Both underlying SDK methods are already correct; this is a
naming-convention decision for a human, not a bug.
- updateActorRole/deleteActorRole and updateSchemaAttribute/
deleteSchemaAttribute PATH findings, and 2 stray '"}' artifacts (still
ground-truth data-quality issues identified in earlier commits, unchanged).
Verified: full pyegeria/ compiles, all touched modules import, micro-tests
pass, functional tests collect. Audit: 652 OK / 7 mismatch (was 646 / 13).
Signed-off-by: Dan Wolfson <dan.wolfson@pdr-associates.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Continuation of the OMVS
.http-ground-truth audit from PR #277. Fixes threeoriginally-uncommitted pieces lost to worktree cleanup, plus a full pass
through the remaining audit mismatches (47 -> 7, all 7 confirmed ground-truth
data-quality issues, not SDK bugs).
Most notable fixes
schema_maker.delete_schema_type,delete_schema_attribute,subject_area.delete_subject_area, andtime_keeper.delete_context_eventall called_async_delete_element_body_request-- a method that does not existanywhere in the codebase. Every call raised
AttributeErrorbefore anyrequest was sent.
project_manager.clear_project_classificationcalled the add-classification helper instead of the delete one --
calling "clear" would have added a classification, not removed one.
my_profile.get_my_profilesent POST to adocumented GET-only, bodyless endpoint.
discards the caller's properties and substitutes a default body of the
wrong
"class"(governance_officer, glossary_manager, actor_manager,valid_metadata).
scripts/omvs_audit.pyhardening: 5 resolver gaps fixed (module-levelbase_path()helpers, local URL-root variables, query-string builders,name-vs-value root detection, union body-type comparison), each verified
against the specific bug it was masking, not just that it goes quiet.
Verification
pyegeria/compiles cleanpytest tests/micro-tests/passespytest tests/functional-tests/ --collect-onlycollects cleanlyscripts/omvs_audit.py: 652 OK / 7 mismatch (was 612 OK / 47 mismatch atthe start of this branch)
Commits
8 commits, each independently verified and scoped to one investigation. See
individual commit messages for the ground-truth citation behind each fix.
🤖 Generated with Claude Code