feat: add query_semantic_graph over the semantic graph views - #48
feat: add query_semantic_graph over the semantic graph views#48killme2008 wants to merge 9 commits into
Conversation
|
All six fixed and verified against 1.3. Both criticals reproduced first. 1. Identifier decoding. Confirmed: 2. Window offset. Reproduced exactly as you measured — edge at 07:05Z, window Bounds now carry 3. next_query. Now carries the window. The integration test was indeed pinning the broken shape — it now runs the suggested query and asserts the retry succeeds, rather than asserting its literal contents. 4. Capability/SQL mismatch. Took the simpler branch you suggested: 1.3 is the floor, so every column actually read is required, and the SQL is fixed instead of capability-shaped. That removes all three sub-issues at once. Worth noting my 5. Input schema. "view": {"enum": ["summary","entities","relationships"], "description": "...", "type": "string"}
"limit": {"minimum": 1, "maximum": 500, "default": 100, "description": "..."}Which filters belong to which view is now in each filter's description rather than only in a runtime error. 6. Description. Rewritten against the shipped docs, not memory. Endpoint pairs. Fixed — Divergence from the bench-validated tool, stated explicitlyYou are right that this shape is not the one the experiments validated, and the PR should say so rather than let the cited results imply coverage they do not have. What differs:
If the graph tool is measured, it should be re-validated in the harness rather than inheriting the earlier numbers. Verification
|
0539264 to
5cfddb7
Compare
5de036e to
5b300c6
Compare
d47dfa7 to
9603d20
Compare
One tool with three views. `summary` reports the entity types, the relationship types, and the endpoint types each relationship connects, so a caller learns the graph's shape without paging through edges to find out -- reading an unfiltered dump is both a round trip and an invitation to read the result as a service call graph. Only `calls` edges carry request, error, and duration counts, so an unfiltered result is ordered by relationship type and endpoint. Ordering a mixed result by RED would sort every other relationship type below a NULL. Passing rel_type=calls opts into the RED ordering. The window is required and half-open over `observed_at`, and the result echoes the window it used. GreptimeDB rejects an unbounded lower bound and otherwise defaults to the last hour, which silently answers a different question than the caller asked. A zero result keeps `items: []` and adds `status`, `applied_filters` and guidance whose next query drops the identifier but keeps the type filters, since an unfiltered read of a large graph is the failure this tool avoids. The views only exist in GreptimeDB 1.3. Registration happens at import, before a connection exists, so lifespan probes -- both views present, carrying the columns read here, and readable by this account -- and withdraws the tool when the answer is conclusive. An unreachable database is not conclusive and leaves the tool in place. Integration coverage seeds declared edges rather than OTLP traces, which gives the graph one RED-bearing relationship type and one without.
Two defects that produced wrong results. Every returned string was run through `json.loads`, so an identifier that looked like a JSON scalar changed type: entity_id "123" came back as the number 123, "true" as a boolean, and "null" as nothing at all. A caller cannot feed those back in, and "null" is unrecoverable. Only the four JSON columns are decoded now. Window bounds were formatted without their offset, so the database read them in the session time zone. Measured on 1.3 with an edge at 07:05Z and a window of [07:00Z, 07:10Z): one row under a +00:00 session, none under +08:00. Since this server takes --timezone, that silently answered a different question. Bounds now carry +00:00, and an integration test runs the suite's window through an Asia/Shanghai session. Four contract defects: - `next_query` in a no-match result omitted the required window, so following it failed on a missing argument. It now carries the window, and the integration test runs the suggested query instead of pinning its shape. - Capability checks disagreed with the SQL: `scope` was selected and filterable without being required, an absent RED column produced `MAX(confidence), , MIN(...)`, and rel_type=calls ordered by columns the probe allowed to be missing. GreptimeDB 1.3 is the floor, so every column read is now required and the SQL is fixed rather than capability-shaped. - The input schema carried no argument semantics: the SDK drops a bare string in `Annotated`. view is a Literal, so it reaches the schema as an enum, and the filters and limit use pydantic Field with descriptions and bounds. - The description misstated the contract. observed_at is the observation bucket, not the client side of a call; confidence 1.0 also covers declared edges; an unpaired client span still produces a virtual-node edge, so the trace-context claim was too strong; unmatched_count was undocumented despite being what separates a callee that stopped answering from a caller that stopped asking. summary now reports endpoint pairs. Two sets lose which combinations exist: service->pod and pod->node read as service->node. Entities also return descriptive and freshness.
9603d20 to
58a3535
Compare
…im the description Three review findings and five API notes. The graph tool serialized rows directly, so `mask_enabled` and `mask_patterns` did not apply to it: an attribute map could publish what the same value would have had masked through execute_sql. Attribute values whose name matches a sensitive pattern are masked now, in `entity_id_attrs` and `descriptive`, and `entity_id` with them when it was assembled from one -- it is those values joined, so leaving it would undo the masking. `MAX(confidence)` flattened two different measurements. The database reports 1.0 for a bucket whose spans paired and 0.5 for one that saw only clients, and `real_wins` switches request_count, error_count and the durations to whichever population that bucket describes: a pair timed by the server span, an unmatched client by its own. Summing across both added those populations together and the maximum hid that it had happened. `confidence` is in the group key now, so such an edge returns one row per population. The description asserted that request_count counts paired calls, which is only true of a bucket that paired; where none did, the database reports the unmatched clients there. It also read a fall in those counts as the callee having stopped answering or the caller having stopped asking, which sampling and missing instrumentation produce equally. Both are gone, durations are stated in seconds, and the entities view now says it returns one row per set of attributes so item_count is not an entity count, and that first_seen and last_seen bound the observation inside the window rather than the entity's lifetime. `Start with view=summary` became a condition rather than an order, and the RCA guidance is out. A truncated result now carries the effective limit and which filters would narrow it, since there is no cursor and an unfiltered result ordered by type can drop later types entirely. A failed query raises instead of returning prose, so `status=no_match` only ever means the query ran and matched nothing. Not covered: an edge observed at both confidences end to end. Declared edges cannot express it -- the computed view keeps one revision per identity, so a second row for the same edge replaces the first whatever its scope or bucket. That needs trace-derived pairing, which the integration suite does not produce.
…ide them The previous change masked the names inside attribute maps and stopped there, so a returned field was never checked against the patterns itself: `mask_patterns=descriptive` left `descriptive` fully readable, and a pattern naming `entity_id`, `src_id` or `dst_id` did nothing. A field whose own name matches is hidden first now, exactly as the column would be through execute_sql; maps that survive that are then masked by the names inside them. Hiding `entity_id` when a masked attribute helped build it does not extend to `relationships`: its view carries no attribute names, so the same value can still appear there as `src_id` or `dst_id` unless a pattern names those columns. The previous message implied those values were hidden everywhere. They are not, and the tool description now says so. The truncation hint claimed ordering by type and endpoint, which is wrong under rel_type=calls; it states that later rows are missing and lists what to narrow by.
Whether to hide `entity_id` was read back off the already-masked row, so a pattern that hid the whole attribute map left nothing that looked like one and the id went out in the clear. Configuring `mask_patterns=entity_id_attrs` therefore re-exposed a value that the default rules had hidden: adding a rule made masking weaker. The decision is taken from the original row now, and a map hidden in full also hides the id built from it. A regression test asserts the property directly -- across pattern sets, the id stays hidden and the sensitive value never appears in the output.
`GraphCapability` carried the column set of each view, which nothing read once the queries stopped being capability-shaped; the probe validates the columns and reports availability. `summary` returned entity_count and relationship_count, both sums of the arrays beside them, and a `complete` that was always true. Every response repeated `bounds`, `time_field` and `observation_bucket_seconds`, which are facts about the tool rather than about the result and are already in its description. Tests lost four duplicates: two that drove the same incompatible-schema branch with a different column, two inputs of one decoding rule, a separate assertion that the no-match retry runs, and two spellings of "the window parses to UTC".
…time A probe that could not run was reported as a result. If startup left the capability undecided and the retry inside a call then failed to read, the tool returned status=unavailable with reason=error and the audit recorded a success, while the identical connection failure one step later, in the query itself, raised. It raises now; only a conclusive answer -- the view being absent, rejected, or the wrong shape -- is still an answer. The startup probe also had no time bound. It runs before the server can serve anything, over a connection whose driver defaults leave both timeouts unset, so a database that accepted the connection and then went quiet would hold initialisation open and take every other tool with it. Wrapping the thread would not have helped, since the blocking read would continue behind it; the probe opens its own connection with connection_timeout and read_timeout instead. Measured against a socket that accepts and never answers: startup completes in 5.0s and keeps the tool, the timeout being inconclusive. Both are driven from the tool entry point in the tests, not from _classify.
Five seconds is tight for a cold graph derivation on a busy instance, and the cost of waiting longer is only paid when the database has stopped answering.
There was a problem hiding this comment.
🟡 Changes recommended
There are documented response-shape/status classification mismatches (especially for summary and non-available capability cases) that can break clients relying on the PR’s stated contract.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR adds a new MCP tool, query_semantic_graph, to query GreptimeDB 1.3’s semantic graph views (greptime_private.semantic_entities / semantic_relationships) over a required half-open time window and returns results in summary, entities, and relationships views, with unit + integration coverage.
Changes:
- Introduces
greptimedb_mcp_server.graphimplementing window parsing, capability probing/negotiation, masking, and query execution for graph entities/relationships. - Registers
query_semantic_graphin the server, including startup probing to withdraw the tool when the graph is conclusively unavailable. - Adds unit and integration tests (including seeding declared edges) and documents the new tool in the README.
File summaries
| File | Description |
|---|---|
| tests/test_graph.py | Adds unit tests for window parsing, ordering, masking, probe classification, and response-guidance behaviors. |
| tests/integration/test_e2e.py | Adds end-to-end integration coverage for graph tool availability, window semantics, ordering, and no-match guidance. |
| tests/integration/conftest.py | Seeds declared graph edges for integration tests and tracks whether the graph is present/seeded. |
| src/greptimedb_mcp_server/server.py | Registers query_semantic_graph tool and performs startup-time capability probing to withdraw the tool when unusable. |
| src/greptimedb_mcp_server/graph.py | Implements graph probing, time-window parsing, SQL queries, ordering, masking, and result envelope construction. |
| README.md | Documents the new query_semantic_graph tool and its GreptimeDB 1.3 dependency and window semantics. |
Review details
Suppressed comments (1)
tests/integration/conftest.py:162
- _seed_declared_edges() claims it returns False only when the graph does not exist, but it catches any mysql.connector.Error (including permission/validation failures). This docstring should reflect that it returns False for any insert failure, not just a missing table.
def _seed_declared_edges(cursor) -> bool:
"""Insert declared edges, reporting False when the graph does not exist."""
columns = (
- Files reviewed: 6/6 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…ardown `summary` carried no `status` when it succeeded but was given one when the graph could not be read, and that unavailable response dropped `window`, `applied_filters` and `limit` from the item views. Each view now answers with its own shape whether the read happened or not: `view`, `status` and `window` always, plus the empty type lists for summary or the empty item envelope for the others. The integration teardown deleted seeded edges by src_id and dst_id alone. `semantic_relationships_declared` is shared and this suite does not create it, so the delete now matches the endpoint types, the relationship type and the provenance as well.
Adds
query_semantic_graph, which readsgreptime_private.semantic_entitiesandgreptime_private.semantic_relationships.What it does
One tool with three views:
summary,entities,relationships.The time window is required. It is half-open,
[start_time, end_time), overobserved_at, which is the 60-second bucket an observation was recorded in. Timestamps are RFC3339; one without an offset is read as UTC. Rows are aggregated across the buckets in the window, and the result echoes back the window and the limit it used.The views only exist in GreptimeDB 1.3. At startup the server checks that both are present, carry the columns it reads, and can be read by the connected account, and drops the tool from the list when the answer is conclusive. On 1.2 it is not offered and the reason is logged. The check uses its own connection with a 10 second timeout, since it runs before the server can serve anything.
Output
Captured on 1.3. One table declares
serviceandk8s.podentities. Four declared edges:frontend -> checkoutandcheckout -> paymentascalls,checkout -> pod-aandpod-a -> node-1asruns_on.view="summary"{ "view": "summary", "window": {"start": "2026-09-09T10:25:42+00:00", "end": "2026-09-09T11:25:42+00:00"}, "entity_types": [ {"type": "k8s.pod", "count": 1}, {"type": "service", "count": 1} ], "relationship_types": [ {"type": "calls", "count": 2, "endpoints": [{"source": "service", "destination": "service", "count": 2}]}, {"type": "runs_on", "count": 2, "endpoints": [{"source": "k8s.pod", "destination": "k8s.node", "count": 1}, {"source": "service", "destination": "k8s.pod", "count": 1}]} ] }Endpoint types come back as pairs. Reduced to a set of sources and a set of destinations,
service -> k8s.podandk8s.pod -> k8s.nodewould read asservice -> k8s.node.view="relationships"andview="entities"returnitemsalongsidestatus,applied_filters,limit,item_countandcomplete:{"view": "relationships", "status": "ok", "applied_filters": {}, "limit": 100, "item_count": 4, "complete": true}A relationship item:
{ "src_type": "service", "src_id": "checkout", "dst_type": "service", "dst_id": "payment", "rel_type": "calls", "provenance": "declared", "confidence": 1.0, "request_count": 100, "unmatched_count": null, "error_count": 45, "duration_sum": 12.5, "duration_count": 100, "duration_max": null, "first_seen": "2026-09-09 10:55:29.902000", "last_seen": "2026-09-09 10:55:29.902000", "fresh_until": "2026-09-09 11:25:42" }An entity item:
{ "entity_type": "service", "entity_id": "checkout", "entity_id_attrs": {"service_name": "checkout"}, "scope": "prod", "descriptive": null, "source_tables": ["public.svc_latency"], "first_seen": "2026-09-09 10:55:00", "last_seen": "2026-09-09 10:55:00", "fresh_until": "2026-09-09 10:56:00" }Ordering, same window:
view="relationships"calls checkout->payment,calls frontend->checkout,runs_on pod-a->node-1,runs_on checkout->pod-aview="relationships", rel_type="calls"checkout->payment(45 errors),frontend->checkout(7 errors)Other cases:
rel_type="calls", src_id="checkout-svc"status: "no_match",items: [], andguidance.next_querywith the id dropped and the type filter and window keptlimitcomplete: falseplus guidance listing the filters that would narrow itview="paths"ToolError: Invalid view: paths. Must be one of: summary, entities, relationshipsview="entities", rel_type="calls"ToolError: Filter rel_type does not apply to view=entities. Available: entity_type, entity_id, scopestart_time="yesterday"ToolError: Invalid start_time: yesterday. Use an RFC3339 timestamp such as 2026-09-05T07:00:00ZToolErrorThings to check when reviewing
One row per edge per confidence. The database reports confidence 1.0 for a bucket whose client and server spans paired and 0.5 for one where only the client was seen, and it switches
request_count,error_countand the durations to whichever population that bucket describes — a pair is timed by the server span, an unmatched client by its own. Summing across both would add two different measurements, soconfidenceis in the group key. An edge seen both ways returns two rows.request_countis not always a count of paired calls. Where a bucket paired nothing, the database reports the unmatched clients there instead.unmatched_countis the client spans with no paired server span. Durations are in seconds.Window bounds carry their offset. Without one the database reads them in the session time zone, and this server takes
--timezone. On 1.3, an edge at 07:05Z queried over[07:00Z, 07:10Z)returned one row under a+00:00session and none under+08:00.Only the four JSON columns are decoded. Decoding every string turned
entity_id"123"into a number and"null"into nothing.Masking. A returned field whose own name matches a pattern is hidden, and attribute maps are then masked by the names inside them.
entitiesalso hidesentity_idwhen a masked attribute helped build it.relationshipscannot do that — its view carries no attribute names — so the same value can still appear there assrc_idordst_idunless a pattern names those columns.entitiesreturns one row per set of attributes, so an entity whose attributes changed inside the window appears more than once anditem_countis not an entity count.first_seenandlast_seenbound the observation inside the window, not the entity's lifetime.Not included.
attributesis left off the aggregated relationship rows: it varies per observation, so grouping by it splits an edge and breaks the totals. There is no path traversal; multi-hop is still a self-join throughexecute_sql. There is no cursor for paging pastlimit.Not covered by tests. An edge observed at both confidences, end to end. Declared edges cannot express it, because the computed view keeps one revision per identity, so a second row for the same edge replaces the first whatever its scope or bucket. That needs trace-derived pairing, which the integration suite does not produce.
This shape is not the one the earlier experiments measured:
summaryis new andbucket=windowwas dropped. It should be re-validated rather than inheriting those results.Verification
255 unit tests. Integration against freshly started instances: 26 passed on 1.3.0-alpha.1, and 21 passed with 5 skipped on 1.2.0-beta.1, where the assertion that the tool is absent runs on both.