Feat/polymorphic nodes - #108
Merged
Merged
Conversation
/resources is the generic node query - it spans assets, timeseries, functions, resources, data sets and policies - but every row arrived coerced into a flat Resource. A timeseries read this way had no unit, a data set no policies, and both carried an isRoot flag meaningless for their type. The api's node-reads/* and node-create/* branches fixed the server half; this is the SDK half. `Node` (src/nodes.rs) is an enum over Asset, TimeSeries, Function, Resource, Dataset and Policy. filter/search/get_by_id return DataWrapper<Node>, by_ids/create and EdgesService::by_ids return GraphDataWrapper<Node>, and ResourceNetwork::nodes is Vec<Node>. The discriminator is a label, not a field: there is no nodeType key on the wire, so `Node` hand-writes Deserialize - buffer, canonicalize each label the way the api's TextValidator does, dispatch. No type-label means RESOURCE; more than one is an error, as it is server-side. Serializing emits the variant's own shape and appends its type-label if absent, and never strips a conflicting one: a body labelled both ASSET and POLICY earns the api's 400 naming both, which is the accurate answer. Also here, because the typed shapes need them: - TimeSeries gains labels and table_engine, Function gains description, source and data_set_id, Dataset gains labels, source, related_resources and data_set_id. Each is a field the api already sends and the SDK silently dropped. - Dataset timestamps move from FixedOffset to Utc, matching every other DTO, and connectedDataSets now reads the JSON strings the api actually sends rather than failing on them - latent only because reads never populate it. - Python maps each variant to its own pyclass, so isinstance works and an object from resources.filter() behaves like one from timeseries.by_ids(). New Asset and Policy classes; node_type on every node class for data-driven dispatch. The dispatch is a hand-written IntoPyObject on a non-pyclass PyNode wrapper, the first such impl in the bindings. - Fixes a panic in the Python bindings' resources.by_ids on an empty graph response. update is deliberately untouched: the api still echoes every node as a flat Resource there, so ResourceService::update keeps returning GraphDataWrapper<Resource>. That asymmetry is the api's NODE_UPDATE_REFACTOR to remove. Gated on the api. The ten branches this consumes are not merged, so against a backend built from master the typed reads fail on a missing valueType - seven python tests, all one cause. Do not publish 0.3.0 until they land. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Running the suites against a backend carrying the api's node-reads and node-create branches turned up three things the earlier gated run could not reach: - `Asset` and `Policy` had `neighbors` but not `related_events`, so a node created with the ASSET label silently lost a method it had as a `Resource` (test_navigation caught it as an AttributeError). Both are now generated by the same macro as `neighbors`, which is why they diverged in the first place: the four older node classes each spell these out by hand, and writing them a fifth and sixth time is what dropped one. - A graph-sourced timeseries reports `securityCategories: []`, not an absent field - `Timeseries` has no NON_NULL include, so the api emits its DTO default. The assertion and both doc claims said "absent". The sharper statement, now recorded: on a graph read *nothing* is missing, so a caller cannot tell a default from a real value and must re-read the node flatly. - test_polymorphic_nodes built a plain resource with no labels at all; the api requires at least one. A plain resource is the node carrying no *type* label, not one carrying none. Its graph-sparseness test also skipped when the projection lagged, which let the assertion quietly never run - it polls now. Rust: 201 passed, 1 failed (mcp_event_update_by_uuid_reindexes_the_external_id, red on purpose). Python: 582 passed, 7 failed, none of them new - four multi-tenant and three errors are 403 "unknown organization" from a deployment without those tenants, two are datapoint fixtures with no data, and one is a pre-existing brittle unfiltered event query that only became reachable now that its fixture builds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Running the suites against the upgraded api found a real defect: any `fetch_related`/`fetch_nearest` traversal reaching a timeseries failed with `missing field valueType`. A graph read carries the shared node fields and nothing else - ['createdTime','dataSetId','externalId','id','labels','lastUpdatedTime', 'metadata','name','relatedResources'] - so no unit, unitExternalId, valueType, tableEngine or securityCategories. They are absent from the payload, not defaulted, which is what the earlier doc claims got wrong. That invalidates the premise this field was left required on. The reasoning was "the api always sends valueType, so gate the SDK on the backend rather than weaken the type". True of the flat reads, false of the graph ones - and a required field there is not a stricter contract, it is a hard error on a call that has nothing to do with value types. `Option<String>` also puts it where its siblings already are: unit, table_engine and security_categories are all optional and all None on this path. Rejected `#[serde(default)]`: it would yield "" for a series whose real type is BIGINT, so `if ts.value_type == "float"` answers wrong in silence. None means "not told", which a caller can act on. Corrects three places that repeated the wrong claim (the nodes module doc, AGENTS.md, and the neo4j field test, which asserted securityCategories was an empty list rather than absent), and pins the real shape in `a_graph_sourced_timeseries_has_none_of_its_type_specific_fields`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
olavgg
approved these changes
Sep 2, 2026
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
2 participants
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.
No description provided.