From 99a1d25001e2c962e96f7494ff8b882899ba3f48 Mon Sep 17 00:00:00 2001 From: Khemchand Joshi Date: Wed, 8 Jul 2026 11:13:18 +0530 Subject: [PATCH 01/11] docs(feat-015): graduate FA-003 to spec + accepted design (ckg query) Graduate the read-only graph query surface (FA-003) into feat-015, targeting 0.6.4. Add the feature spec and the accepted design doc; register both in the trackers. The design resolves the caller-facing surface (Cypher-subset text) and the internal trust boundary (validated AST -> per-backend compile). Hardened for extensibility per review: polymorphic Compiler classes + singledispatch (not a dialect flag), capability tiers (not intersection-forever), a shared bounded-cursor making max_expansions real+tested on every backend (not approximated), a capability-driven tool registry, and a reusable cli formatter. Scope: all three backends (Kuzu + Neo4j + SurrealDB) query-capable at ship. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/design/README.md | 1 + .../design-015-read-only-graph-query.md | 506 ++++++++++++++++++ docs/features/TRACKER.md | 1 + .../feat-015-read-only-graph-query.md | 277 ++++++++++ 4 files changed, 785 insertions(+) create mode 100644 docs/design/design-015-read-only-graph-query.md create mode 100644 docs/features/feat-015-read-only-graph-query.md diff --git a/docs/design/README.md b/docs/design/README.md index a99f559..8030d60 100644 --- a/docs/design/README.md +++ b/docs/design/README.md @@ -31,5 +31,6 @@ history. Supersede with a new doc; never delete. |---|---|---| | [design-001-core-contracts-module](design-001-core-contracts-module.md) | feat-001 | accepted | | [design-009-temporal-evolution-layer](design-009-temporal-evolution-layer.md) | feat-009 | accepted | +| [design-015-read-only-graph-query](design-015-read-only-graph-query.md) | feat-015 | accepted | _(Add a row per feature as its design is written.)_ diff --git a/docs/design/design-015-read-only-graph-query.md b/docs/design/design-015-read-only-graph-query.md new file mode 100644 index 0000000..d164be9 --- /dev/null +++ b/docs/design/design-015-read-only-graph-query.md @@ -0,0 +1,506 @@ +# design-015: Read-only graph query surface (`ckg query --graph` / `ckg_query`) + +Mirrors [feat-015](../features/feat-015-read-only-graph-query.md). The *how*: +file layout, exact types, resolved decisions, chunk plan. + +| Field | Value | +|---|---| +| **Status** | accepted | +| **Target** | 0.6.4 | +| **Backend scope** | **all three** query-capable at ship — Kuzu + Neo4j (shared Cypher compiler) + SurrealDB (SurrealQL compiler), full 3-backend conformance | + +--- + +## The one idea + +**Never execute caller text.** Caller writes a Cypher-subset string → we **parse** +it into our own frozen **query AST** → **validate** the AST against the locked +feat-001 vocabulary + exclusion rules → each backend **compiles** the AST to its +native dialect → **execute** under a read-only, bounded session. The AST is the +single trust boundary; everything portable and everything safe hangs off it. + +``` +text ──parse──▶ QueryAst ──validate──▶ QueryAst' ──┬─ compile_cypher(kuzu) ──▶ Kuzu ─┐ + (parser.py) (ast.py) (validator.py) ├─ compile_cypher(neo4j) ──▶ Neo4j ─┼─▶ ResultTable + └─ compile_surreal ──▶ Surreal ┘ {columns, rows, truncated} +``` + +## Guiding constraints + +- **Framework-free** (ADR-0001): the entire query engine lives under + `store.query`, importing only `core` (kinds/models/contracts). No `agentforge` + import. Only the thin `ckg_query` tool wrapper in `serve/tools.py` is framework + layer. +- **The locked ABCs are not touched.** `GraphStore` (feat-001/003) stays as-is; + out-of-tree adapters keep working. Query capability is an **optional protocol** + (`QueryCapable`) an adapter opts into — a backend without it reports + `query.enabled: false` and still serves the typed verbs. +- **Parser is hand-written, no new dependency.** The subset is bounded; a small + tokenizer + recursive-descent parser fits the lean-install ethos (mirrors how + `[watch]`/`[rerank]` keep the base install lean). No `lark`/`pyparsing`. The + accepted grammar is a **versioned EBNF spec** (`store/query/GRAMMAR.md`) that + the parser mirrors production-for-production — the grammar is the source of + truth and the query-language version number tracks it. + +### Extensibility principles (no effort-driven workarounds) + +The feature is deliberately built as **seams, not special-cases**, so the *content* +(more constructs, more backends, richer bounds) grows additively without touching +shared code. The 0.6.4 slice ships a small core, but the shape is extension-ready: + +- **Add a query construct** → new AST node + one parser production + one validator + rule + one `visit_*` method per compiler. No edits to existing branches (traversal + is dispatch-based, §Compilers), so it is O(1) additive, not a shared-code rewrite. +- **Add a backend** → new `QueryCapable` adapter + one `Compiler` subclass + pass + the conformance suite. No `if backend == …` anywhere — dialects are polymorphic + classes, selected by the registry, never by flags. +- **Grow the subset** → a **capability-tier** model (below) lets a stronger backend + advertise more than the common core *without* forking behaviour or breaking the + "identical results" guarantee. The subset is versioned and grows; it is never a + frozen lowest-common-denominator. +- **Bounds are a required contract, not best-effort.** Resource bounding + (`max_rows`, `timeout_ms`, `max_expansions`) is part of the `QueryCapable` + contract and is *proven on every backend* by a runaway-query conformance test. + A backend that cannot bound natively must implement a portable fallback — we do + **not** ship "approximated where the backend doesn't expose it." + +### Capability tiers (how the subset grows without divergence) + +Every supported construct is tagged with a **capability** (e.g. `core.match`, +`core.where`, `agg.basic`, `path.varlen`, `agg.collect`). A backend adapter +declares the set of capabilities it supports: + +```python +class QueryCapable(Protocol): + capabilities: ClassVar[frozenset[str]] # what this backend can execute identically + ... +``` + +- The **core tier** (`core.*` + `agg.basic` + bounded `path.varlen`) is + **mandatory** — every query-capable backend must support it and prove identical + results in conformance. That is the 0.6.4 subset. +- A construct outside a backend's declared capabilities is rejected *for that + backend* with a precise "this backend does not support ; supported + here: …" error — **not** silently degraded, and **not** removed from stronger + backends. So Neo4j can later expose `agg.collect` or richer predicates that + SurrealDB lacks, additively, while the conformance suite still guarantees every + construct is identical across the backends that *do* claim it. + +This replaces the naive "intersection forever" rule with a model that (a) ships the +same safe common core today and (b) has a real growth path that never forks +semantics. + +## Package layout + +``` +src/agentforge_graph/store/query/ + __init__.py # exports: parse_query, validate_query, describe_schema, + # QueryAst, CompiledQuery, QuerySettings, ResultTable, + # QueryCapable, Compiler, QueryError (+ subclasses) + GRAMMAR.md # versioned EBNF of the accepted subset — source of truth + ast.py # frozen dataclasses: QueryAst + pattern/expr/return nodes + parser.py # Cypher-subset text -> QueryAst (tokenizer + recursive descent) + validator.py # vocabulary + exclusion + per-backend capability checks + capability.py # QueryCapable protocol, QuerySettings, ResultTable, CAPABILITIES + compile_base.py # Compiler ABC (singledispatch visitor) + CompiledQuery + compile_cypher.py # CypherCompiler + KuzuCypherCompiler + Neo4jCypherCompiler + compile_surreal.py# SurrealCompiler (full AST -> SurrealQL translator) + execute.py # shared bounded-cursor driver: timeout + row cap + expansion counter + schema.py # describe_schema() -> node/edge kinds + queryable attributes + errors.py # QueryError -> ParseError | ValidationError | GuardrailError + # | QueryDisabled | CapabilityError + +# extended (not new) files: + store/kuzu_store.py # + run_query() (implements QueryCapable, dialect=cypher/kuzu) + store/neo4j_store.py # + run_query() (QueryCapable, dialect=cypher/neo4j, read-only session) + store/surreal_store.py # + run_query() (QueryCapable, dialect=surrealql) + store/facade.py # + Store.query_graph() + Store.query_enabled + ingest/codegraph.py # + CodeGraph.query_graph() + CodeGraph.describe_schema() + serve/engine.py # + _Engine.query_graph() (folds staleness envelope) + serve/tools.py # + CkgQuery tool + QueryInput; registered in ALL_TOOLS + config.py # + QueryConfig(_Block, KEY="query") + cli.py # query subcommand: + --graph / --schema / --format / --limit +``` + +## The AST (`ast.py`) + +Frozen dataclasses, `core`-typed (labels are real `NodeKind`/`EdgeKind`, direction +reuses `core.Direction`). Sketch: + +```python +@dataclass(frozen=True) +class NodePattern: + var: str | None + label: NodeKind | None # validated: must be a real NodeKind + props: tuple[tuple[str, Literal], ...] # inline {name: "x"} equalities + +@dataclass(frozen=True) +class RelPattern: + var: str | None + kind: EdgeKind | None # validated: must be a real EdgeKind + direction: Direction # "out" | "in" | "both" + min_hops: int = 1 + max_hops: int = 1 # None (unbounded [*]) -> ValidationError + +@dataclass(frozen=True) +class PathPattern: # alternating node (rel node)* + elements: tuple[NodePattern | RelPattern, ...] + +# WHERE expression tree (closed set of node types) +Expr = Compare | BoolOp | Not | InList | StringPred | PatternExists +# Compare(lhs: PropRef, op: '=|<>|<|<=|>|>=', rhs: Literal) +# BoolOp(op: 'AND|OR', operands) / Not(operand) +# InList(lhs: PropRef, values: tuple[Literal, ...]) +# StringPred(lhs: PropRef, op: 'STARTS_WITH|ENDS_WITH|CONTAINS', rhs: str) +# PatternExists(pattern: PathPattern) # (a)-[:CALLS]->(b) existence + +@dataclass(frozen=True) +class ReturnItem: + expr: PropRef | Aggregate | CountStar # Aggregate.func in {count,collect,min,max,avg} + alias: str | None + +@dataclass(frozen=True) +class QueryAst: + match: tuple[PathPattern, ...] + where: Expr | None + returns: tuple[ReturnItem, ...] + distinct: bool + order_by: tuple[tuple[str, bool], ...] # (key, descending) + skip: int | None + limit: int | None +``` + +`PropRef(var, prop)` addresses `f.name`, `f.path`, `f.source`, `f.confidence`, +`n.attrs.` — mapped to the `Node`/`Provenance` fields in the compiler +(`source`/`confidence` live on `provenance`; `attrs.*` on the `attrs` dict; `span` +exposed as `start_line`/`end_line`). + +## Validator (`validator.py`) — the trust boundary + +Walks the AST and rejects with a structured `ValidationError` (message says *why*, +lists the offending token). Rules: + +- Every `NodePattern.label` ∈ `NodeKind`; every `RelPattern.kind` ∈ `EdgeKind`; + every `PropRef.prop` is a known/queryable attribute (from `schema.py`). +- **Hard exclusions** (also unrepresentable in the AST, so this is belt-and- + suspenders): write/DDL verbs never parse (grammar has no `CREATE/MERGE/SET/ + DELETE/DETACH/DROP`); no procedure/`CALL`; **unbounded var-length path** + (`max_hops is None`) rejected; a `MATCH` with ≥2 disconnected patterns and no + `WHERE`/inline predicate joining them (Cartesian product) rejected. +- `LIMIT`/`SKIP` are non-negative ints; aggregates only in `RETURN`. + +Parse-time exclusion is gate #1 of the read-only story; the read-only DB session +(where the backend supports it) is gate #2. + +Validation is **two-phase**: (1) a backend-independent phase (vocabulary + +exclusions above) that runs once; (2) a **capability phase** against the target +backend's declared `capabilities` — a construct the backend does not claim raises +`CapabilityError` naming the missing tier and what *is* supported here. Phase 2 is +what lets the subset grow per-backend without ever silently diverging (§Capability +tiers). The `visit`/walk is `@singledispatchmethod`-dispatched so adding an AST +node adds a validator registration, never edits an existing branch. + +## Compilers — polymorphic, dispatch-based (no dialect flags) + +A compiler is a **class per dialect**, not a function with a `dialect` branch, so +divergences are overridden methods and a new dialect is a new subclass — never an +edit to a shared if/elif: + +```python +class Compiler(ABC): # store/query/compile_base.py + dialect: ClassVar[str] + def compile(self, ast: QueryAst) -> CompiledQuery: ... # drives the walk + # one visit_* per AST node — @singledispatchmethod, so adding an AST node + # is a new registration, never a change to existing branches: + @singledispatchmethod + def visit(self, node) -> str: raise NotImplementedError(type(node)) + @visit.register + def _(self, n: NodePattern) -> str: ... + @visit.register + def _(self, n: Compare) -> str: ... + # …one per node type + +class CypherCompiler(Compiler): # compile_cypher.py — shared Cypher core + dialect = "cypher" +class KuzuCypherCompiler(CypherCompiler): # overrides ONLY the few Kuzu deltas + dialect = "kuzu" # (var-length syntax, id() form) +class Neo4jCypherCompiler(CypherCompiler): # overrides ONLY Neo4j deltas + dialect = "neo4j" # (elementId, tx timeout hint) +class SurrealCompiler(Compiler): # compile_surreal.py — full translator + dialect = "surrealql" # AST -> SELECT … ->kind->node … WHERE … +``` + +- **Literals are always parameterized** (`$p0`, `$p1`, …) — never string-spliced, + on every dialect. Single injection-free path. +- **Result shape is compile-fixed:** every compiler returns + `CompiledQuery(text, params, columns)` where `columns` is the projected RETURN + order, so `ResultTable.columns` is backend-independent by construction. +- Kuzu/Neo4j share the `CypherCompiler` body and override only genuine dialect + deltas; SurrealDB is a full sibling translator. The conformance suite proves all + three produce identical `ResultTable`s for the core tier. + +Adding a construct = register one `visit_*` on each compiler (three small methods), +guided by the grammar spec. Adding a dialect = one `Compiler` subclass. Neither +touches existing compiler code. + +## Capability + execution (`capability.py`, adapter `run_query`) + +```python +@runtime_checkable +class QueryCapable(Protocol): + query_dialect: ClassVar[str] # "kuzu" | "neo4j" | "surrealql" + capabilities: ClassVar[frozenset[str]] # tiers this backend executes identically + read_only_execution: ClassVar[bool] # True = backend enforces a read-only session (gate #2) + async def run_query(self, ast: QueryAst, s: QuerySettings) -> ResultTable: ... + +@dataclass(frozen=True) +class QuerySettings: # resolved from QueryConfig + max_rows: int + timeout_ms: int + max_expansions: int + +@dataclass(frozen=True) +class ResultTable: + columns: tuple[str, ...] + rows: tuple[tuple[Any, ...], ...] + truncated: bool + stopped_reason: str | None = None # None | "row_cap" | "timeout" | "expansion_cap" +``` + +Each adapter's `run_query` = compile (via its `Compiler`) → execute on its own +driver under bounds → normalize. Guardrails are a **required part of the contract**, +identical in effect on every backend and **proven by a runaway-query conformance +test** (§Conformance). A backend where a bound is not native must implement a +portable fallback — there is no "best-effort where it's hard" path. + +- **Read-only** is layered. Gate #1: the AST cannot represent a write (grammar has + no write verbs), so no compiler can emit one — this holds on *all* backends + unconditionally. Gate #2: a genuine read-only session where the backend offers + one (Neo4j `execute_read`). A backend declares which gates it provides via + `read_only_execution`; a conformance test submits a write-shaped attempt through + the same session and asserts refusal, so the guarantee is *tested per backend*, + not asserted in prose. +- **Timeout:** `asyncio.wait_for(…, timeout_ms/1000)` wraps every driver call + (portable, all backends); Neo4j additionally gets a server-side `tx_timeout`. On + trip → `stopped_reason="timeout"`. +- **Row cap:** compiler appends `LIMIT min(requested, max_rows) + 1`; `max_rows+1` + rows ⇒ drop the extra, `truncated=True`, `stopped_reason="row_cap"`. +- **`max_expansions` (intermediate-row budget) — enforced on every backend, no + approximation.** Where a backend exposes a native budget/profile hook it is used + directly. Where it does not (embedded Kuzu, SurrealDB), the adapter executes the + bounded query through a **paged/streamed cursor with an intermediate-row counter** + in `store.query.execute`: rows are pulled in bounded batches and the running + count is checked against `max_expansions`; on exceed the cursor is closed and the + partial result returns with `truncated=True`, `stopped_reason="expansion_cap"`. + The unbounded-path ban + per-pattern `max_hops` cap keep the intermediate set + finite so the counter always terminates. This is the portable fallback that makes + the bound a real, tested guarantee rather than a documented gap. + +`store/query/execute.py` holds the shared bounded-cursor driver + normalization so +each adapter supplies only its native "run this compiled statement, yield rows" +primitive — the bounding logic is written once, not re-implemented per backend. + +The facade wires it: + +```python +# store/facade.py +@property +def query_enabled(self) -> bool: + return isinstance(self._graph, QueryCapable) + +@property +def query_capabilities(self) -> frozenset[str]: + return self._graph.capabilities if isinstance(self._graph, QueryCapable) else frozenset() + +async def query_graph(self, text: str, settings: QuerySettings) -> ResultTable: + if not isinstance(self._graph, QueryCapable): + raise QueryDisabled(self._graph_driver_name) + ast = parse_query(text) # ParseError on bad syntax + validate_query(ast, backend=self._graph.capabilities) # ValidationError / CapabilityError + return await self._graph.run_query(ast, settings) +``` + +`query_capabilities` is what `serve/engine.py` folds into `engine.capabilities` for +the capability-driven tool registry and what `ckg_status` / `describe_schema` +report, so callers see exactly what the active backend can do. + +`CodeGraph.query_graph` / `CodeGraph.describe_schema` are thin pass-throughs used +by both CLI and the tool. + +## CLI (`cli.py`, `query` subcommand) + +Extend the existing `query` parser (keeps the NL/hybrid path intact): + +- `--graph ''` — structural query (mutually exclusive with the positional NL + `query`). +- `--schema` — print the queryable vocabulary (`describe_schema()`), no query. +- `--format {table,json}` (default `table`) — **new to this repo**, introduced as a + reusable `cli/format.py` helper (`render_table(columns, rows) -> str`, + `emit(result, fmt)`), *not* inline in the handler. Other verbs can adopt the same + helper later without a rewrite; this PR only wires it to `query`. `table` mirrors + `_routes`'s column-width alignment; `json` emits `{columns, rows, truncated, + stopped_reason}`. +- `--limit N` — caller LIMIT (still clamped to `query.max_rows`). + +Handler branches before the existing retrieve dispatch. `QueryError` → stderr +message (the structured "why") + exit 2, reusing the ENH-026 exit-2 convention. + +## MCP tool (`serve/tools.py`, `serve/engine.py`) + +```python +class QueryInput(_Fed): + query: str = Field(..., description="Cypher-subset structural query") + limit: int | None = Field(None, description="row cap (clamped to server max)") + params: dict[str, Any] | None = Field(None) + +class CkgQuery(_CkgTool): + name = "ckg_query" + description = "Escape hatch for precise STRUCTURAL questions no typed verb " + "covers (e.g. classes tagged X with no inbound CALLS). Read-only. " + "Use ckg_search for semantic 'find code about…' questions." + input_schema = QueryInput + async def run(self, **kw) -> str: + data = await eng.query_graph(query, limit, params) # {columns,rows,truncated,...} + return json.dumps(data) +``` + +`_Engine.query_graph` folds the **staleness envelope** onto the table exactly like +the survey methods: `{columns, rows, truncated, **(await self.staleness()), +tool_api_version, query_lang_version}`. + +**Capability-driven registration (not always-on-plus-error).** feat-008's toolset +becomes capability-aware rather than a hardcoded list: each tool declares what it +needs and the server includes the tools whose requirements the live config + +backend satisfy. + +```python +class _CkgTool(Tool): + requires: ClassVar[frozenset[str]] = frozenset() # capability gates, default none +class CkgQuery(_CkgTool): + requires = frozenset({"query"}) # needs query.enabled + a QueryCapable backend +``` + +`code_graph_tools()` filters `ALL_TOOLS` by `tool.requires ⊆ engine.capabilities`, +so `ckg_query` appears exactly when the query surface is actually available and is +cleanly absent otherwise — an agent never discovers a tool that only errors. This +is a reusable gate: future capability-gated tools (write tools post-1.0, backend- +specific verbs) use the same mechanism instead of each inventing an opt-out. + +The contract test becomes **profile-parameterized**: `test_schemas.py` asserts the +tool set + schemas for each profile — `{query: enabled}` includes `ckg_query` +(properties = query/limit/params/service), `{query: disabled}` excludes it. Both +are pinned, so drift on either path fails CI. This keeps the snapshot deterministic +*and* honest, without the "register a tool that returns an error" compromise. + +## Config (`config.py`) + +```python +class QueryConfig(_Block): + KEY: ClassVar[str] = "query" + enabled: bool = True + max_rows: int = 1000 + timeout_ms: int = 5000 + max_expansions: int = 50000 + allow_in_mcp: bool = True +``` + +Auto-discovered by `block_keys()` (no registration list). Read via +`QueryConfig.load(config)`. `ckg_status` reports `query_lang_version` ("1.0") + +whether the active backend is query-capable. + +## Conformance suite (the load-bearing test) + +Extend `core/conformance.py` with `QueryConformance` — a base class (pytest-free at +import, like `GraphStoreConformance`) holding a fixed query set run against a shared +fixture graph (extend `make_sample_subgraph()` with tagged classes, interfaces, +call edges so predicates/aggregates/pattern-existence are all exercised). Each +`test_*` asserts the **normalized `ResultTable` is identical** across backends. + +The suite has three mandatory parts, each a *contract every query-capable backend +must pass* — this is what turns the extensibility principles into enforced +guarantees rather than prose: + +1. **Result parity** — the fixed query set returns identical normalized + `ResultTable`s across all backends claiming the core tier. +2. **Bounded execution** — a deliberately runaway query (large fan-out) returns a + partial result with `truncated=True` and the right `stopped_reason` on *every* + backend, proving the `max_expansions`/timeout/row-cap fallback actually works + (no backend is exempt with "not natively supported"). +3. **Read-only** — a write-shaped attempt through the same session is refused on + every backend (belt-and-suspenders with the AST gate). + +A new backend added later opts into query support purely by subclassing this suite +and passing — same pattern as feat-003's storage conformance, no bespoke wiring. + +Per-backend wiring extends the existing `tests/store/test_*_conformance.py`: + +- **Kuzu** — embedded, runs in default CI (the always-on gate). +- **Neo4j / SurrealDB** — env-gated live (needs the running server; colima/docker), + matching feat-003's existing backend-test gating and the storage-backends + conformance job. Same three-part suite, must match Kuzu's rows. + +## Chunk plan (the feat-015 PR = these commits, in order) + +| # | Chunk | Lands | +|---|---|---| +| 1 | **Grammar + AST + parser + validator + schema + capability** (`GRAMMAR.md`, `ast.py`, `parser.py`, `validator.py`, `schema.py`, `capability.py`, `errors.py`) | The whole trust boundary + the capability seam, pure + framework-free. Unit tests: every excluded construct rejected; every supported construct → expected AST; two-phase validation incl. `CapabilityError`; `describe_schema()`. **Heaviest chunk.** | +| 2 | **Compiler base + Cypher + Kuzu execution** (`compile_base.py` visitor, `compile_cypher.py`, `execute.py` bounded cursor, `KuzuGraphStore.run_query`, `Store.query_graph`, `CodeGraph.query_graph`) + Kuzu `QueryConformance` (all 3 parts) | First end-to-end vertical slice on the default backend; the bounded-cursor + parity/bound/read-only conformance land here. | +| 3 | **Neo4j execution** (`Neo4jCypherCompiler`, `Neo4jGraphStore.run_query`, read-only session) + env-gated Neo4j conformance | Proves the Cypher compiler + bounds are backend-portable (subclass only, no shared edits). | +| 4 | **SurrealQL compiler + SurrealDB execution** (`SurrealCompiler`, `SurrealGraphStore.run_query`) + env-gated Surreal conformance | The full translator; closes the 3-backend story, all three conformance parts green. | +| 5 | **CLI** — `cli/format.py` helper + `ckg query --graph / --schema / --format / --limit` + `test_cli_query.py` | Power-user surface + the reusable formatter seam. | +| 6 | **MCP tool + capability registry** — `_CkgTool.requires`, capability-filtered `code_graph_tools()`, `CkgQuery`, `_Engine.query_graph` + staleness envelope, profile-parameterized `test_schemas.py` | Agent surface + the honest, reusable tool-gating seam. | +| 7 | **Config + status + docs** — `QueryConfig`, `query_lang_version` in `ckg_status`, guide + README + changelog | Ship polish. | + +Each chunk is a self-contained Conventional Commit; the gate (ruff/mypy/pytest/≥90%) +must pass per chunk. Chunks 1–2 are the bulk (they carry the extension seams); 3–4 +add breadth as pure additions behind the conformance suite; 5–7 are the surfaces. + +## Extensibility seams (summary) + +Every "this might grow later" axis is a declared seam, so growth is additive and +none of it is a future rewrite: + +| To add… | You touch | You do **not** touch | +|---|---|---| +| A query construct | new AST node + 1 parser production + 1 validator registration + 1 `visit_*` per compiler + a `GRAMMAR.md` bump | any existing branch (singledispatch), the backends' shared code | +| A storage backend | 1 `QueryCapable` adapter + 1 `Compiler` subclass + subclass the conformance suite | the registry mechanism, other backends, the core | +| A backend-specific capability | declare it in that adapter's `capabilities` + tier-gate it in the validator | other backends' behaviour, the core-tier guarantee | +| A guardrail (e.g. memory cap) | 1 field on `QuerySettings` + enforcement in the shared `execute.py` cursor | per-backend code (bounding is written once) | +| A capability-gated tool | subclass `_CkgTool` with `requires={…}` | the registration/filter logic | +| A CLI output format | 1 branch in `cli/format.py` | every command (they share the helper) | + +## Alternatives considered + +| Option | Why not | +|---|---| +| JSON query DSL as the public surface | Rejected at analysis — Cypher/openCypher/GQL (ISO 39075) is the field standard for property-graph traversal; JSON DSLs suit doc/search key-lookup, not multi-hop patterns. | +| Parser library (`lark`/`pyparsing`) | New dependency vs a bounded subset that a hand-written recursive-descent parser handles; keeps base install lean. | +| Pass raw text to Kuzu (lean on its parser) | Breaks portability (no SurrealDB path), safety (no validation), and opens an injection surface. The AST-as-trust-boundary is the whole point. | +| Add `run_query` to the `GraphStore` ABC | Breaks out-of-tree adapters + the locked contract (feat-001/003). Optional `QueryCapable` protocol instead — opt-in, non-breaking. | +| Compiler as one function with a `dialect` flag | if/elif on dialect doesn't scale past two backends and mixes their logic. Polymorphic `Compiler` subclasses + singledispatch visitor instead — a new dialect/construct is additive. | +| `ckg_query` always registered, `run()` returns an error when unavailable | A tool that only errors is dishonest discovery and made the contract test config-coupled. Capability-driven registration (`requires ⊆ engine.capabilities`) + profile-parameterized snapshot is the reusable, honest seam. | +| "Subset = intersection of all backends, forever" | Permanently caps stronger backends at the weakest one, with no growth path. Capability tiers ship the same safe core today but let a backend advertise more additively, still conformance-guaranteed identical where claimed. | +| `max_expansions` "approximated where the backend has no native budget" | That's an effort-driven workaround. A shared bounded-cursor with an intermediate-row counter (`execute.py`) makes the bound real and *tested* on every backend. | + +## Risks + +| Risk | Mitigation | +|---|---| +| SurrealQL can't match Cypher semantics for some construct | The construct lives in a capability tier SurrealDB doesn't claim → `CapabilityError` there, still available (identically) on backends that do claim it. Core tier is conformance-proven identical on all three. Never silently divergent. | +| Hand-written parser bugs / injection | Bounded grammar defined in `GRAMMAR.md`, no `eval`, parameterized literals only on every dialect; property/fuzz tests on the parser; the validator is a second closed-set gate. | +| `max_expansions` enforcement | Real and tested, not approximated: shared bounded-cursor counter (`execute.py`) + unbounded-path ban + `max_hops` cap; the runaway-query conformance test proves the bound trips with `stopped_reason="expansion_cap"` on every backend. | +| Live-backend conformance flakes/omitted in CI | Kuzu conformance is the always-on gate; Neo4j/Surreal env-gated like existing feat-003 backend tests; a backend that skips is reported, not assumed-passing. | +| Capability tiers add complexity we don't use yet | The 0.6.4 ship declares only the core tier on all three backends, so there is no branching in practice today — but the *seam* (declared `capabilities` + phase-2 validation) costs a few lines now and avoids a rewrite when the first backend-specific construct lands. | + +## Open questions (resolve before `accepted`) + +1. **Parameter passing in the tool** — expose `params` in `ckg_query` v1, or defer + (literals-in-text only) to keep the first tool contract minimal? *(Lean: include + `params` — it's the safe way for an agent to pass values; small cost.)* +2. **`--format json` shape** — resolved to a **reusable `cli/format.py` helper** + wired only to `query` in this PR (other verbs adopt it later without a rewrite), + rather than either inline one-off code or a full sweep of every command now. Any + objection to that middle path? +3. **Attribute allow-list granularity** — expose `n.attrs.*` wholesale, or only a + curated attribute set in `describe_schema()`? *(Lean: curated set the schema + advertises; unknown `attrs.*` keys pass through as opaque string compares.)* diff --git a/docs/features/TRACKER.md b/docs/features/TRACKER.md index 836a781..5a6e3e1 100644 --- a/docs/features/TRACKER.md +++ b/docs/features/TRACKER.md @@ -59,6 +59,7 @@ Legend: `proposed` → `accepted` → `in-progress` → `shipped` (also | [012](feat-012-llm-enrichment.md) | LLM enrichment (summaries, tags) | 3 diff | 0.4 | shipped (tags + summaries) | 006 | — | ✅ | | [013](feat-013-agent-auto-configuration.md) | Agent auto-configuration & frictionless first run | 4 adoption | 0.6.2 | **shipped (0.6.2)** | 008 | — | ✅ | | [014](feat-014-watch-and-ci-indexing.md) | Watch mode (local) + CI-triggered central indexing | 4 adoption | 0.6.3 | **shipped (0.6.3)** | 004 | — | ✅ | +| [015](feat-015-read-only-graph-query.md) | Read-only graph query (`ckg query --graph` / `ckg_query`) | 1 serve | 0.6.4 | **in-progress (chunk 1)** | 001,003,008 | — | ✅ | --- diff --git a/docs/features/feat-015-read-only-graph-query.md b/docs/features/feat-015-read-only-graph-query.md new file mode 100644 index 0000000..e8d9061 --- /dev/null +++ b/docs/features/feat-015-read-only-graph-query.md @@ -0,0 +1,277 @@ +# feat-015: Read-only graph query language (`ckg query --graph` / `ckg_query`) + +## Metadata + +| Field | Value | +|---|---| +| **ID** | feat-015 | +| **Title** | Read-only, guard-railed graph query surface (Cypher-subset) + `ckg_query` verb/tool | +| **Status** | in-progress | +| **Target version** | 0.6.4 | +| **Layer** | 1 serve (retrieval & serving) — escape-hatch complement to the typed verbs | +| **Area** | `store` (query AST, validator, per-backend compilers, execution) · `serve` (`ckg_query` tool) · CLI | +| **Depends on** | feat-001 (locked node/edge vocabulary), feat-003 (storage adapters), feat-008 (tool API) | +| **Graduated from** | [FA-003](../feature-analysis/FA-003-read-only-graph-query-language.md) | +| **Relates to** | feat-006 (hybrid retrieval — the *guided/semantic* path this complements) | + +--- + +## 1. Why this feature + +The CKG today answers questions through **fixed, typed verbs**: `ckg search`, +`ckg impact`, `ckg neighbors`, `ckg routes`, `ckg decisions`, `ckg history`, and +so on. Each is excellent for the question it was designed for, and the guided +shape is exactly right for the common case — an agent that just wants "who calls +this" does not want a query language. + +But the typed verbs are a **closed set.** Any question that does not map to an +existing verb is **unanswerable without us shipping a new verb**: + +- "Find all public methods on classes tagged `Repository` that have no inbound + CALLS edge." +- "List routes whose handler imports the legacy auth module." +- "Show interfaces implemented by more than five classes." + +Power users and sophisticated agents hit this ceiling quickly, and every such +question becomes a feature request. This feature adds a **read-only, +guard-railed query surface** over the typed graph so arbitrary structural +questions can be expressed directly — without weakening the safety or the +guided-verb experience. + +## 2. Why it ships in the engine + +- **The typed graph already has the answers.** We index nodes, edges, kinds, + provenance, and attributes; a query surface unlocks data we already store + instead of gating it behind bespoke verbs. +- **It absorbs verb-sprawl pressure.** feat-008 §8 flags "every feature wants a + tool" as a real risk. A general query verb is the pressure-relief valve: novel + questions get a query, not a new tool. +- **Read-only discipline must be enforced centrally.** Letting an LLM emit graph + queries is only safe if writes, unbounded traversals, and resource-exhausting + patterns are rejected by *us*, not trusted to the caller — the same stance + feat-008 takes on `depth`/`k` clamps. The enforcement is a correctness + boundary, not a preference, so it belongs in the deterministic engine + (ADR-0001): query AST + validator + per-backend compilers live in `store` with + no `agentforge` import; only the thin `ckg_query` tool wrapper is framework + layer. + +## 3. How consumers benefit + +- **Power user (CLI):** `ckg query --graph 'MATCH (c:Class)-[:TAGGED]->(t) WHERE + t.name = "Repository" RETURN c.name'` answers an ad-hoc structural question with + no code change on our side. +- **Agent (MCP):** a new `ckg_query` tool lets a capable agent compose a precise + structural query when no typed verb fits — then fall back to the guided verbs + for everything routine. +- **Us:** fewer one-off verb requests; novel needs are expressible today and + inform which queries deserve to *become* a typed verb later (the promotion + path, §10). + +## 4. Feature specifications + +### 4.1 User-facing experience + +```bash +# CLI — ad-hoc structural query, read-only +ckg query --graph 'MATCH (f:Function) WHERE NOT (f)<-[:CALLS]-() RETURN f.name, f.path LIMIT 50' + +# Output formats +ckg query --graph '' --format table # default, human-readable +ckg query --graph '' --format json # machine-readable rows + +# Introspect the queryable vocabulary (node/edge kinds + attributes) +ckg query --schema +``` + +The existing natural-language path — `ckg query ""` (feat-006 hybrid +retrieval) — is untouched. `--graph` selects the structural surface; the two are +documented as complementary (retrieval to *discover*, query to *interrogate*). + +```jsonc +// MCP tool +{ + "name": "ckg_query", + "arguments": { + "query": "MATCH (c:Class)-[:IMPLEMENTS]->(i:Interface) RETURN i.name, count(c) AS impls ORDER BY impls DESC", + "limit": 100 + } +} +// → { columns: [...], rows: [...], truncated: false, indexed_commit, dirty } +``` + +The result carries the same **staleness envelope** as every other tool +(`indexed_commit`, `dirty`, `truncated`) — feat-008's no-silent-caps rule applies +here too. + +### 4.2 Public API / contract + +- **CLI:** `ckg query --graph ''` (structural) and `ckg query --schema` + (vocabulary introspection). The `ckg query ""` NL path is unchanged. +- **MCP tool:** `ckg_query { query, limit?, params? }` → columnar result + (`columns`, `rows`) + staleness envelope. **Read-only** — registered alongside + feat-008's locked toolset. +- **Query language:** a **read-only subset** over our locked vocabulary + (feat-001 node/edge kinds). Supported shape (target): `MATCH` patterns, `WHERE` + (comparisons, `AND/OR/NOT`, `IN`, string predicates, pattern-existence + `(a)-[:KIND]->(b)`), `RETURN` with projection + aliases, aggregates (`count`, + `collect`, `min/max/avg`), `ORDER BY`, `SKIP`/`LIMIT`. Provenance/attributes + are queryable columns (`f.source`, `f.confidence`, `n.attrs.*`). +- **Hard exclusions (rejected at parse):** any write/DDL + (`CREATE/MERGE/SET/DELETE/DROP/DETACH`), procedure/function calls that touch + the host, variable-length unbounded paths (`[*]` with no upper bound), and + Cartesian products without a join predicate. A rejected query returns a + structured error explaining *why*, not a stack trace. + +### 4.3 Internal mechanics + +**The portability problem.** The query surface must behave identically across our +pluggable backends (ADR-0006): Kuzu and Neo4j speak openCypher; SurrealDB speaks +SurrealQL. We **never** pass raw user text to a backend. Instead: + +1. **Parse** the incoming query into our own read-only **query AST** (validated + against the locked vocabulary and the exclusion rules in §4.2). This is the + single trust boundary — anything the AST cannot represent cannot run. +2. **Compile** the AST to each backend's native dialect via a per-backend + compiler in the `store` adapter (Cypher for Kuzu/Neo4j; SurrealQL for + SurrealDB). The same conformance discipline that proves storage adapters + interchangeable (feat-003) extends to a **query-conformance suite**: one query + set, identical results across backends. +3. **Execute** with enforced guardrails: statement timeout, max rows, max + expansions/traversal bound, and a read-only transaction/connection. Results + are normalized to `{columns, rows}` regardless of backend. + +- **Read-only enforcement is layered:** AST exclusions (parse-time) + read-only + DB session (execution-time). Two independent gates. +- **Guardrail defaults mirror feat-008:** row cap, depth/expansion cap, + wall-clock timeout — all configurable, all reported in `truncated`. + +### 4.4 Module packaging + +- `agentforge_graph.store` — query AST, validator, per-backend compilers, + execution. Stays in the **deterministic engine**; no framework import + (ADR-0001). +- `agentforge_graph.serve` — the `ckg_query` tool wrapper (framework layer). +- Ships in the base install; no new extra. A backend that has no query compiler + reports `query.enabled: false` and still serves the typed verbs. + +### 4.5 Configuration + +```yaml +query: + enabled: true # the structural surface can be disabled wholesale + max_rows: 1000 # hard row cap (truncation reported) + timeout_ms: 5000 # per-query wall-clock budget + max_expansions: 50000 # traversal/intermediate-row bound + allow_in_mcp: true # expose ckg_query as an MCP tool (vs CLI-only) +``` + +Read via the engine's framework-free config path (ADR-0001): `app.query.*` or a +standalone `ckg.yaml` `query:` block. + +## 5. Plug-and-play & upgrade story + +- The supported query shape is **versioned**: adding a clause is minor, a + semantics change is major. `ckg_status` reports the query-language version so + long-lived clients detect mismatch (as it does for the tool API). +- A new storage backend ships a query compiler and must pass the + query-conformance suite before it is considered query-capable; until then it + serves typed verbs but reports `query.enabled: false`. + +## 6. Cross-language parity + +n/a at the indexed-language level — the query surface is over the graph, not the +source. The parity that *does* matter: identical results across storage backends, +covered by the query-conformance suite (§4.3, §7). + +## 7. Test strategy + +- **Parser/validator unit tests:** every excluded construct + (write/DDL/unbounded-path/host-call) is rejected with a clear error; every + supported construct parses to the expected AST. +- **Query-conformance suite:** a fixed query set runs against each query-capable + backend over a shared fixture index; **results must match** (the load-bearing + test). Structured identically to feat-003's storage-conformance suite so a new + backend opts in by passing it. +- **Guardrail tests:** a deliberately expensive query hits the row/timeout/ + expansion cap, returns partial results with `truncated: true`, and does not + exhaust memory. +- **Read-only tests:** a write/DDL query is rejected at parse *and* would be + rejected by the read-only session (belt-and-suspenders). +- **Tool-contract test:** `ckg_query` JSON-schema snapshot (drift fails CI, per + feat-008's contract discipline). +- **Agent-in-the-loop (env-gated):** an agent answers a structural question it + has no typed verb for, using `ckg_query`, and produces a grounded result. + +## 8. Risks & open questions + +| Risk / Question | Mitigation / Decision | +|---|---| +| LLM emits a resource-exhausting query | Timeout + row cap + expansion bound + unbounded-path ban; all enforced engine-side, reported in result | +| Backend dialect divergence yields different results | Query-conformance suite is mandatory; a backend that fails it is not query-capable (`query.enabled: false`) | +| Re-implementing a query parser is heavy | Scope to a **small read-only subset**, not full Cypher. Kuzu (default backend) is an openCypher engine, so validate-then-near-pass-through; the AST layer is what stays portable | +| Surface area for injection/abuse | No raw pass-through ever; only the validated AST compiles to a backend. Single trust boundary | +| Overlaps with typed verbs / confuses agents | Tool description positions `ckg_query` as the *escape hatch* for questions no typed verb covers; guided verbs remain the default | +| Exposing internal schema to callers | `ckg query --schema` / a schema-introspection helper documents node/edge kinds + queryable attributes so callers query a documented vocabulary | +| **Backend coverage for the 0.6.4 slice** | **Resolved in design (design-015): which backends are query-capable at first ship vs follow-up.** The conformance suite is structured so backends opt in incrementally; non-capable backends report `query.enabled: false` | + +## 9. Out of scope + +- **Write queries / graph mutation from agents** — needs a provenance + authz + story (feat-008 defers write tools post-1.0); this feature is strictly + read-only. +- Full openCypher / SurrealQL surface — we expose a curated, safe subset. +- A visual query builder. +- Stored/parameterized server-side query templates (possible follow-up if common + queries emerge — those are also the candidates to promote to a typed verb). + +## 10. Design notes + +**The AST is the whole design.** The portability story (Kuzu/Neo4j Cypher vs +SurrealDB SurrealQL) and the safety story (read-only, bounded) both hang on one +decision: **never execute caller text directly — validate into our own AST, then +compile per backend.** That single trust boundary is what makes the feature safe +to expose to an LLM and portable across ADR-0006 backends at the same time. + +**Lean on Kuzu.** Our default backend is already an openCypher engine, so the +Kuzu compiler is close to a pass-through *after* AST validation — the real +engineering is the validator + the SurrealDB compiler + the conformance suite. +This makes the default-backend path cheap and the portability cost explicit. + +**Relationship to feat-006.** `ckg_query` is **not** a replacement for hybrid +retrieval — retrieval is the right tool for "find code about X" (semantics, +ranking, provenance weighting). `ckg_query` is for *precise structural* questions +with exact predicates. Document the two as complementary: retrieval to discover, +query to interrogate. + +**Promotion path.** Queries that show up repeatedly are evidence for a new typed +verb. Treat `ckg_query` usage as a backlog signal: the most common ad-hoc queries +are the best candidates to graduate into first-class, optimized verbs. + +**Resolved decision (reviewed at analysis).** Expose a **Cypher-subset text +syntax** as the caller-facing surface; keep our **structured AST as the internal +trust boundary** (it resembles a JSON DSL, but it is an implementation detail, not +the public language). Callers never have their text executed — it is parsed and +validated into the AST, which each backend compiles from. + +Rationale — the industry-standard approach for labeled property graphs: the +ecosystem has standardized on Cypher/openCypher/GQL (Neo4j, Kuzu, Memgraph, +FalkorDB, Neptune all speak Cypher-family syntax; **GQL was published as ISO/IEC +39075 in April 2024**, the first new ISO database query language since SQL, and it +is directly Cypher-lineage). JSON query DSLs are the norm for document/search +engines (key-lookup/filter), not multi-hop graph traversal. And it fits our +backends: Kuzu and Neo4j *are* openCypher engines, so on the common path we +validate-then-near-pass-through rather than translating into a foreign dialect. + +Accepted cost: we own a **small, read-only** Cypher-subset parser (the validation +trust boundary), bounded deliberately — not full Cypher — with the +query-conformance suite keeping backends result-identical. + +## 11. References + +- [FA-003](../feature-analysis/FA-003-read-only-graph-query-language.md) — source analysis. +- [feat-001](feat-001-graph-schema-and-core-contracts.md) — locked node/edge vocabulary the query surface is typed against. +- [feat-003](feat-003-graph-storage-adapters.md) — storage adapters + conformance-suite discipline this extends. +- [feat-008](feat-008-mcp-server-and-tool-api.md) — tool API, staleness envelope, guardrail/clamp stance. +- [feat-006](feat-006-hybrid-retrieval.md) — the semantic/guided path `ckg_query` complements. +- ADR-0001 (framework-free engine core), ADR-0006 (pluggable storage backends). +- design-015 (the *how*: file layout, AST shape, chunk plan) — written next. From 7a0ddf0f2deb1bcedb90229fc01ad8354c5fd890 Mon Sep 17 00:00:00 2001 From: Khemchand Joshi Date: Wed, 8 Jul 2026 11:13:35 +0530 Subject: [PATCH 02/11] feat-015: query AST + parser + validator (chunk 1, trust boundary) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First chunk of the read-only graph query surface: the framework-free store/query/ package that turns caller text into a validated AST. No backend execution yet (chunk 2). - GRAMMAR.md: versioned EBNF (v1.0) of the accepted Cypher subset; the source of truth the parser mirrors. Write verbs / CALL / WITH have no production. - ast.py: frozen query AST (the single trust boundary). - parser.py: hand-written tokenizer + recursive-descent parser, no new dependency. Syntax only; labels/kinds kept as raw strings. - validator.py: two-phase gate — (1) vocabulary (labels in NodeKind, rel types in EdgeKind, curated properties or attrs.*, bound-variable + Cartesian-product checks) and (2) capability tiers against the target backend. - schema.py: curated node-property catalogue mapped to the shared _rowmap columns (f.path -> sym_path, span -> span_start/end, provenance columns), so properties resolve identically on every backend; describe_schema() for --schema. QUERY_LANG_VERSION reported by ckg status (chunk 7). - capability.py: capability tiers, the optional QueryCapable protocol, and QuerySettings / ResultTable used by execution (chunk 2). - Extend the ADR-0001 layering test to the new subpackage. Resolved: a comparison's RHS is a parameterized literal (no property-to-property compare), so WHERE cannot join disconnected patterns — multi-pattern MATCH must be connected by shared variables. 72 unit tests. Full gate green: 1023 passed, 94.62% coverage; ruff + mypy --strict clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/agentforge_graph/store/query/GRAMMAR.md | 87 ++++ src/agentforge_graph/store/query/__init__.py | 58 +++ src/agentforge_graph/store/query/ast.py | 153 ++++++ .../store/query/capability.py | 77 +++ src/agentforge_graph/store/query/errors.py | 66 +++ src/agentforge_graph/store/query/parser.py | 448 ++++++++++++++++++ src/agentforge_graph/store/query/schema.py | 104 ++++ src/agentforge_graph/store/query/validator.py | 250 ++++++++++ tests/store/query/__init__.py | 0 tests/store/query/test_parser.py | 198 ++++++++ tests/store/query/test_schema.py | 69 +++ tests/store/query/test_validator.py | 158 ++++++ tests/store/test_layering.py | 4 +- 13 files changed, 1671 insertions(+), 1 deletion(-) create mode 100644 src/agentforge_graph/store/query/GRAMMAR.md create mode 100644 src/agentforge_graph/store/query/__init__.py create mode 100644 src/agentforge_graph/store/query/ast.py create mode 100644 src/agentforge_graph/store/query/capability.py create mode 100644 src/agentforge_graph/store/query/errors.py create mode 100644 src/agentforge_graph/store/query/parser.py create mode 100644 src/agentforge_graph/store/query/schema.py create mode 100644 src/agentforge_graph/store/query/validator.py create mode 100644 tests/store/query/__init__.py create mode 100644 tests/store/query/test_parser.py create mode 100644 tests/store/query/test_schema.py create mode 100644 tests/store/query/test_validator.py diff --git a/src/agentforge_graph/store/query/GRAMMAR.md b/src/agentforge_graph/store/query/GRAMMAR.md new file mode 100644 index 0000000..6abf9b0 --- /dev/null +++ b/src/agentforge_graph/store/query/GRAMMAR.md @@ -0,0 +1,87 @@ +# CKG query grammar (read-only Cypher subset) + +**Language version: 1.0** (`QUERY_LANG_VERSION` in `schema.py`; reported by +`ckg status`). This file is the **source of truth** for what the parser accepts. +Adding a clause is a *minor* bump; changing the meaning of an existing clause is +a *major* bump. Keep the parser productions and this EBNF in lock-step. + +The surface is a deliberately small, **read-only** subset of openCypher. Caller +text is never executed — it is parsed into the `QueryAst` (the trust boundary), +validated, then compiled per backend. Anything not in this grammar does not +parse; a few shapes the grammar *can* express are rejected by the validator (see +"Validator-enforced rules"). + +## EBNF + +```ebnf +query = "MATCH" pattern { "," pattern } + [ "WHERE" expr ] + "RETURN" [ "DISTINCT" ] returnItem { "," returnItem } + [ "ORDER" "BY" orderKey { "," orderKey } ] + [ "SKIP" INT ] + [ "LIMIT" INT ] ; + +pattern = nodePat { relPat nodePat } ; +nodePat = "(" [ IDENT ] [ ":" IDENT ] [ props ] ")" ; +props = "{" propEq { "," propEq } "}" ; +propEq = IDENT ":" literal ; + +relPat = ( "-" | "<-" ) [ "[" [ IDENT ] [ ":" IDENT ] [ varlen ] "]" ] ( "-" | "->" ) ; +varlen = "*" [ INT ] [ ".." [ INT ] ] ; (* bounds; unbounded rejected at validation *) + +expr = orExpr ; +orExpr = andExpr { "OR" andExpr } ; +andExpr = notExpr { "AND" notExpr } ; +notExpr = "NOT" notExpr | primary ; +primary = "(" expr ")" | patternExists | predicate ; +patternExists= pattern ; (* a path with >= 1 relationship *) +predicate = propRef ( compareOp literal + | "IN" "[" literal { "," literal } "]" + | stringOp STRING ) ; +compareOp = "=" | "<>" | "<" | "<=" | ">" | ">=" ; +stringOp = "STARTS" "WITH" | "ENDS" "WITH" | "CONTAINS" ; + +returnItem = ( aggregate | propRef | IDENT ) [ "AS" IDENT ] ; +aggregate = ( "count" | "collect" | "min" | "max" | "avg" ) + "(" [ "DISTINCT" ] ( propRef | IDENT | "*" ) ")" ; (* "*" only for count *) +orderKey = ( propRef | IDENT ) [ "ASC" | "DESC" ] ; + +propRef = IDENT "." IDENT { "." IDENT } ; (* f.name, n.attrs.role *) +literal = STRING | [ "-" ] ( INT | FLOAT ) | "true" | "false" | "null" ; + +IDENT = /[A-Za-z_][A-Za-z0-9_]*/ ; +STRING = /'...'/ | /"..."/ ; (* backslash escapes: \n \t \r \\ \' \" *) +INT = /[0-9]+/ ; +FLOAT = /[0-9]+\.[0-9]+/ ; +``` + +Keywords are case-insensitive. Aggregate function names are not reserved. + +## Excluded by construction (no production — a `ParseError`) + +Writes/DDL (`CREATE`, `MERGE`, `SET`, `DELETE`, `DETACH`, `REMOVE`, `DROP`), +procedure/function `CALL`, and the `WITH` / `UNWIND` / `FOREACH` / `LOAD CSV` / +`USE` clauses. There is simply no rule that accepts them, so the read-only +guarantee starts at the grammar. + +## Validator-enforced rules (parses, but rejected) + +- Node labels must be a `NodeKind`; relationship types must be an `EdgeKind` + (feat-001 locked vocabulary). +- Property references must be a curated name (`name`, `kind`, `path`, + `start_line`, `end_line`, `source`, `extractor`, `commit`, `confidence`) or an + opaque `attrs.`. Every referenced variable must be bound in `MATCH`. +- **Unbounded variable-length** paths (`[*]`, `[*2..]`) are rejected — give an + upper bound (`[:CALLS*1..3]`). +- Multiple `MATCH` patterns must be connected by shared variables; disconnected + patterns (a Cartesian product) are rejected. A comparison's right-hand side is + a **literal** — property-to-property comparison is not in the v1 subset, so a + `WHERE` cannot be used to join otherwise-disconnected patterns. + +## Capability tiers + +Each construct maps to a capability a backend declares: `core`, `agg.basic`, +`pattern.exists`, `string.pred`, `path.varlen` (the mandatory **core tier**), and +optional extensions such as `agg.collect`. A construct the target backend does +not declare raises a `CapabilityError` naming what is supported — it is never +silently degraded. diff --git a/src/agentforge_graph/store/query/__init__.py b/src/agentforge_graph/store/query/__init__.py new file mode 100644 index 0000000..1ccf0df --- /dev/null +++ b/src/agentforge_graph/store/query/__init__.py @@ -0,0 +1,58 @@ +"""agentforge_graph.store.query — the read-only graph query surface (feat-015). + +A caller writes a bounded Cypher-subset string; we parse it into our own frozen +``QueryAst`` (the single trust boundary), validate that AST against the locked +feat-001 vocabulary + read-only exclusion rules + the target backend's declared +capabilities, and (chunk 2) compile it per backend and execute it under enforced +bounds. Caller text is never executed directly. + +This package is deterministic engine code: it imports only ``core`` and +never ``agentforge`` (ADR-0001). The accepted grammar is specified in +``GRAMMAR.md`` and versioned by ``QUERY_LANG_VERSION``. +""" + +from __future__ import annotations + +from .ast import QueryAst +from .capability import ( + ALL_CAPABILITIES, + CORE_TIER, + QueryCapable, + QuerySettings, + ResultTable, +) +from .errors import ( + CapabilityError, + GuardrailError, + ParseError, + QueryDisabled, + QueryError, + ValidationError, +) +from .parser import parse_query +from .schema import QUERY_LANG_VERSION, SchemaDescription, describe_schema +from .validator import validate_query + +__all__ = [ + # pipeline + "parse_query", + "validate_query", + "describe_schema", + # types + "QueryAst", + "QueryCapable", + "QuerySettings", + "ResultTable", + "SchemaDescription", + # capabilities + "CORE_TIER", + "ALL_CAPABILITIES", + "QUERY_LANG_VERSION", + # errors + "QueryError", + "ParseError", + "ValidationError", + "CapabilityError", + "GuardrailError", + "QueryDisabled", +] diff --git a/src/agentforge_graph/store/query/ast.py b/src/agentforge_graph/store/query/ast.py new file mode 100644 index 0000000..d728248 --- /dev/null +++ b/src/agentforge_graph/store/query/ast.py @@ -0,0 +1,153 @@ +"""The read-only query AST — the single trust boundary (feat-015). + +Caller text is parsed into these frozen dataclasses and nothing else runs. +Anything the AST cannot represent cannot reach a backend: there is no node for +a write, a procedure call, or an unbounded path, so those are unexpressible by +construction (the parser has no production for them, and the validator rejects +the few dangerous shapes the grammar *can* express, e.g. an unbounded +variable-length rel). + +Labels/kinds/property keys are kept as **raw strings** here — the parser only +proves *syntax*. The validator (``validator.py``) is what checks them against +the locked feat-001 vocabulary, so parse (syntax) and validate (meaning) stay +cleanly separated. +""" + +from __future__ import annotations + +from dataclasses import dataclass + +from agentforge_graph.core import Direction + +# A scalar literal value the caller wrote (string/int/float/bool/null). +type LitValue = str | int | float | bool | None + + +@dataclass(frozen=True) +class Lit: + """A literal value, wrapped so ``None`` is unambiguous vs "absent".""" + + value: LitValue + + +@dataclass(frozen=True) +class PropRef: + """A property access: ``f.name``, ``f.path``, ``n.attrs.role``.""" + + var: str + path: tuple[str, ...] # ("name",) | ("attrs", "role") + + +@dataclass(frozen=True) +class VarRef: + """A bare bound variable, e.g. ``RETURN f`` or ``count(f)``.""" + + var: str + + +# --- MATCH patterns --------------------------------------------------------- + + +@dataclass(frozen=True) +class NodePattern: + var: str | None + label: str | None # raw; validated against NodeKind later + props: tuple[tuple[str, Lit], ...] = () # inline {key: literal} equalities + + +@dataclass(frozen=True) +class RelPattern: + var: str | None + kind: str | None # raw; validated against EdgeKind later + direction: Direction # "out" (->) | "in" (<-) | "both" (-) + min_hops: int = 1 + max_hops: int | None = 1 # None => unbounded [*] => rejected by the validator + + +@dataclass(frozen=True) +class PathPattern: + """Alternating ``NodePattern (RelPattern NodePattern)*``.""" + + elements: tuple[NodePattern | RelPattern, ...] + + +# --- WHERE expression tree (a closed set) ----------------------------------- + + +@dataclass(frozen=True) +class Compare: + lhs: PropRef + op: str # = | <> | < | <= | > | >= + rhs: Lit + + +@dataclass(frozen=True) +class InList: + lhs: PropRef + values: tuple[Lit, ...] + + +@dataclass(frozen=True) +class StringPred: + lhs: PropRef + op: str # STARTS_WITH | ENDS_WITH | CONTAINS + rhs: str + + +@dataclass(frozen=True) +class Not: + operand: Expr + + +@dataclass(frozen=True) +class BoolOp: + op: str # AND | OR + operands: tuple[Expr, ...] + + +@dataclass(frozen=True) +class PatternExists: + """Existence of a path in a WHERE clause, e.g. ``NOT (f)<-[:CALLS]-()``.""" + + pattern: PathPattern + + +type Expr = Compare | InList | StringPred | Not | BoolOp | PatternExists + + +# --- RETURN / ORDER BY ------------------------------------------------------ + + +@dataclass(frozen=True) +class Aggregate: + func: str # count | collect | min | max | avg + arg: PropRef | VarRef | None # None == count(*) + distinct: bool = False + + +type ReturnExpr = PropRef | VarRef | Aggregate + + +@dataclass(frozen=True) +class ReturnItem: + expr: ReturnExpr + alias: str | None = None + + +@dataclass(frozen=True) +class OrderKey: + ref: PropRef | VarRef # a property or a bound var / RETURN alias + descending: bool = False + + +@dataclass(frozen=True) +class QueryAst: + """A fully-parsed read-only query.""" + + match: tuple[PathPattern, ...] + returns: tuple[ReturnItem, ...] + where: Expr | None = None + distinct: bool = False + order_by: tuple[OrderKey, ...] = () + skip: int | None = None + limit: int | None = None diff --git a/src/agentforge_graph/store/query/capability.py b/src/agentforge_graph/store/query/capability.py new file mode 100644 index 0000000..8875ed5 --- /dev/null +++ b/src/agentforge_graph/store/query/capability.py @@ -0,0 +1,77 @@ +"""Capability tiers + the optional ``QueryCapable`` backend protocol (feat-015). + +Every supported construct is tagged with a **capability**. A backend adapter +declares the set it can execute *identically* to the others. This replaces a +naive "intersection of all backends, forever" rule (which would cap a strong +backend at the weakest one) with a model that ships the same safe common core +today and grows additively: a construct outside a backend's declared set is +rejected *for that backend* with a precise error — never silently degraded, and +never removed from backends that do support it. The conformance suite guarantees +every construct is identical across the backends that claim it. + +The ``CORE_TIER`` is mandatory: every query-capable backend must support it and +prove identical results in conformance. That is the 0.6.4 subset. +""" + +from __future__ import annotations + +from dataclasses import dataclass +from typing import Any, ClassVar, Protocol, runtime_checkable + +from .ast import QueryAst + +# --- capability names ------------------------------------------------------- + +CORE = "core" # MATCH/WHERE/RETURN/ORDER/SKIP/LIMIT + comparisons + IN + booleans +AGG_BASIC = "agg.basic" # count / min / max / avg +PATTERN_EXISTS = "pattern.exists" # (a)-[:KIND]->(b) existence in WHERE +STRING_PRED = "string.pred" # STARTS WITH / ENDS WITH / CONTAINS +PATH_VARLEN = "path.varlen" # bounded variable-length rel, e.g. [:CALLS*1..3] + +# Optional (a backend MAY advertise these; not part of the mandatory core): +AGG_COLLECT = "agg.collect" # collect() list aggregation + +# Every query-capable backend MUST support these and prove identical results. +CORE_TIER: frozenset[str] = frozenset({CORE, AGG_BASIC, PATTERN_EXISTS, STRING_PRED, PATH_VARLEN}) + +# All capabilities the query language defines (core + optional). +ALL_CAPABILITIES: frozenset[str] = CORE_TIER | {AGG_COLLECT} + + +@dataclass(frozen=True) +class QuerySettings: + """Execution bounds, resolved from ``QueryConfig`` (chunk 7).""" + + max_rows: int = 1000 + timeout_ms: int = 5000 + max_expansions: int = 50_000 + + +@dataclass(frozen=True) +class ResultTable: + """A normalized, backend-independent columnar result. + + ``columns`` is fixed by the query's RETURN order at compile time, so a row + set is identical across backends. ``stopped_reason`` records *which* bound + truncated the result (no silent caps, per feat-008).""" + + columns: tuple[str, ...] + rows: tuple[tuple[Any, ...], ...] + truncated: bool = False + stopped_reason: str | None = None # None | "row_cap" | "timeout" | "expansion_cap" + + +@runtime_checkable +class QueryCapable(Protocol): + """A ``GraphStore`` adapter that can execute a validated ``QueryAst``. + + Optional — a backend without it reports ``query.enabled: false`` and still + serves the typed verbs (the locked ``GraphStore`` ABC is untouched). An + adapter opting in declares its ``query_dialect`` and ``capabilities`` and + implements ``run_query`` (execution + guardrails land in chunk 2).""" + + query_dialect: ClassVar[str] # "kuzu" | "neo4j" | "surrealql" + capabilities: ClassVar[frozenset[str]] # tiers this backend executes identically + read_only_execution: ClassVar[bool] # True if it enforces a read-only session (gate #2) + + async def run_query(self, ast: QueryAst, settings: QuerySettings) -> ResultTable: ... diff --git a/src/agentforge_graph/store/query/errors.py b/src/agentforge_graph/store/query/errors.py new file mode 100644 index 0000000..77adc6c --- /dev/null +++ b/src/agentforge_graph/store/query/errors.py @@ -0,0 +1,66 @@ +"""Errors for the read-only query surface (feat-015). + +One hierarchy, one trust boundary. Everything a caller can do wrong surfaces +as a ``QueryError`` subclass carrying a *why*, never a backend stack trace: + +- ``ParseError`` — the text is not in the accepted grammar (syntax). +- ``ValidationError`` — parses, but violates the vocabulary/exclusion rules. +- ``CapabilityError`` — valid, but the target backend does not offer the + capability tier the query needs (a ``ValidationError`` so callers can catch + either with one ``except``). +- ``GuardrailError`` — raised at execution time when a bound is hit in a way + that cannot be turned into a partial result (chunk 2). +- ``QueryDisabled`` — the active backend is not query-capable at all + (chunk 2, facade). +""" + +from __future__ import annotations + + +class QueryError(Exception): + """Base for every read-only-query error.""" + + +class ParseError(QueryError): + """The query text is not in the accepted Cypher-subset grammar.""" + + def __init__( + self, message: str, *, position: int | None = None, query: str | None = None + ) -> None: + self.position = position + self.query = query + if position is not None and query is not None: + super().__init__(f"{message} (at offset {position})\n {query}\n {' ' * position}^") + else: + super().__init__(message) + + +class ValidationError(QueryError): + """Parses, but breaks a vocabulary or exclusion rule.""" + + +class CapabilityError(ValidationError): + """Valid, but the target backend does not support a required capability.""" + + def __init__(self, capability: str, supported: frozenset[str]) -> None: + self.capability = capability + self.supported = supported + super().__init__( + f"this backend does not support '{capability}'; " + f"supported capabilities: {', '.join(sorted(supported)) or '(none)'}" + ) + + +class GuardrailError(QueryError): + """A resource bound was hit and could not be reported as a partial result.""" + + +class QueryDisabled(QueryError): + """The active storage backend is not query-capable.""" + + def __init__(self, driver: str) -> None: + self.driver = driver + super().__init__( + f"the '{driver}' backend does not provide a query surface " + f"(query.enabled is false for this store)" + ) diff --git a/src/agentforge_graph/store/query/parser.py b/src/agentforge_graph/store/query/parser.py new file mode 100644 index 0000000..bbc3ce3 --- /dev/null +++ b/src/agentforge_graph/store/query/parser.py @@ -0,0 +1,448 @@ +"""The Cypher-subset parser: text -> ``QueryAst`` (feat-015). + +Hand-written tokenizer + recursive-descent parser over the bounded grammar in +``GRAMMAR.md`` (no parser-library dependency — the base install stays lean). The +parser proves *syntax only*: it produces an AST with raw string labels/kinds and +never executes anything. Vocabulary and safety checks live in ``validator.py``. + +Write verbs (CREATE/MERGE/SET/DELETE/…), procedure ``CALL``, ``WITH``/``UNWIND`` +and friends have no production here, so they surface as a plain ``ParseError``. +""" + +from __future__ import annotations + +import re +from dataclasses import dataclass + +from agentforge_graph.core import Direction + +from .ast import ( + Aggregate, + BoolOp, + Compare, + Expr, + InList, + Lit, + NodePattern, + Not, + OrderKey, + PathPattern, + PatternExists, + PropRef, + QueryAst, + RelPattern, + ReturnExpr, + ReturnItem, + StringPred, + VarRef, +) +from .errors import ParseError + +# --- tokenizer -------------------------------------------------------------- + +_KEYWORDS = frozenset( + { + "MATCH", + "WHERE", + "RETURN", + "DISTINCT", + "AS", + "ORDER", + "BY", + "SKIP", + "LIMIT", + "AND", + "OR", + "NOT", + "IN", + "STARTS", + "ENDS", + "WITH", + "CONTAINS", + "ASC", + "DESC", + "TRUE", + "FALSE", + "NULL", + } +) +_AGG_FUNCS = frozenset({"count", "collect", "min", "max", "avg"}) + +_TOKEN_RE = re.compile( + r""" + (?P\s+) + | (?P'(?:\\.|[^'\\])*'|"(?:\\.|[^"\\])*") + | (?P\d+\.\d+) + | (?P\d+) + | (?P->) + | (?P<-) + | (?P<=) + | (?P>=) + | (?P<>) + | (?P[()\[\]{}:.,*=<>\-]) + | (?P[A-Za-z_][A-Za-z0-9_]*) + """, + re.VERBOSE, +) + + +@dataclass(frozen=True) +class _Token: + type: str + value: str + pos: int + + +def _tokenize(text: str) -> list[_Token]: + toks: list[_Token] = [] + pos = 0 + n = len(text) + while pos < n: + m = _TOKEN_RE.match(text, pos) + if m is None: + raise ParseError(f"unexpected character {text[pos]!r}", position=pos, query=text) + pos = m.end() + kind = m.lastgroup + assert kind is not None + value = m.group() + if kind == "WS": + continue + if kind == "IDENT" and value.upper() in _KEYWORDS: + toks.append(_Token(value.upper(), value, m.start())) + elif kind == "PUNCT": + toks.append(_Token(value, value, m.start())) # type == the char + else: + toks.append(_Token(kind, value, m.start())) + toks.append(_Token("EOF", "", n)) + return toks + + +_ESCAPES = {"n": "\n", "t": "\t", "r": "\r"} + + +def _unquote(raw: str) -> str: + return re.sub(r"\\(.)", lambda mo: _ESCAPES.get(mo.group(1), mo.group(1)), raw[1:-1]) + + +# --- parser ----------------------------------------------------------------- + + +class _Parser: + def __init__(self, text: str, tokens: list[_Token]) -> None: + self.text = text + self.toks = tokens + self.i = 0 + + # cursor helpers + def _cur(self) -> _Token: + return self.toks[self.i] + + def _at(self, type_: str) -> bool: + return self.toks[self.i].type == type_ + + def _next_is(self, type_: str) -> bool: + return self.toks[self.i + 1].type == type_ + + def _advance(self) -> _Token: + tok = self.toks[self.i] + self.i += 1 + return tok + + def _accept(self, type_: str) -> _Token | None: + return self._advance() if self._at(type_) else None + + def _expect(self, type_: str, what: str) -> _Token: + tok = self.toks[self.i] + if tok.type != type_: + found = "end of input" if tok.type == "EOF" else repr(tok.value) + raise ParseError(f"expected {what}, found {found}", position=tok.pos, query=self.text) + return self._advance() + + # query := MATCH pattern (, pattern)* [WHERE expr] RETURN [DISTINCT] item (, item)* + # [ORDER BY key (, key)*] [SKIP int] [LIMIT int] + def parse(self) -> QueryAst: + self._expect("MATCH", "'MATCH'") + patterns = [self._pattern()] + while self._accept(","): + patterns.append(self._pattern()) + + where: Expr | None = None + if self._accept("WHERE"): + where = self._expr() + + self._expect("RETURN", "'RETURN'") + distinct = self._accept("DISTINCT") is not None + returns = [self._return_item()] + while self._accept(","): + returns.append(self._return_item()) + + order_by: list[OrderKey] = [] + if self._accept("ORDER"): + self._expect("BY", "'BY'") + order_by.append(self._order_key()) + while self._accept(","): + order_by.append(self._order_key()) + + skip = int(self._expect("INT", "an integer").value) if self._accept("SKIP") else None + limit = int(self._expect("INT", "an integer").value) if self._accept("LIMIT") else None + + self._expect("EOF", "end of query") + return QueryAst( + match=tuple(patterns), + returns=tuple(returns), + where=where, + distinct=distinct, + order_by=tuple(order_by), + skip=skip, + limit=limit, + ) + + # pattern := nodePat (relPat nodePat)* + def _pattern(self) -> PathPattern: + elements: list[NodePattern | RelPattern] = [self._node_pattern()] + while self._at("-") or self._at("ARROW_L"): + rel = self._rel_pattern() + node = self._node_pattern() + elements.append(rel) + elements.append(node) + return PathPattern(tuple(elements)) + + # nodePat := "(" [var] [":" Label] [ "{" propEq (, propEq)* "}" ] ")" + def _node_pattern(self) -> NodePattern: + self._expect("(", "'('") + var = self._accept("IDENT") + label = None + if self._accept(":"): + label = self._expect("IDENT", "a node label").value + props: list[tuple[str, Lit]] = [] + if self._accept("{"): + props.append(self._prop_eq()) + while self._accept(","): + props.append(self._prop_eq()) + self._expect("}", "'}'") + self._expect(")", "')'") + return NodePattern(var.value if var else None, label, tuple(props)) + + def _prop_eq(self) -> tuple[str, Lit]: + key = self._expect("IDENT", "a property name").value + self._expect(":", "':'") + return (key, self._literal()) + + # relPat := ("<-"|"-") [ "[" [var] [":" KIND] [varlen] "]" ] ("->"|"-") + def _rel_pattern(self) -> RelPattern: + left = self._accept("ARROW_L") is not None + if not left: + self._expect("-", "'-' or '<-'") + + var: str | None = None + kind: str | None = None + min_hops: int = 1 + max_hops: int | None = 1 + if self._accept("["): + v = self._accept("IDENT") + var = v.value if v else None + if self._accept(":"): + kind = self._expect("IDENT", "a relationship type").value + if self._accept("*"): + min_hops, max_hops = self._varlen() + self._expect("]", "']'") + + right = self._accept("ARROW_R") is not None + if not right: + self._expect("-", "'-' or '->'") + + if left and right: + raise ParseError( + "a relationship cannot point both ways ('<-...->')", + position=self._cur().pos, + query=self.text, + ) + direction: Direction = "in" if left else ("out" if right else "both") + return RelPattern(var, kind, direction, min_hops, max_hops) + + # varlen (after the "*"): "" | INT | ".." INT? | INT ".." INT? + def _varlen(self) -> tuple[int, int | None]: + lo = int(self._advance().value) if self._at("INT") else None + if self._accept("."): # a ".." range (two DOT tokens) + self._expect(".", "'..'") + hi = int(self._advance().value) if self._at("INT") else None + return (lo if lo is not None else 1, hi) + # no range: bare "*" (unbounded) or "*n" (exactly n) + if lo is None: + return (1, None) + return (lo, lo) + + # --- WHERE expression --------------------------------------------------- + + def _expr(self) -> Expr: + return self._or() + + def _or(self) -> Expr: + operands = [self._and()] + while self._accept("OR"): + operands.append(self._and()) + return operands[0] if len(operands) == 1 else BoolOp("OR", tuple(operands)) + + def _and(self) -> Expr: + operands = [self._not()] + while self._accept("AND"): + operands.append(self._not()) + return operands[0] if len(operands) == 1 else BoolOp("AND", tuple(operands)) + + def _not(self) -> Expr: + if self._accept("NOT"): + return Not(self._not()) + return self._primary() + + def _primary(self) -> Expr: + if self._at("("): + # "(" is ambiguous: a grouped expr "(a.x = 1)" vs a pattern-existence + # predicate "(a)-[:X]->(b)". Try the pattern; a real path (>= 3 + # elements: node rel node) is PatternExists, otherwise backtrack. + save = self.i + try: + pat = self._pattern() + if len(pat.elements) >= 3: + return PatternExists(pat) + except ParseError: + pass + self.i = save + self._expect("(", "'('") + inner = self._expr() + self._expect(")", "')'") + return inner + return self._predicate() + + def _predicate(self) -> Expr: + lhs = self._prop_ref() + if self._accept("IN"): + self._expect("[", "'['") + values = [self._literal()] + while self._accept(","): + values.append(self._literal()) + self._expect("]", "']'") + return InList(lhs, tuple(values)) + if self._accept("STARTS"): + self._expect("WITH", "'WITH'") + return StringPred( + lhs, "STARTS_WITH", _unquote(self._expect("STRING", "a string").value) + ) + if self._accept("ENDS"): + self._expect("WITH", "'WITH'") + return StringPred(lhs, "ENDS_WITH", _unquote(self._expect("STRING", "a string").value)) + if self._accept("CONTAINS"): + return StringPred(lhs, "CONTAINS", _unquote(self._expect("STRING", "a string").value)) + return Compare(lhs, self._compare_op(), self._literal()) + + def _compare_op(self) -> str: + for type_, op in ( + ("=", "="), + ("NE", "<>"), + ("LE", "<="), + ("GE", ">="), + ("<", "<"), + (">", ">"), + ): + if self._accept(type_): + return op + tok = self._cur() + raise ParseError( + f"expected a comparison operator, found {tok.value!r}", + position=tok.pos, + query=self.text, + ) + + def _prop_ref(self) -> PropRef: + var = self._expect("IDENT", "a variable").value + self._expect(".", "'.' (property access)") + segs = [self._expect("IDENT", "a property name").value] + while self._accept("."): + segs.append(self._expect("IDENT", "a property name").value) + return PropRef(var, tuple(segs)) + + # --- RETURN / ORDER BY -------------------------------------------------- + + def _return_item(self) -> ReturnItem: + expr = self._return_expr() + alias = self._expect("IDENT", "an alias").value if self._accept("AS") else None + return ReturnItem(expr, alias) + + def _return_expr(self) -> ReturnExpr: + if self._at("IDENT") and self._cur().value.lower() in _AGG_FUNCS and self._next_is("("): + return self._aggregate() + return self._prop_or_var() + + def _aggregate(self) -> Aggregate: + func = self._advance().value.lower() + self._expect("(", "'('") + distinct = self._accept("DISTINCT") is not None + arg: PropRef | VarRef | None + if self._accept("*"): + if func != "count": + tok = self._cur() + raise ParseError( + f"only count(*) is allowed, not {func}(*)", position=tok.pos, query=self.text + ) + arg = None + else: + arg = self._prop_or_var() + self._expect(")", "')'") + return Aggregate(func, arg, distinct) + + def _prop_or_var(self) -> PropRef | VarRef: + var = self._expect("IDENT", "a variable").value + if self._accept("."): + segs = [self._expect("IDENT", "a property name").value] + while self._accept("."): + segs.append(self._expect("IDENT", "a property name").value) + return PropRef(var, tuple(segs)) + return VarRef(var) + + def _order_key(self) -> OrderKey: + ref = self._prop_or_var() + descending = False + if self._accept("ASC"): + descending = False + elif self._accept("DESC"): + descending = True + return OrderKey(ref, descending) + + # --- literals ----------------------------------------------------------- + + def _literal(self) -> Lit: + negative = self._accept("-") is not None + if self._at("INT"): + v = int(self._advance().value) + return Lit(-v if negative else v) + if self._at("FLOAT"): + f = float(self._advance().value) + return Lit(-f if negative else f) + if negative: + tok = self._cur() + raise ParseError( + f"expected a number after '-', found {tok.value!r}", + position=tok.pos, + query=self.text, + ) + if self._at("STRING"): + return Lit(_unquote(self._advance().value)) + if self._accept("TRUE"): + return Lit(True) + if self._accept("FALSE"): + return Lit(False) + if self._accept("NULL"): + return Lit(None) + tok = self._cur() + raise ParseError( + f"expected a literal, found {tok.value!r}", position=tok.pos, query=self.text + ) + + +def parse_query(text: str) -> QueryAst: + """Parse Cypher-subset text into a ``QueryAst`` (syntax only). + + Raises ``ParseError`` with the offending offset on malformed input. The + result is *not yet validated* — call ``validate_query`` next.""" + if not text.strip(): + raise ParseError("empty query") + return _Parser(text, _tokenize(text)).parse() diff --git a/src/agentforge_graph/store/query/schema.py b/src/agentforge_graph/store/query/schema.py new file mode 100644 index 0000000..fbdea8a --- /dev/null +++ b/src/agentforge_graph/store/query/schema.py @@ -0,0 +1,104 @@ +"""The queryable schema — node/edge kinds + the curated property catalogue. + +This is the vocabulary a caller writes against, and the single source of truth +that (a) the validator uses to accept/reject property references and (b) the +per-backend compilers (chunk 2) use to map a logical property name to its +physical column. The physical columns are the *shared* row schema every +property-graph adapter persists (``store/_rowmap.py``): ``name``, ``kind``, +``sym_path``, ``span_start``/``span_end`` and the ``prov_*`` provenance columns. +Because that mapping is identical across Kuzu/Neo4j/SurrealDB, ``f.path`` & +friends resolve the same way on every backend — portability by construction. + +Anything not in the curated set is addressable as ``n.attrs.`` — an opaque +passthrough over the node's free-form ``attrs`` JSON (compared as a string). +""" + +from __future__ import annotations + +from dataclasses import dataclass +from typing import Any + +from agentforge_graph.core import EdgeKind, NodeKind + +# Bumped when the accepted grammar/semantics change (minor = additive clause, +# major = semantics change). Reported by ``ckg status`` so long-lived clients +# detect a mismatch. Tracks store/query/GRAMMAR.md. +QUERY_LANG_VERSION = "1.0" + +# Prefix for opaque free-form attribute access: ``n.attrs.``. +ATTRS_PREFIX = "attrs" + + +@dataclass(frozen=True) +class PropertySpec: + """A curated, queryable node property and its physical backing column.""" + + name: str # logical name written in a query (the ``x`` in ``f.x``) + column: str # physical column in the shared row schema (_rowmap.py) + type: str # "str" | "int" | "float" + doc: str + + +# The curated node properties, mapped to the shared physical row schema. Order +# is display order for ``ckg query --schema``. +NODE_PROPERTIES: tuple[PropertySpec, ...] = ( + PropertySpec("name", "name", "str", "the symbol's name"), + PropertySpec("kind", "kind", "str", "the node kind (also filterable as a :Label)"), + PropertySpec("path", "sym_path", "str", "repo-relative file path of the symbol"), + PropertySpec("start_line", "span_start", "int", "1-based start line of the symbol's span"), + PropertySpec("end_line", "span_end", "int", "1-based end line of the symbol's span"), + PropertySpec("source", "prov_source", "str", "provenance: parsed | resolved | llm | manual"), + PropertySpec("extractor", "prov_extractor", "str", "producer name + version"), + PropertySpec("commit", "prov_commit", "str", "git sha the fact was derived at"), + PropertySpec("confidence", "prov_confidence", "float", "provenance confidence in [0.0, 1.0]"), +) + +PROPERTY_BY_NAME: dict[str, PropertySpec] = {p.name: p for p in NODE_PROPERTIES} + + +def is_known_property(path: tuple[str, ...]) -> bool: + """True if a property path is addressable: a curated single-segment name, + or an ``attrs.`` opaque path (at least one key after ``attrs``).""" + if len(path) == 1: + return path[0] in PROPERTY_BY_NAME + return path[0] == ATTRS_PREFIX and len(path) >= 2 + + +def is_attrs_ref(path: tuple[str, ...]) -> bool: + """True for an opaque ``attrs.`` reference.""" + return len(path) >= 2 and path[0] == ATTRS_PREFIX + + +@dataclass(frozen=True) +class SchemaDescription: + """The full queryable vocabulary, for ``ckg query --schema`` / introspection.""" + + node_kinds: tuple[str, ...] + edge_kinds: tuple[str, ...] + node_properties: tuple[PropertySpec, ...] + attrs_note: str + lang_version: str + + def to_dict(self) -> dict[str, Any]: + return { + "query_lang_version": self.lang_version, + "node_kinds": list(self.node_kinds), + "edge_kinds": list(self.edge_kinds), + "node_properties": [ + {"name": p.name, "type": p.type, "doc": p.doc} for p in self.node_properties + ], + "attrs": self.attrs_note, + } + + +def describe_schema() -> SchemaDescription: + """The documented vocabulary callers query against.""" + return SchemaDescription( + node_kinds=tuple(k.value for k in NodeKind), + edge_kinds=tuple(k.value for k in EdgeKind), + node_properties=NODE_PROPERTIES, + attrs_note=( + "any other attribute is addressable as n.attrs. (opaque, compared as a string)" + ), + lang_version=QUERY_LANG_VERSION, + ) diff --git a/src/agentforge_graph/store/query/validator.py b/src/agentforge_graph/store/query/validator.py new file mode 100644 index 0000000..1876518 --- /dev/null +++ b/src/agentforge_graph/store/query/validator.py @@ -0,0 +1,250 @@ +"""The validator — the second (semantic) half of the trust boundary (feat-015). + +The parser proves syntax; this proves *meaning* against the locked feat-001 +vocabulary and the read-only exclusion rules, then (phase 2) against the target +backend's declared capabilities. A query that survives here is safe to compile. + +Two phases, run in order: + +1. **Backend-independent** — labels ∈ ``NodeKind``, rel types ∈ ``EdgeKind``, + property refs are curated names or ``attrs.*``, every referenced variable is + bound, no unbounded variable-length path, no un-joined Cartesian product. +2. **Capability** — each construct maps to a capability tier; a construct the + target backend does not declare raises ``CapabilityError`` (a + ``ValidationError`` subclass) naming what is supported. This is what lets the + subset grow per-backend without ever silently diverging. +""" + +from __future__ import annotations + +from collections.abc import Iterator + +from agentforge_graph.core import EdgeKind, NodeKind + +from .ast import ( + Aggregate, + BoolOp, + Compare, + Expr, + InList, + NodePattern, + Not, + PathPattern, + PatternExists, + PropRef, + QueryAst, + RelPattern, + StringPred, + VarRef, +) +from .capability import ( + AGG_BASIC, + AGG_COLLECT, + CORE_TIER, + PATH_VARLEN, + PATTERN_EXISTS, + STRING_PRED, +) +from .errors import CapabilityError, ValidationError +from .schema import PROPERTY_BY_NAME, is_known_property + +_NODE_KIND_VALUES = frozenset(k.value for k in NodeKind) +_EDGE_KIND_VALUES = frozenset(k.value for k in EdgeKind) + + +def validate_query(ast: QueryAst, capabilities: frozenset[str] = CORE_TIER) -> QueryAst: + """Validate an AST against the vocabulary, the exclusion rules, and the + target backend's ``capabilities``. Returns the AST unchanged on success; + raises ``ValidationError`` / ``CapabilityError`` otherwise.""" + _check_vocabulary(ast) + _check_exclusions(ast) + _check_capabilities(ast, capabilities) + return ast + + +# --- iteration helpers ------------------------------------------------------ + + +def _all_patterns(ast: QueryAst) -> Iterator[PathPattern]: + yield from ast.match + if ast.where is not None: + for e in _walk_expr(ast.where): + if isinstance(e, PatternExists): + yield e.pattern + + +def _walk_expr(expr: Expr) -> Iterator[Expr]: + yield expr + if isinstance(expr, BoolOp): + for operand in expr.operands: + yield from _walk_expr(operand) + elif isinstance(expr, Not): + yield from _walk_expr(expr.operand) + + +def _rels(ast: QueryAst) -> Iterator[RelPattern]: + for pat in _all_patterns(ast): + for el in pat.elements: + if isinstance(el, RelPattern): + yield el + + +def _nodes(ast: QueryAst) -> Iterator[NodePattern]: + for pat in _all_patterns(ast): + for el in pat.elements: + if isinstance(el, NodePattern): + yield el + + +def _where_prop_refs(ast: QueryAst) -> Iterator[PropRef]: + if ast.where is None: + return + for e in _walk_expr(ast.where): + if isinstance(e, (Compare, InList, StringPred)): + yield e.lhs + + +def _return_prop_refs(ast: QueryAst) -> Iterator[PropRef]: + for item in ast.returns: + expr = item.expr + if isinstance(expr, PropRef): + yield expr + elif isinstance(expr, Aggregate) and isinstance(expr.arg, PropRef): + yield expr.arg + + +# --- phase 1: vocabulary ---------------------------------------------------- + + +def _check_vocabulary(ast: QueryAst) -> None: + for node in _nodes(ast): + if node.label is not None and node.label not in _NODE_KIND_VALUES: + raise ValidationError( + f"unknown node label ':{node.label}'. Valid kinds: " + f"{', '.join(sorted(_NODE_KIND_VALUES))}" + ) + for key, _ in node.props: + if key not in PROPERTY_BY_NAME: + raise ValidationError( + f"unknown inline property '{key}'. Queryable properties: " + f"{', '.join(sorted(PROPERTY_BY_NAME))} (or attrs.)" + ) + for rel in _rels(ast): + if rel.kind is not None and rel.kind not in _EDGE_KIND_VALUES: + raise ValidationError( + f"unknown relationship type ':{rel.kind}'. Valid kinds: " + f"{', '.join(sorted(_EDGE_KIND_VALUES))}" + ) + + bound = _bound_vars(ast) + return_aliases = {item.alias for item in ast.returns if item.alias} + for ref in (*_where_prop_refs(ast), *_return_prop_refs(ast)): + _check_property(ref) + _require_bound(ref.var, bound) + # RETURN/ORDER bare vars must be bound; ORDER may also name a RETURN alias. + for item in ast.returns: + if isinstance(item.expr, VarRef): + _require_bound(item.expr.var, bound) + elif isinstance(item.expr, Aggregate) and isinstance(item.expr.arg, VarRef): + _require_bound(item.expr.arg.var, bound) + for order_key in ast.order_by: + if isinstance(order_key.ref, PropRef): + _check_property(order_key.ref) + _require_bound(order_key.ref.var, bound) + elif order_key.ref.var not in bound and order_key.ref.var not in return_aliases: + raise ValidationError( + f"ORDER BY references '{order_key.ref.var}', which is not a bound " + f"variable or a RETURN alias" + ) + + +def _require_bound(var: str, bound: set[str]) -> None: + if var not in bound: + raise ValidationError(f"variable '{var}' is not bound in the MATCH clause") + + +def _check_property(ref: PropRef) -> None: + if not is_known_property(ref.path): + raise ValidationError( + f"unknown property '{ref.var}.{'.'.join(ref.path)}'. Queryable properties: " + f"{', '.join(sorted(PROPERTY_BY_NAME))} (or attrs.)" + ) + + +def _bound_vars(ast: QueryAst) -> set[str]: + bound: set[str] = set() + for pat in _all_patterns(ast): + for el in pat.elements: + if el.var is not None: + bound.add(el.var) + return bound + + +# --- phase 1: exclusions ---------------------------------------------------- + + +def _check_exclusions(ast: QueryAst) -> None: + for rel in _rels(ast): + if rel.max_hops is None: + raise ValidationError( + "unbounded variable-length path is not allowed; give an upper bound, " + "e.g. [:CALLS*1..3]" + ) + if rel.min_hops < 1 or rel.max_hops < rel.min_hops: + raise ValidationError( + f"invalid path length *{rel.min_hops}..{rel.max_hops}: need 1 <= min <= max" + ) + if ast.skip is not None and ast.skip < 0: + raise ValidationError("SKIP must be non-negative") + if ast.limit is not None and ast.limit < 0: + raise ValidationError("LIMIT must be non-negative") + _check_cartesian(ast) + + +def _check_cartesian(ast: QueryAst) -> None: + # Multiple top-level MATCH patterns must form one connected graph via shared + # variables; otherwise they cross-product. A WHERE cannot join them — a + # comparison's right side is a literal (parameterized), never another + # property — so connectivity is checked structurally, not via WHERE. + if len(ast.match) < 2: + return + var_sets = [{el.var for el in pat.elements if el.var is not None} for pat in ast.match] + parent = list(range(len(var_sets))) + + def find(x: int) -> int: + while parent[x] != x: + parent[x] = parent[parent[x]] + x = parent[x] + return x + + for i in range(len(var_sets)): + for j in range(i + 1, len(var_sets)): + if var_sets[i] & var_sets[j]: + parent[find(i)] = find(j) + if len({find(i) for i in range(len(var_sets))}) > 1: + raise ValidationError( + "disconnected MATCH patterns form a Cartesian product; connect them " + "with a shared variable" + ) + + +# --- phase 2: capability ---------------------------------------------------- + + +def _check_capabilities(ast: QueryAst, capabilities: frozenset[str]) -> None: + required: set[str] = set() + for rel in _rels(ast): + if (rel.min_hops, rel.max_hops) != (1, 1): + required.add(PATH_VARLEN) + if ast.where is not None: + for e in _walk_expr(ast.where): + if isinstance(e, StringPred): + required.add(STRING_PRED) + elif isinstance(e, PatternExists): + required.add(PATTERN_EXISTS) + for item in ast.returns: + if isinstance(item.expr, Aggregate): + required.add(AGG_COLLECT if item.expr.func == "collect" else AGG_BASIC) + for cap in sorted(required): + if cap not in capabilities: + raise CapabilityError(cap, capabilities) diff --git a/tests/store/query/__init__.py b/tests/store/query/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/store/query/test_parser.py b/tests/store/query/test_parser.py new file mode 100644 index 0000000..d224363 --- /dev/null +++ b/tests/store/query/test_parser.py @@ -0,0 +1,198 @@ +"""Parser unit tests (feat-015 chunk 1): text -> QueryAst, syntax only. + +Every supported construct parses to the expected AST; write verbs and other +out-of-grammar text raise ParseError. Vocabulary is *not* checked here (that is +the validator's job) — these tests assert shape, not meaning. +""" + +from __future__ import annotations + +import pytest + +from agentforge_graph.store.query import ParseError, parse_query +from agentforge_graph.store.query.ast import ( + Aggregate, + BoolOp, + Compare, + InList, + Lit, + NodePattern, + Not, + PatternExists, + PropRef, + RelPattern, + StringPred, + VarRef, +) + + +def test_minimal_match_return() -> None: + ast = parse_query("MATCH (f:Function) RETURN f.name") + assert len(ast.match) == 1 + (node,) = ast.match[0].elements + assert node == NodePattern("f", "Function", ()) + assert ast.returns[0].expr == PropRef("f", ("name",)) + assert ast.returns[0].alias is None + + +def test_relationship_direction_out() -> None: + ast = parse_query("MATCH (a:Class)-[:IMPLEMENTS]->(b:Interface) RETURN a.name") + a, rel, b = ast.match[0].elements + assert isinstance(rel, RelPattern) + assert rel.kind == "IMPLEMENTS" and rel.direction == "out" + assert isinstance(a, NodePattern) and isinstance(b, NodePattern) + + +def test_relationship_direction_in_and_anon_node() -> None: + ast = parse_query("MATCH (f:Function)<-[:CALLS]-() RETURN f.name") + f, rel, anon = ast.match[0].elements + assert isinstance(rel, RelPattern) and rel.direction == "in" and rel.kind == "CALLS" + assert anon == NodePattern(None, None, ()) + + +def test_undirected_relationship() -> None: + ast = parse_query("MATCH (a)-[:REFERENCES]-(b) RETURN a.name") + _, rel, _ = ast.match[0].elements + assert isinstance(rel, RelPattern) and rel.direction == "both" + + +@pytest.mark.parametrize( + "text,expected", + [ + ("MATCH (f:Function)-[:CALLS*1..3]->(g) RETURN g.name", (1, 3)), + ("MATCH (f:Function)-[:CALLS*2]->(g) RETURN g.name", (2, 2)), + ("MATCH (f:Function)-[:CALLS*..4]->(g) RETURN g.name", (1, 4)), + ], +) +def test_bounded_varlen(text: str, expected: tuple[int, int]) -> None: + _, rel, _ = parse_query(text).match[0].elements + assert isinstance(rel, RelPattern) + assert (rel.min_hops, rel.max_hops) == expected + + +def test_unbounded_varlen_parses_with_none_maxhops() -> None: + # The parser accepts it (syntax); the validator is what rejects it. + _, rel, _ = parse_query("MATCH (f)-[:CALLS*]->(g) RETURN g.name").match[0].elements + assert isinstance(rel, RelPattern) and rel.max_hops is None + + +def test_inline_props() -> None: + ast = parse_query('MATCH (c:Class {name: "Repo"}) RETURN c.name') + (node,) = ast.match[0].elements + assert isinstance(node, NodePattern) + assert node.props == (("name", Lit("Repo")),) + + +def test_where_comparisons_and_booleans() -> None: + ast = parse_query( + 'MATCH (f:Function) WHERE f.confidence >= 0.5 AND f.name <> "x" RETURN f.name' + ) + assert isinstance(ast.where, BoolOp) and ast.where.op == "AND" + left, right = ast.where.operands + assert left == Compare(PropRef("f", ("confidence",)), ">=", Lit(0.5)) + assert right == Compare(PropRef("f", ("name",)), "<>", Lit("x")) + + +def test_where_in_list() -> None: + ast = parse_query('MATCH (n:Class) WHERE n.source IN ["parsed", "resolved"] RETURN n.name') + assert ast.where == InList(PropRef("n", ("source",)), (Lit("parsed"), Lit("resolved"))) + + +def test_where_string_predicates() -> None: + ast = parse_query('MATCH (f:Function) WHERE f.path STARTS WITH "src/" RETURN f.name') + assert ast.where == StringPred(PropRef("f", ("path",)), "STARTS_WITH", "src/") + + +def test_where_not_pattern_exists() -> None: + ast = parse_query("MATCH (f:Function) WHERE NOT (f)<-[:CALLS]-() RETURN f.name, f.path") + assert isinstance(ast.where, Not) + assert isinstance(ast.where.operand, PatternExists) + # two projections + assert [i.expr for i in ast.returns] == [PropRef("f", ("name",)), PropRef("f", ("path",))] + + +def test_grouped_expr_not_mistaken_for_pattern() -> None: + ast = parse_query('MATCH (f:Function) WHERE (f.name = "a" OR f.name = "b") RETURN f.name') + assert isinstance(ast.where, BoolOp) and ast.where.op == "OR" + + +def test_attrs_property_path() -> None: + ast = parse_query('MATCH (s:Service) WHERE s.attrs.framework = "fastapi" RETURN s.name') + assert ast.where == Compare(PropRef("s", ("attrs", "framework")), "=", Lit("fastapi")) + + +def test_aggregate_with_alias_and_order_limit() -> None: + ast = parse_query( + "MATCH (c:Class)-[:IMPLEMENTS]->(i:Interface) " + "RETURN i.name, count(c) AS impls ORDER BY impls DESC LIMIT 10" + ) + agg_item = ast.returns[1] + assert agg_item.expr == Aggregate("count", VarRef("c"), False) + assert agg_item.alias == "impls" + assert ast.order_by[0].descending is True + assert ast.limit == 10 + + +def test_count_star_and_distinct() -> None: + ast = parse_query("MATCH (f:Function) RETURN DISTINCT count(*) AS n") + assert ast.distinct is True + assert ast.returns[0].expr == Aggregate("count", None, False) + + +def test_skip_and_limit() -> None: + ast = parse_query("MATCH (f:Function) RETURN f.name SKIP 5 LIMIT 10") + assert ast.skip == 5 and ast.limit == 10 + + +def test_negative_number_literal() -> None: + ast = parse_query("MATCH (n:Chunk) WHERE n.start_line > -1 RETURN n.name") + assert ast.where == Compare(PropRef("n", ("start_line",)), ">", Lit(-1)) + + +def test_multiple_comma_patterns() -> None: + ast = parse_query('MATCH (a:Class), (b:Interface) WHERE a.name = "x" RETURN a.name') + assert len(ast.match) == 2 + + +@pytest.mark.parametrize( + "text", + [ + 'CREATE (n:Class {name: "x"}) RETURN n.name', + "MATCH (n:Class) SET n.name = 'y' RETURN n.name", + "MATCH (n:Class) DELETE n", + "MATCH (n:Class) DETACH DELETE n", + "MATCH (n:Class) REMOVE n.name RETURN n", + "CALL db.labels()", + "MATCH (n:Class) WITH n RETURN n.name", + "UNWIND [1,2,3] AS x RETURN x", + "MERGE (n:Class) RETURN n", + ], +) +def test_write_and_out_of_grammar_rejected(text: str) -> None: + with pytest.raises(ParseError): + parse_query(text) + + +@pytest.mark.parametrize( + "text", + [ + "", + " ", + "RETURN 1", # no MATCH + "MATCH (f:Function)", # no RETURN + "MATCH (f RETURN f.name", # unbalanced paren + "MATCH (f:Function) RETURN f.", # dangling property + "MATCH (f:Function)-[:CALLS>(g) RETURN g.name", # malformed rel + "MATCH (f:Function) RETURN f.name LIMIT", # missing int + "MATCH (a)<-[:X]->(b) RETURN a.name", # both-way arrow + ], +) +def test_malformed_queries_rejected(text: str) -> None: + with pytest.raises(ParseError): + parse_query(text) + + +def test_parse_error_carries_position() -> None: + with pytest.raises(ParseError) as exc: + parse_query("MATCH (f:Function) RETURN f.name LIMIT xyz") + assert exc.value.position is not None diff --git a/tests/store/query/test_schema.py b/tests/store/query/test_schema.py new file mode 100644 index 0000000..78b3db1 --- /dev/null +++ b/tests/store/query/test_schema.py @@ -0,0 +1,69 @@ +"""Schema-introspection unit tests (feat-015 chunk 1). + +The curated property catalogue must stay aligned with the shared physical row +schema (store/_rowmap.py) so every logical property maps to a real column on +every backend — this is what makes ``f.path`` portable. +""" + +from __future__ import annotations + +from agentforge_graph.core import EdgeKind, NodeKind +from agentforge_graph.store._rowmap import node_params +from agentforge_graph.store.query.schema import ( + NODE_PROPERTIES, + QUERY_LANG_VERSION, + describe_schema, + is_attrs_ref, + is_known_property, +) + + +def test_describe_schema_lists_all_kinds() -> None: + desc = describe_schema() + assert set(desc.node_kinds) == {k.value for k in NodeKind} + assert set(desc.edge_kinds) == {k.value for k in EdgeKind} + assert desc.lang_version == QUERY_LANG_VERSION + + +def test_to_dict_is_json_shaped() -> None: + d = describe_schema().to_dict() + assert d["query_lang_version"] == QUERY_LANG_VERSION + assert "node_kinds" in d and "edge_kinds" in d + assert all({"name", "type", "doc"} <= set(p) for p in d["node_properties"]) + + +def test_every_property_maps_to_a_real_physical_column() -> None: + # Build a representative node row and assert every curated property's backing + # column actually exists in the shared row schema — no property promises a + # column the backends don't persist. + fixture = _sample_row() + for spec in NODE_PROPERTIES: + assert spec.column in fixture, f"{spec.name} -> {spec.column} missing from row schema" + + +def test_is_known_property() -> None: + assert is_known_property(("name",)) + assert is_known_property(("path",)) + assert is_known_property(("attrs", "framework")) + assert is_known_property(("attrs", "a", "b")) + assert not is_known_property(("bogus",)) + assert not is_known_property(("attrs",)) # needs a key after attrs + + +def test_is_attrs_ref() -> None: + assert is_attrs_ref(("attrs", "x")) + assert not is_attrs_ref(("name",)) + assert not is_attrs_ref(("attrs",)) + + +def _sample_row() -> dict[str, object]: + from agentforge_graph.core import Descriptor, Node, NodeKind, Provenance, SymbolID + + node = Node( + id=SymbolID.for_symbol("py", "repo", "pkg/mod.py", Descriptor.term("func")), + kind=NodeKind.FUNCTION, + name="func", + span=(1, 10), + provenance=Provenance.parsed("tree-sitter-python@0.23"), + ) + return node_params(node, origin_path="pkg/mod.py") diff --git a/tests/store/query/test_validator.py b/tests/store/query/test_validator.py new file mode 100644 index 0000000..034f820 --- /dev/null +++ b/tests/store/query/test_validator.py @@ -0,0 +1,158 @@ +"""Validator unit tests (feat-015 chunk 1): the trust boundary. + +Phase 1 (backend-independent): vocabulary + exclusions. Phase 2: capability +tiers. A valid query returns unchanged; every rule has a rejecting case. +""" + +from __future__ import annotations + +import pytest + +from agentforge_graph.store.query import ( + CapabilityError, + ValidationError, + parse_query, + validate_query, +) +from agentforge_graph.store.query.capability import ( + AGG_COLLECT, + CORE_TIER, +) + + +def _v(text: str, capabilities: frozenset[str] = CORE_TIER) -> None: + validate_query(parse_query(text), capabilities) + + +# --- valid queries ---------------------------------------------------------- + + +@pytest.mark.parametrize( + "text", + [ + "MATCH (f:Function) RETURN f.name", + 'MATCH (c:Class {name: "Repo"}) RETURN c.path', + "MATCH (a:Class)-[:IMPLEMENTS]->(i:Interface) RETURN i.name, count(a) AS n ORDER BY n DESC", + "MATCH (f:Function) WHERE NOT (f)<-[:CALLS]-() RETURN f.name", + 'MATCH (s:Service) WHERE s.attrs.framework = "fastapi" RETURN s.name', + "MATCH (f:Function)-[:CALLS*1..3]->(g:Function) RETURN g.name", + "MATCH (a:Class)-[:IMPLEMENTS]->(i:Interface), (a)-[:INHERITS]->(b:Class) RETURN a.name", + ], +) +def test_valid_queries_pass(text: str) -> None: + _v(text) # does not raise + + +def test_returns_ast_unchanged() -> None: + ast = parse_query("MATCH (f:Function) RETURN f.name") + assert validate_query(ast) is ast + + +# --- vocabulary ------------------------------------------------------------- + + +def test_unknown_node_label_rejected() -> None: + with pytest.raises(ValidationError, match="unknown node label"): + _v("MATCH (x:Widget) RETURN x.name") + + +def test_unknown_edge_kind_rejected() -> None: + with pytest.raises(ValidationError, match="unknown relationship type"): + _v("MATCH (a:Class)-[:FROBS]->(b:Class) RETURN a.name") + + +def test_unknown_property_rejected() -> None: + with pytest.raises(ValidationError, match="unknown property"): + _v("MATCH (f:Function) RETURN f.bogus") + + +def test_attrs_property_allowed() -> None: + _v('MATCH (f:Function) WHERE f.attrs.anything = "x" RETURN f.name') + + +def test_unknown_inline_property_rejected() -> None: + with pytest.raises(ValidationError, match="unknown inline property"): + _v('MATCH (f:Function {bogus: "x"}) RETURN f.name') + + +def test_unbound_variable_in_return_rejected() -> None: + with pytest.raises(ValidationError, match="not bound"): + _v("MATCH (f:Function) RETURN g.name") + + +def test_unbound_variable_in_where_rejected() -> None: + with pytest.raises(ValidationError, match="not bound"): + _v('MATCH (f:Function) WHERE g.name = "x" RETURN f.name') + + +def test_order_by_alias_is_allowed() -> None: + _v("MATCH (c:Class)-[:IMPLEMENTS]->(i:Interface) RETURN i.name, count(c) AS n ORDER BY n DESC") + + +def test_order_by_unknown_ref_rejected() -> None: + with pytest.raises(ValidationError, match="ORDER BY"): + _v("MATCH (f:Function) RETURN f.name ORDER BY nope DESC") + + +# --- exclusions ------------------------------------------------------------- + + +def test_unbounded_varlen_rejected() -> None: + with pytest.raises(ValidationError, match="unbounded variable-length"): + _v("MATCH (f:Function)-[:CALLS*]->(g:Function) RETURN g.name") + + +def test_open_ended_varlen_rejected() -> None: + with pytest.raises(ValidationError, match="unbounded variable-length"): + _v("MATCH (f:Function)-[:CALLS*2..]->(g:Function) RETURN g.name") + + +def test_inverted_varlen_bounds_rejected() -> None: + with pytest.raises(ValidationError, match="min <= max"): + _v("MATCH (f:Function)-[:CALLS*5..2]->(g:Function) RETURN g.name") + + +def test_cartesian_product_rejected() -> None: + with pytest.raises(ValidationError, match="Cartesian"): + _v("MATCH (a:Class), (b:Interface) RETURN a.name, b.name") + + +def test_joined_multi_pattern_allowed_via_shared_var() -> None: + # 'a' appears in both patterns => connected, not a Cartesian product. + _v("MATCH (a:Class)-[:IMPLEMENTS]->(i:Interface), (a)-[:INHERITS]->(b:Class) RETURN a.name") + + +def test_disconnected_patterns_rejected_even_with_where() -> None: + # A WHERE cannot join disconnected patterns (RHS is a literal, not a + # property), so this is still a Cartesian product. + with pytest.raises(ValidationError, match="Cartesian"): + _v('MATCH (a:Class), (b:Interface) WHERE a.name = "x" RETURN a.name, b.name') + + +# --- capability tiers ------------------------------------------------------- + + +def test_collect_rejected_without_capability() -> None: + # collect() needs agg.collect, which the core tier does not include. + with pytest.raises(CapabilityError) as exc: + _v("MATCH (c:Class)-[:IMPLEMENTS]->(i:Interface) RETURN i.name, collect(c) AS impls") + assert exc.value.capability == AGG_COLLECT + + +def test_collect_allowed_when_backend_advertises_it() -> None: + _v( + "MATCH (c:Class)-[:IMPLEMENTS]->(i:Interface) RETURN i.name, collect(c) AS impls", + capabilities=CORE_TIER | {AGG_COLLECT}, + ) + + +def test_capability_error_is_a_validation_error() -> None: + # Callers can catch both parse-time vocabulary and capability rejects as one. + with pytest.raises(ValidationError): + _v("MATCH (c:Class) RETURN collect(c) AS x") + + +def test_varlen_requires_path_capability() -> None: + empty: frozenset[str] = frozenset({"core", "agg.basic", "pattern.exists", "string.pred"}) + with pytest.raises(CapabilityError): + _v("MATCH (f:Function)-[:CALLS*1..3]->(g:Function) RETURN g.name", capabilities=empty) diff --git a/tests/store/test_layering.py b/tests/store/test_layering.py index a0df279..2f43982 100644 --- a/tests/store/test_layering.py +++ b/tests/store/test_layering.py @@ -33,7 +33,9 @@ def _offenders(path: pathlib.Path) -> list[str]: def test_store_does_not_import_agentforge() -> None: - files = sorted(pathlib.Path(store.__file__).parent.glob("*.py")) + store_dir = pathlib.Path(store.__file__).parent + # store/*.py plus the query subpackage (feat-015) — both are engine code. + files = sorted(store_dir.glob("*.py")) + sorted(store_dir.glob("query/*.py")) files.append(pathlib.Path(config.__file__)) offenders = [o for path in files for o in _offenders(path)] assert not offenders, offenders From 76fcd8310fd43927aaa2ba5f99076a0241c9ada0 Mon Sep 17 00:00:00 2001 From: Khemchand Joshi Date: Wed, 8 Jul 2026 11:36:16 +0530 Subject: [PATCH 03/11] feat-015: Cypher compiler + Kuzu execution (chunk 2, first e2e slice) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first end-to-end vertical slice of the read-only query surface: a validated AST now compiles to openCypher and runs against the default (Kuzu) backend under enforced bounds, returning a normalized columnar result. - compile_base.py: Compiler ABC + CompiledQuery + ParamAllocator. A class per dialect, not a dialect flag — a new dialect is a subclass, a new construct is one added emit-arm. - compile_cypher.py: CypherCompiler (+ Kuzu/Neo4j subclasses) over the single generic-table schema — (f:Function) -> (f:CkgNode {kind:'Function'}), a logical property -> its physical column (f.path -> f.sym_path) via the curated catalogue, all literals parameterized. Verified against a real embedded Kuzu. - execute.py: the shared bounded driver. pull_bounded is pure and clock-injected (row cap + expansion backstop + soft timeout -> partial result); run_bounded runs the DB work on one worker thread with a hard wait_for backstop. Bounds are a real, tested guarantee on every backend, not approximated where hard. - KuzuGraphStore.run_query + QueryCapable class attrs (dialect=kuzu, caps=core+agg.collect, read_only_execution=False — the AST gate is the read-only guarantee for the embedded backend). - Store.query_graph / query_enabled / query_capabilities; CodeGraph.query_graph / describe_schema / query_enabled / query_capabilities. - conformance.py: QueryConformance — a 3-part contract (result parity, bounded execution, read-only) every query-capable backend must pass; Kuzu subclass is the always-on CI gate. attrs.* is gated behind an optional attrs.access capability that no v1 backend advertises (Kuzu/Neo4j store attrs as a JSON string, which native Cypher cannot destructure without a workaround that breaks under aggregation) — an honest deferral through the capability seam. The curated columns cover the real structural-query use cases. Deviations (for the spec at merge): the compiler visitor uses match statements, not functools.singledispatchmethod (mypy-clean, same additive property); QueryConformance lives in store/query/, not core/ (core must not import store). Gate: 1050 passed, 45 skipped, 94.68% coverage; ruff + mypy --strict clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/agentforge_graph/ingest/codegraph.py | 29 +++ src/agentforge_graph/store/facade.py | 30 +++ src/agentforge_graph/store/kuzu_store.py | 36 +++- .../store/query/capability.py | 7 +- .../store/query/compile_base.py | 65 ++++++ .../store/query/compile_cypher.py | 196 ++++++++++++++++++ .../store/query/conformance.py | 184 ++++++++++++++++ src/agentforge_graph/store/query/execute.py | 94 +++++++++ src/agentforge_graph/store/query/validator.py | 14 +- tests/store/query/test_compile_cypher.py | 69 ++++++ tests/store/query/test_execute.py | 63 ++++++ .../query/test_kuzu_query_conformance.py | 25 +++ tests/store/query/test_validator.py | 20 +- tests/store/test_facade.py | 53 +++++ 14 files changed, 879 insertions(+), 6 deletions(-) create mode 100644 src/agentforge_graph/store/query/compile_base.py create mode 100644 src/agentforge_graph/store/query/compile_cypher.py create mode 100644 src/agentforge_graph/store/query/conformance.py create mode 100644 src/agentforge_graph/store/query/execute.py create mode 100644 tests/store/query/test_compile_cypher.py create mode 100644 tests/store/query/test_execute.py create mode 100644 tests/store/query/test_kuzu_query_conformance.py diff --git a/src/agentforge_graph/ingest/codegraph.py b/src/agentforge_graph/ingest/codegraph.py index 5e5e5df..f2d05db 100644 --- a/src/agentforge_graph/ingest/codegraph.py +++ b/src/agentforge_graph/ingest/codegraph.py @@ -31,6 +31,7 @@ from agentforge_graph.repomap import RankedSymbol from agentforge_graph.retrieve import ContextPack from agentforge_graph.retrieve.retriever import Mode + from agentforge_graph.store.query import QuerySettings, ResultTable, SchemaDescription logger = logging.getLogger(__name__) @@ -495,6 +496,34 @@ async def retrieve( allow_ids=allow_ids, ) + async def query_graph( + self, text: str, settings: QuerySettings | None = None + ) -> ResultTable: + """Execute a read-only structural query (feat-015). ``settings`` bounds + the run (row cap / timeout / expansions); defaults are used if omitted. + Raises ``QueryError`` on bad input, ``QueryDisabled`` on a non-query + backend.""" + from agentforge_graph.store.query import QuerySettings as _QuerySettings + + return await self._store.query_graph(text, settings or _QuerySettings()) + + def describe_schema(self) -> SchemaDescription: + """The queryable vocabulary (node/edge kinds + properties) for + ``ckg query --schema`` (feat-015). Pure — needs no index.""" + from agentforge_graph.store.query import describe_schema + + return describe_schema() + + @property + def query_enabled(self) -> bool: + """True if the active backend can execute structural queries (feat-015).""" + return self._store.query_enabled + + @property + def query_capabilities(self) -> frozenset[str]: + """Capability tiers the active backend executes (feat-015).""" + return self._store.query_capabilities + async def repo_map( self, budget_tokens: int | None = None, diff --git a/src/agentforge_graph/store/facade.py b/src/agentforge_graph/store/facade.py index 250692f..58d790c 100644 --- a/src/agentforge_graph/store/facade.py +++ b/src/agentforge_graph/store/facade.py @@ -17,6 +17,14 @@ from .errors import SchemaVersionError, StoreError from .location import is_read_only, resolve_root +from .query import ( + QueryCapable, + QueryDisabled, + QuerySettings, + ResultTable, + parse_query, + validate_query, +) from .registry import graph_driver, vector_driver # Store-level on-disk layout version. Bumped when the .ckg/ layout changes; @@ -74,6 +82,28 @@ async def expand( nodes[nb.id] = nb return QueryResult(nodes=list(nodes.values())) + @property + def query_enabled(self) -> bool: + """True if the active graph backend can execute structural queries.""" + return isinstance(self.graph, QueryCapable) + + @property + def query_capabilities(self) -> frozenset[str]: + """The capability tiers the active backend executes (empty if none).""" + graph = self.graph + return graph.capabilities if isinstance(graph, QueryCapable) else frozenset() + + async def query_graph(self, text: str, settings: QuerySettings) -> ResultTable: + """Parse, validate (against this backend's capabilities), and execute a + read-only structural query. Raises ``QueryError`` on bad input or + ``QueryDisabled`` if the backend is not query-capable.""" + graph = self.graph + if not isinstance(graph, QueryCapable): + raise QueryDisabled(type(graph).__name__) + ast = parse_query(text) + validate_query(ast, graph.capabilities) + return await graph.run_query(ast, settings) + async def close(self) -> None: await self.graph.close() await self.vectors.close() diff --git a/src/agentforge_graph/store/kuzu_store.py b/src/agentforge_graph/store/kuzu_store.py index 410fff8..ad8b87e 100644 --- a/src/agentforge_graph/store/kuzu_store.py +++ b/src/agentforge_graph/store/kuzu_store.py @@ -55,6 +55,10 @@ from ._rowmap import ( node_params as _node_params, ) +from .query.ast import QueryAst +from .query.capability import AGG_COLLECT, CORE_TIER, QuerySettings, ResultTable +from .query.compile_cypher import KuzuCypherCompiler +from .query.execute import run_bounded SCHEMA_VERSION = 1 @@ -86,7 +90,18 @@ def _rows(result: Any) -> list[Any]: class KuzuGraphStore(GraphStore): - """Embedded graph store backed by a Kuzu database directory.""" + """Embedded graph store backed by a Kuzu database directory. + + Implements the optional ``QueryCapable`` protocol (feat-015): it compiles a + validated ``QueryAst`` to openCypher over the single-table schema and runs it + under the shared bounds. Read-only rests on the AST gate (no write can be + expressed); Kuzu embedded has no read-only session mode, so + ``read_only_execution`` is False.""" + + # --- QueryCapable (feat-015) --- + query_dialect = "kuzu" + capabilities = CORE_TIER | {AGG_COLLECT} + read_only_execution = False def __init__(self, db: kuzu.Database, conn: kuzu.Connection, path: Path) -> None: self._db = db @@ -372,6 +387,25 @@ def _adjacent_sync( edges += [_edge_from_rel(row[0], row[1], node_id) for row in _rows(res)] return edges + async def run_query(self, ast: QueryAst, settings: QuerySettings) -> ResultTable: + compiler = KuzuCypherCompiler() + compiled = compiler.compile(ast, settings) + effective_limit = compiler.effective_limit(ast, settings) + + def make_source() -> Any: + result: Any = self._conn.execute(compiled.text, compiled.params) + while result.has_next(): + yield tuple(result.get_next()) + + async with self._lock: + bounded = await run_bounded(make_source, settings, effective_limit) + return ResultTable( + columns=compiled.columns, + rows=tuple(bounded.rows), + truncated=bounded.truncated, + stopped_reason=bounded.stopped_reason, + ) + async def close(self) -> None: async with self._lock: if self._closed: diff --git a/src/agentforge_graph/store/query/capability.py b/src/agentforge_graph/store/query/capability.py index 8875ed5..2c08ab7 100644 --- a/src/agentforge_graph/store/query/capability.py +++ b/src/agentforge_graph/store/query/capability.py @@ -30,12 +30,17 @@ # Optional (a backend MAY advertise these; not part of the mandatory core): AGG_COLLECT = "agg.collect" # collect() list aggregation +ATTRS_ACCESS = "attrs.access" # querying free-form n.attrs. (needs a real +# map column, not a JSON string). No v1 backend advertises it: Kuzu/Neo4j store +# attrs as a JSON string, which native Cypher can't destructure portably without +# a workaround that breaks under aggregation — so attrs.* cleanly reports as +# unsupported via the capability seam until a backend can back it faithfully. # Every query-capable backend MUST support these and prove identical results. CORE_TIER: frozenset[str] = frozenset({CORE, AGG_BASIC, PATTERN_EXISTS, STRING_PRED, PATH_VARLEN}) # All capabilities the query language defines (core + optional). -ALL_CAPABILITIES: frozenset[str] = CORE_TIER | {AGG_COLLECT} +ALL_CAPABILITIES: frozenset[str] = CORE_TIER | {AGG_COLLECT, ATTRS_ACCESS} @dataclass(frozen=True) diff --git a/src/agentforge_graph/store/query/compile_base.py b/src/agentforge_graph/store/query/compile_base.py new file mode 100644 index 0000000..64b4574 --- /dev/null +++ b/src/agentforge_graph/store/query/compile_base.py @@ -0,0 +1,65 @@ +"""Compiler infrastructure shared by every dialect (feat-015). + +A compiler turns a *validated* ``QueryAst`` into a ``CompiledQuery`` — the +backend's native statement plus its bound parameters and the fixed output column +order. Literals are always parameterized (never string-spliced), so there is one +injection-free path on every dialect. + +``Compiler`` is a class per dialect (``compile_cypher.py``, ``compile_surreal.py`` +in chunk 4), not a function with a ``dialect`` flag — a new dialect is a new +subclass and a new construct is one added emit-arm, never an edit to an existing +branch. +""" + +from __future__ import annotations + +from abc import ABC, abstractmethod +from dataclasses import dataclass, field +from typing import Any, ClassVar + +from .ast import QueryAst +from .capability import QuerySettings + + +@dataclass(frozen=True) +class CompiledQuery: + """A backend statement + its parameters + the fixed output column order.""" + + text: str + params: dict[str, Any] + columns: tuple[str, ...] + + +@dataclass +class ParamAllocator: + """Allocates ``$p0, $p1, …`` placeholders so literals are never spliced.""" + + prefix: str = "p" + params: dict[str, Any] = field(default_factory=dict) + _n: int = 0 + + def add(self, value: Any) -> str: + key = f"{self.prefix}{self._n}" + self._n += 1 + self.params[key] = value + return key + + +class Compiler(ABC): + """AST -> native statement for one backend dialect.""" + + dialect: ClassVar[str] + + @abstractmethod + def compile(self, ast: QueryAst, settings: QuerySettings) -> CompiledQuery: + """Compile a validated AST, applying the row cap so at most + ``settings.max_rows`` rows can come back (plus one, to detect + truncation).""" + + @staticmethod + def effective_limit(ast: QueryAst, settings: QuerySettings) -> int: + """The row cap actually applied: the caller's LIMIT clamped to the + server maximum. The executor fetches this + 1 to detect truncation.""" + if ast.limit is None: + return settings.max_rows + return min(ast.limit, settings.max_rows) diff --git a/src/agentforge_graph/store/query/compile_cypher.py b/src/agentforge_graph/store/query/compile_cypher.py new file mode 100644 index 0000000..d9c716e --- /dev/null +++ b/src/agentforge_graph/store/query/compile_cypher.py @@ -0,0 +1,196 @@ +"""The openCypher compiler — Kuzu and Neo4j (feat-015). + +Both backends persist the *open* graph schema through the same ``_rowmap`` +shape: a single ``CkgNode`` table/label and a single ``CkgEdge`` type, with the +graph's ``kind`` as a string column (ADR-0005). So a caller's ``(f:Function)`` +does not map to a native label — it maps to ``(f:CkgNode {kind: 'Function'})`` — +and a logical property maps to its physical column (``f.path`` -> ``f.sym_path``) +via the curated ``schema.NODE_PROPERTIES`` catalogue. Because that shape is +identical on Kuzu and Neo4j, one compiler body serves both; the two subclasses +exist for the handful of genuine dialect deltas and are near-empty today. + +A bare node variable projects/aggregates over the node's ``id`` (a useful scalar +symbol id), so ``RETURN f`` returns ids and ``count(c)`` counts matched nodes. +""" + +from __future__ import annotations + +from typing import ClassVar + +from .ast import ( + Aggregate, + BoolOp, + Compare, + Expr, + InList, + NodePattern, + Not, + PathPattern, + PatternExists, + PropRef, + QueryAst, + RelPattern, + ReturnExpr, + StringPred, + VarRef, +) +from .capability import QuerySettings +from .compile_base import CompiledQuery, Compiler, ParamAllocator +from .schema import PROPERTY_BY_NAME + +_NODE_LABEL = "CkgNode" +_EDGE_LABEL = "CkgEdge" +_STRING_OPS = {"STARTS_WITH": "STARTS WITH", "ENDS_WITH": "ENDS WITH", "CONTAINS": "CONTAINS"} + + +class _Ctx: + """Per-compile mutable state: the parameter allocator and the set of node + variables already declared (so a repeated variable is referenced, not + re-labelled).""" + + def __init__(self) -> None: + self.params = ParamAllocator() + self.declared: set[str] = set() + + +class CypherCompiler(Compiler): + """AST -> openCypher over the single-table CkgNode/CkgEdge schema.""" + + dialect: ClassVar[str] = "cypher" + + def compile(self, ast: QueryAst, settings: QuerySettings) -> CompiledQuery: + ctx = _Ctx() + match_txt = ", ".join(self._pattern(p, ctx) for p in ast.match) + where_txt = f" WHERE {self._expr(ast.where, ctx)}" if ast.where is not None else "" + ret_txt, columns = self._returns(ast, ctx) + distinct = "DISTINCT " if ast.distinct else "" + order_txt = self._order(ast, ctx) + skip_txt = f" SKIP {ast.skip}" if ast.skip is not None else "" + limit_txt = f" LIMIT {self.effective_limit(ast, settings) + 1}" + text = ( + f"MATCH {match_txt}{where_txt} " + f"RETURN {distinct}{ret_txt}{order_txt}{skip_txt}{limit_txt}" + ) + return CompiledQuery(text=text, params=dict(ctx.params.params), columns=tuple(columns)) + + # --- patterns ----------------------------------------------------------- + + def _pattern(self, path: PathPattern, ctx: _Ctx) -> str: + out = [] + for el in path.elements: + out.append(self._node(el, ctx) if isinstance(el, NodePattern) else self._rel(el, ctx)) + return "".join(out) + + def _node(self, n: NodePattern, ctx: _Ctx) -> str: + # A repeated variable is referenced only — declare its label/props once. + if n.var is not None and n.var in ctx.declared: + return f"({n.var})" + if n.var is not None: + ctx.declared.add(n.var) + inline: dict[str, object] = {} + if n.label is not None: + inline["kind"] = n.label + for key, lit in n.props: + inline[PROPERTY_BY_NAME[key].column] = lit.value + return f"({n.var or ''}:{_NODE_LABEL}{self._inline(inline, ctx)})" + + def _rel(self, r: RelPattern, ctx: _Ctx) -> str: + left = "<-" if r.direction == "in" else "-" + right = "->" if r.direction == "out" else "-" + span = "" if (r.min_hops, r.max_hops) == (1, 1) else f"*{r.min_hops}..{r.max_hops}" + inline: dict[str, object] = {"kind": r.kind} if r.kind is not None else {} + return f"{left}[{r.var or ''}:{_EDGE_LABEL}{span}{self._inline(inline, ctx)}]{right}" + + def _inline(self, props: dict[str, object], ctx: _Ctx) -> str: + if not props: + return "" + body = ", ".join(f"{col}: ${ctx.params.add(v)}" for col, v in props.items()) + return f" {{{body}}}" + + # --- WHERE -------------------------------------------------------------- + + def _expr(self, expr: Expr, ctx: _Ctx) -> str: + # One arm per Expr node — adding a construct adds an arm, never edits one. + match expr: + case Compare(lhs, op, rhs): + return f"{self._prop(lhs)} {op} ${ctx.params.add(rhs.value)}" + case InList(lhs, values): + items = ", ".join(f"${ctx.params.add(v.value)}" for v in values) + return f"{self._prop(lhs)} IN [{items}]" + case StringPred(lhs, op, rhs): + return f"{self._prop(lhs)} {_STRING_OPS[op]} ${ctx.params.add(rhs)}" + case Not(operand): + return f"NOT ({self._expr(operand, ctx)})" + case BoolOp(op, operands): + joined = f" {op} ".join(self._expr(o, ctx) for o in operands) + return f"({joined})" + case PatternExists(pattern): + return self._pattern(pattern, ctx) + raise AssertionError(f"unhandled expr node: {type(expr).__name__}") # pragma: no cover + + # --- RETURN / ORDER BY -------------------------------------------------- + + def _returns(self, ast: QueryAst, ctx: _Ctx) -> tuple[str, list[str]]: + items: list[str] = [] + columns: list[str] = [] + for item in ast.returns: + text, default_col = self._return_expr(item.expr) + columns.append(item.alias or default_col) + items.append(f"{text} AS {item.alias}" if item.alias is not None else text) + return ", ".join(items), columns + + def _return_expr(self, expr: ReturnExpr) -> tuple[str, str]: + match expr: + case PropRef(): + return self._prop(expr), f"{expr.var}.{'.'.join(expr.path)}" + case VarRef(var): + return f"{var}.id", var + case Aggregate(func, arg, distinct): + inner = self._agg_arg(arg) + distinct_txt = "DISTINCT " if distinct else "" + col = f"{func}({'*' if arg is None else self._agg_label(arg)})" + return f"{func}({distinct_txt}{inner})", col + raise AssertionError(f"unhandled return expr: {type(expr).__name__}") # pragma: no cover + + def _agg_arg(self, arg: PropRef | VarRef | None) -> str: + if arg is None: + return "*" + return f"{arg.var}.id" if isinstance(arg, VarRef) else self._prop(arg) + + def _agg_label(self, arg: PropRef | VarRef) -> str: + return arg.var if isinstance(arg, VarRef) else f"{arg.var}.{'.'.join(arg.path)}" + + def _order(self, ast: QueryAst, ctx: _Ctx) -> str: + if not ast.order_by: + return "" + aliases = {item.alias for item in ast.returns if item.alias} + keys: list[str] = [] + for key in ast.order_by: + if isinstance(key.ref, PropRef): + ref = self._prop(key.ref) + elif key.ref.var in aliases: + ref = key.ref.var # a RETURN alias + else: + ref = f"{key.ref.var}.id" # a bound node variable + keys.append(f"{ref} DESC" if key.descending else ref) + return " ORDER BY " + ", ".join(keys) + + # --- shared ------------------------------------------------------------- + + def _prop(self, ref: PropRef) -> str: + # attrs.* never reaches a Cypher backend (gated by the attrs.access + # capability, which Kuzu/Neo4j do not advertise), so the path is a + # single curated segment here. + return f"{ref.var}.{PROPERTY_BY_NAME[ref.path[0]].column}" + + +class KuzuCypherCompiler(CypherCompiler): + """Kuzu deltas over the shared Cypher body (none needed today).""" + + dialect: ClassVar[str] = "kuzu" + + +class Neo4jCypherCompiler(CypherCompiler): + """Neo4j deltas over the shared Cypher body (populated in chunk 3).""" + + dialect: ClassVar[str] = "neo4j" diff --git a/src/agentforge_graph/store/query/conformance.py b/src/agentforge_graph/store/query/conformance.py new file mode 100644 index 0000000..b0369cb --- /dev/null +++ b/src/agentforge_graph/store/query/conformance.py @@ -0,0 +1,184 @@ +"""Reusable query-conformance suite for ``QueryCapable`` backends (feat-015). + +A backend proves it runs the read-only query surface correctly — and identically +to every other backend — by subclassing ``QueryConformance`` and providing an +async ``store`` fixture (the same pattern as feat-003's ``GraphStoreConformance``). +The suite has three mandatory parts, each a contract every query-capable backend +must pass, so the extensibility guarantees are *enforced*, not asserted in prose: + +1. **Result parity** — a fixed query set returns the canonical expected rows. + Because every backend subclasses this same suite over the same fixture, "the + expected rows" are identical across backends by construction. +2. **Bounded execution** — a runaway query returns a partial result with the + right ``stopped_reason`` (row / expansion cap), on every backend. +3. **Read-only** — a write cannot reach execution (the AST has no write node). + +Pytest-free at import (the test package's pytest-asyncio collects the ``test_`` +methods via the subclass), mirroring ``core/conformance.py``. +""" + +from __future__ import annotations + +from agentforge_graph.core import ( + Descriptor, + Edge, + EdgeKind, + FileSubgraph, + Node, + NodeKind, + Provenance, + SymbolID, +) + +from .capability import QueryCapable, QuerySettings, ResultTable +from .errors import ParseError +from .parser import parse_query +from .validator import validate_query + +_LANG, _REPO, _PATH = "py", "sample", "src/app.py" +_PROV = Provenance.parsed("query-conformance", "c0") + + +def _nid(descriptor: str) -> str: + return SymbolID.for_symbol(_LANG, _REPO, _PATH, descriptor) + + +def _node(descriptor: str, kind: NodeKind, name: str, span: tuple[int, int] = (1, 5)) -> Node: + return Node(id=_nid(descriptor), kind=kind, name=name, span=span, provenance=_PROV) + + +def make_query_fixture() -> FileSubgraph: + """A small graph exercising every core construct: classes implementing an + interface, a call chain (for var-length), an orphan, and a tag.""" + repo = _node(Descriptor.type("Repo"), NodeKind.CLASS, "Repo") + cache = _node(Descriptor.type("Cache"), NodeKind.CLASS, "Cache") + plain = _node(Descriptor.type("Plain"), NodeKind.CLASS, "Plain") + store_if = _node(Descriptor.type("Store"), NodeKind.INTERFACE, "Store") + empty_if = _node(Descriptor.type("Empty"), NodeKind.INTERFACE, "Empty") + foo = _node(Descriptor.term("foo"), NodeKind.FUNCTION, "foo") + bar = _node(Descriptor.term("bar"), NodeKind.FUNCTION, "bar") + baz = _node(Descriptor.term("baz"), NodeKind.FUNCTION, "baz") + qux = _node(Descriptor.term("qux"), NodeKind.FUNCTION, "qux") + tag = _node(Descriptor.term("Repository"), NodeKind.PATTERN_TAG, "Repository") + nodes = [repo, cache, plain, store_if, empty_if, foo, bar, baz, qux, tag] + edges = [ + Edge(src=repo.id, dst=store_if.id, kind=EdgeKind.IMPLEMENTS, provenance=_PROV), + Edge(src=cache.id, dst=store_if.id, kind=EdgeKind.IMPLEMENTS, provenance=_PROV), + Edge(src=foo.id, dst=bar.id, kind=EdgeKind.CALLS, provenance=_PROV), + Edge(src=bar.id, dst=baz.id, kind=EdgeKind.CALLS, provenance=_PROV), + Edge(src=repo.id, dst=tag.id, kind=EdgeKind.TAGGED, provenance=_PROV), + ] + return FileSubgraph(path=_PATH, content_hash="h0", nodes=nodes, edges=edges) + + +class QueryConformance: + """Subclass in a backend's test module; provide an async ``store`` fixture + yielding a fresh ``QueryCapable`` graph store.""" + + async def _run( + self, store: QueryCapable, text: str, settings: QuerySettings | None = None + ) -> ResultTable: + ast = validate_query(parse_query(text), store.capabilities) + return await store.run_query(ast, settings or QuerySettings()) + + @staticmethod + def _col0(rt: ResultTable) -> set[object]: + return {row[0] for row in rt.rows} + + async def _load(self, store: QueryCapable) -> None: + await store.upsert(make_query_fixture()) # type: ignore[attr-defined] + + # --- 1. result parity --------------------------------------------------- + + async def test_simple_projection(self, store: QueryCapable) -> None: + await self._load(store) + rt = await self._run(store, "MATCH (f:Function) RETURN f.name") + assert rt.columns == ("f.name",) + assert self._col0(rt) == {"foo", "bar", "baz", "qux"} + assert not rt.truncated + + async def test_property_mapping(self, store: QueryCapable) -> None: + await self._load(store) + rt = await self._run(store, 'MATCH (c:Class {name: "Repo"}) RETURN c.path, c.start_line') + assert rt.columns == ("c.path", "c.start_line") + assert rt.rows == (("src/app.py", 1),) + + async def test_aggregate_and_order(self, store: QueryCapable) -> None: + await self._load(store) + rt = await self._run( + store, + "MATCH (c:Class)-[:IMPLEMENTS]->(i:Interface) " + "RETURN i.name, count(c) AS impls ORDER BY impls DESC", + ) + assert rt.columns == ("i.name", "impls") + assert rt.rows == (("Store", 2),) + + async def test_pattern_exists_negation(self, store: QueryCapable) -> None: + await self._load(store) + # functions with no inbound CALLS: foo (caller) and qux (orphan). + rt = await self._run(store, "MATCH (f:Function) WHERE NOT (f)<-[:CALLS]-() RETURN f.name") + assert self._col0(rt) == {"foo", "qux"} + + async def test_variable_length_path(self, store: QueryCapable) -> None: + await self._load(store) + # foo -CALLS-> bar -CALLS-> baz : within 1..2 hops foo reaches bar, baz. + rt = await self._run( + store, 'MATCH (a:Function {name: "foo"})-[:CALLS*1..2]->(b:Function) RETURN b.name' + ) + assert self._col0(rt) == {"bar", "baz"} + + async def test_string_predicate(self, store: QueryCapable) -> None: + await self._load(store) + rt = await self._run(store, 'MATCH (c:Class) WHERE c.name STARTS WITH "C" RETURN c.name') + assert self._col0(rt) == {"Cache"} + + async def test_in_list(self, store: QueryCapable) -> None: + await self._load(store) + rt = await self._run( + store, 'MATCH (f:Function) WHERE f.name IN ["foo", "qux"] RETURN f.name' + ) + assert self._col0(rt) == {"foo", "qux"} + + async def test_tagged_classes(self, store: QueryCapable) -> None: + await self._load(store) + rt = await self._run( + store, + 'MATCH (c:Class)-[:TAGGED]->(t:PatternTag) WHERE t.name = "Repository" RETURN c.name', + ) + assert self._col0(rt) == {"Repo"} + + # --- 2. bounded execution ---------------------------------------------- + + async def test_row_cap_truncates(self, store: QueryCapable) -> None: + await self._load(store) + rt = await self._run(store, "MATCH (f:Function) RETURN f.name", QuerySettings(max_rows=2)) + assert len(rt.rows) == 2 + assert rt.truncated and rt.stopped_reason == "row_cap" + + async def test_expansion_cap_is_a_real_backstop(self, store: QueryCapable) -> None: + await self._load(store) + # A hard ceiling on rows pulled, independent of the pushed LIMIT. + rt = await self._run( + store, + "MATCH (f:Function) RETURN f.name", + QuerySettings(max_rows=100, max_expansions=1), + ) + assert len(rt.rows) == 1 + assert rt.truncated and rt.stopped_reason == "expansion_cap" + + # --- 3. read-only ------------------------------------------------------- + + async def test_write_cannot_reach_execution(self, store: QueryCapable) -> None: + await self._load(store) + # The grammar has no write production, so a write is rejected before it + # could ever be compiled or run — the read-only guarantee starts here. + for text in ( + 'CREATE (n:Class {name: "x"}) RETURN n.name', + "MATCH (n:Class) DETACH DELETE n", + "MATCH (n:Class) SET n.name = 'y' RETURN n.name", + ): + try: + await self._run(store, text) + except ParseError: + continue + raise AssertionError(f"write-shaped query was not rejected: {text}") diff --git a/src/agentforge_graph/store/query/execute.py b/src/agentforge_graph/store/query/execute.py new file mode 100644 index 0000000..e2a3300 --- /dev/null +++ b/src/agentforge_graph/store/query/execute.py @@ -0,0 +1,94 @@ +"""The shared bounded-execution driver (feat-015). + +Guardrails are written **once** here and reused by every backend, so a bound is a +real, tested guarantee on all of them rather than "best-effort where the backend +happens to support it". A backend adapter supplies only its native "run this +compiled statement and yield rows" primitive; this module wraps it with: + +- **row cap** — stop after ``max_rows`` (the compiler already pushed + ``LIMIT max_rows + 1``; pulling one extra row is how truncation is detected); +- **expansion cap** — a hard ceiling on rows pulled from the cursor, independent + of the pushed LIMIT: a backstop if a backend does not honour the row cap; +- **timeout** — a soft wall-clock deadline checked between rows (returns the + partial result gathered so far), plus a hard ``asyncio.wait_for`` backstop that + raises ``GuardrailError`` if a single fetch hangs past the budget. + +``pull_bounded`` is pure and clock-injected, so the whole policy is unit-tested +with a scripted row source and a fake clock — no database, no sleeps. +""" + +from __future__ import annotations + +import asyncio +import time +from collections.abc import Callable, Iterator +from dataclasses import dataclass +from typing import Any + +from .capability import QuerySettings +from .errors import GuardrailError + +# Extra wall-clock grace for the hard asyncio backstop over the soft in-loop +# deadline, so the soft deadline (which yields a partial result) normally wins. +_HARD_TIMEOUT_GRACE_S = 2.0 + +Row = tuple[Any, ...] + + +@dataclass(frozen=True) +class BoundedResult: + rows: list[Row] + truncated: bool + stopped_reason: str | None # None | "row_cap" | "expansion_cap" | "timeout" + + +def pull_bounded( + next_row: Callable[[], Row | None], + settings: QuerySettings, + effective_limit: int, + now: Callable[[], float] = time.monotonic, +) -> BoundedResult: + """Pull rows from ``next_row`` (returns ``None`` when exhausted) under the + row/expansion/timeout bounds. Pure; ``now`` is injected for tests.""" + deadline = now() + settings.timeout_ms / 1000 + rows: list[Row] = [] + reason: str | None = None + while True: + if now() >= deadline: + reason = "timeout" + break + if len(rows) >= settings.max_expansions: + reason = "expansion_cap" + break + row = next_row() + if row is None: + break + rows.append(row) + if len(rows) > effective_limit: + reason = "row_cap" + rows = rows[:effective_limit] + break + return BoundedResult(rows=rows, truncated=reason is not None, stopped_reason=reason) + + +async def run_bounded( + make_source: Callable[[], Iterator[Row]], + settings: QuerySettings, + effective_limit: int, + now: Callable[[], float] = time.monotonic, +) -> BoundedResult: + """Run ``make_source`` (which executes the statement and returns a row + iterator) on a worker thread under the bounds. The DB work and the pull + happen on one thread, so a non-thread-safe connection stays serial.""" + + def work() -> BoundedResult: + source = make_source() + return pull_bounded(lambda: next(source, None), settings, effective_limit, now) + + try: + return await asyncio.wait_for( + asyncio.to_thread(work), + timeout=settings.timeout_ms / 1000 + _HARD_TIMEOUT_GRACE_S, + ) + except TimeoutError as exc: # a single fetch hung past the hard backstop + raise GuardrailError(f"query exceeded the {settings.timeout_ms}ms time budget") from exc diff --git a/src/agentforge_graph/store/query/validator.py b/src/agentforge_graph/store/query/validator.py index 1876518..7f60733 100644 --- a/src/agentforge_graph/store/query/validator.py +++ b/src/agentforge_graph/store/query/validator.py @@ -40,13 +40,14 @@ from .capability import ( AGG_BASIC, AGG_COLLECT, + ATTRS_ACCESS, CORE_TIER, PATH_VARLEN, PATTERN_EXISTS, STRING_PRED, ) from .errors import CapabilityError, ValidationError -from .schema import PROPERTY_BY_NAME, is_known_property +from .schema import PROPERTY_BY_NAME, is_attrs_ref, is_known_property _NODE_KIND_VALUES = frozenset(k.value for k in NodeKind) _EDGE_KIND_VALUES = frozenset(k.value for k in EdgeKind) @@ -245,6 +246,17 @@ def _check_capabilities(ast: QueryAst, capabilities: frozenset[str]) -> None: for item in ast.returns: if isinstance(item.expr, Aggregate): required.add(AGG_COLLECT if item.expr.func == "collect" else AGG_BASIC) + for ref in _all_prop_refs(ast): + if is_attrs_ref(ref.path): + required.add(ATTRS_ACCESS) for cap in sorted(required): if cap not in capabilities: raise CapabilityError(cap, capabilities) + + +def _all_prop_refs(ast: QueryAst) -> Iterator[PropRef]: + yield from _where_prop_refs(ast) + yield from _return_prop_refs(ast) + for key in ast.order_by: + if isinstance(key.ref, PropRef): + yield key.ref diff --git a/tests/store/query/test_compile_cypher.py b/tests/store/query/test_compile_cypher.py new file mode 100644 index 0000000..7c2c977 --- /dev/null +++ b/tests/store/query/test_compile_cypher.py @@ -0,0 +1,69 @@ +"""Cypher compiler unit tests (feat-015 chunk 2): AST -> Cypher text + params. + +Pure (no DB). Asserts the single-table mapping, property->column mapping, literal +parameterization, and the row-cap LIMIT. End-to-end execution is covered by the +Kuzu conformance suite. +""" + +from __future__ import annotations + +from agentforge_graph.store.query import parse_query, validate_query +from agentforge_graph.store.query.capability import AGG_COLLECT, CORE_TIER, QuerySettings +from agentforge_graph.store.query.compile_cypher import KuzuCypherCompiler + +_CAPS = CORE_TIER | {AGG_COLLECT} + + +def _compile(text: str, max_rows: int = 1000): + ast = validate_query(parse_query(text), _CAPS) + return KuzuCypherCompiler().compile(ast, QuerySettings(max_rows=max_rows)) + + +def test_label_maps_to_kind_predicate_inline() -> None: + cq = _compile("MATCH (f:Function) RETURN f.name") + assert "(f:CkgNode {kind: $p0})" in cq.text + assert cq.params == {"p0": "Function"} + assert cq.columns == ("f.name",) + + +def test_property_maps_to_physical_column() -> None: + cq = _compile('MATCH (c:Class {name: "Repo"}) RETURN c.path, c.start_line') + assert "c.sym_path" in cq.text and "c.span_start" in cq.text + assert cq.columns == ("c.path", "c.start_line") + + +def test_literals_are_parameterized_never_spliced() -> None: + cq = _compile('MATCH (f:Function) WHERE f.name = "x" AND f.confidence >= 0.5 RETURN f.name') + assert '"x"' not in cq.text and "0.5" not in cq.text + assert "x" in cq.params.values() and 0.5 in cq.params.values() + + +def test_relationship_and_direction() -> None: + cq = _compile("MATCH (a:Class)-[:IMPLEMENTS]->(i:Interface) RETURN a.name") + assert "-[:CkgEdge {kind: $" in cq.text and "]->" in cq.text + + +def test_varlen_span_rendered() -> None: + cq = _compile("MATCH (f:Function)-[:CALLS*1..3]->(g:Function) RETURN g.name") + assert "CkgEdge*1..3" in cq.text + + +def test_aggregate_over_node_var_uses_id() -> None: + cq = _compile("MATCH (c:Class)-[:IMPLEMENTS]->(i:Interface) RETURN i.name, count(c) AS impls") + assert "count(c.id) AS impls" in cq.text + assert cq.columns == ("i.name", "impls") + + +def test_row_cap_limit_is_effective_plus_one() -> None: + assert " LIMIT 6" in _compile("MATCH (f:Function) RETURN f.name", max_rows=5).text + # caller LIMIT below the cap wins: + assert " LIMIT 4" in _compile("MATCH (f:Function) RETURN f.name LIMIT 3", max_rows=5).text + + +def test_repeated_variable_declared_once() -> None: + cq = _compile( + "MATCH (a:Class)-[:IMPLEMENTS]->(i:Interface), (a)-[:INHERITS]->(b:Class) RETURN a.name" + ) + # 'a' gets its label/kind once; the second occurrence is a bare reference. + assert cq.text.count("a:CkgNode {kind:") == 1 + assert "(a)-[:CkgEdge {kind: $" in cq.text diff --git a/tests/store/query/test_execute.py b/tests/store/query/test_execute.py new file mode 100644 index 0000000..7c8266a --- /dev/null +++ b/tests/store/query/test_execute.py @@ -0,0 +1,63 @@ +"""Unit tests for the pure bounded-execution policy (feat-015 chunk 2). + +``pull_bounded`` is clock-injected and DB-free, so the row/expansion/timeout +matrix is tested with a scripted row source and a fake clock. +""" + +from __future__ import annotations + +import contextlib +from collections.abc import Callable + +from agentforge_graph.store.query.capability import QuerySettings +from agentforge_graph.store.query.execute import pull_bounded + + +def _source(n: int) -> Callable[[], tuple[object, ...] | None]: + rows = iter([(i,) for i in range(n)]) + return lambda: next(rows, None) + + +def _clock(ticks: list[float]) -> Callable[[], float]: + seq = iter(ticks) + last = [0.0] + + def now() -> float: + with contextlib.suppress(StopIteration): + last[0] = next(seq) + return last[0] + + return now + + +def test_complete_result_not_truncated() -> None: + res = pull_bounded(_source(3), QuerySettings(max_rows=10), effective_limit=10, now=lambda: 0.0) + assert res.rows == [(0,), (1,), (2,)] + assert not res.truncated and res.stopped_reason is None + + +def test_row_cap_truncates_and_trims() -> None: + res = pull_bounded(_source(10), QuerySettings(max_rows=3), effective_limit=3, now=lambda: 0.0) + assert res.rows == [(0,), (1,), (2,)] + assert res.truncated and res.stopped_reason == "row_cap" + + +def test_expansion_cap_backstops_before_row_cap() -> None: + res = pull_bounded( + _source(10), + QuerySettings(max_rows=100, max_expansions=2), + effective_limit=100, + now=lambda: 0.0, + ) + assert res.rows == [(0,), (1,)] + assert res.truncated and res.stopped_reason == "expansion_cap" + + +def test_timeout_returns_partial() -> None: + # clock jumps past the deadline after two rows are pulled. + now = _clock([0.0, 0.0, 0.001, 0.001, 10.0]) + res = pull_bounded( + _source(100), QuerySettings(max_rows=100, timeout_ms=1000), effective_limit=100, now=now + ) + assert res.truncated and res.stopped_reason == "timeout" + assert len(res.rows) < 100 diff --git a/tests/store/query/test_kuzu_query_conformance.py b/tests/store/query/test_kuzu_query_conformance.py new file mode 100644 index 0000000..d402128 --- /dev/null +++ b/tests/store/query/test_kuzu_query_conformance.py @@ -0,0 +1,25 @@ +"""Run feat-015's QueryConformance against the Kuzu adapter (chunk 2). + +Kuzu is embedded, so this is the always-on CI gate for the query surface; the +Neo4j and SurrealDB subclasses (chunks 3-4) run the same suite env-gated. +""" + +from __future__ import annotations + +from collections.abc import AsyncIterator +from pathlib import Path + +import pytest + +from agentforge_graph.store import KuzuGraphStore +from agentforge_graph.store.query.conformance import QueryConformance + + +class TestKuzuQuery(QueryConformance): + @pytest.fixture + async def store(self, tmp_path: Path) -> AsyncIterator[KuzuGraphStore]: + s = await KuzuGraphStore.open(tmp_path / "graph.kuzu") + try: + yield s + finally: + await s.close() diff --git a/tests/store/query/test_validator.py b/tests/store/query/test_validator.py index 034f820..eaad87d 100644 --- a/tests/store/query/test_validator.py +++ b/tests/store/query/test_validator.py @@ -34,7 +34,6 @@ def _v(text: str, capabilities: frozenset[str] = CORE_TIER) -> None: 'MATCH (c:Class {name: "Repo"}) RETURN c.path', "MATCH (a:Class)-[:IMPLEMENTS]->(i:Interface) RETURN i.name, count(a) AS n ORDER BY n DESC", "MATCH (f:Function) WHERE NOT (f)<-[:CALLS]-() RETURN f.name", - 'MATCH (s:Service) WHERE s.attrs.framework = "fastapi" RETURN s.name', "MATCH (f:Function)-[:CALLS*1..3]->(g:Function) RETURN g.name", "MATCH (a:Class)-[:IMPLEMENTS]->(i:Interface), (a)-[:INHERITS]->(b:Class) RETURN a.name", ], @@ -66,8 +65,23 @@ def test_unknown_property_rejected() -> None: _v("MATCH (f:Function) RETURN f.bogus") -def test_attrs_property_allowed() -> None: - _v('MATCH (f:Function) WHERE f.attrs.anything = "x" RETURN f.name') +def test_attrs_property_structurally_known_but_gated() -> None: + # attrs.* parses and is a known property shape, but needs the (optional) + # attrs.access capability, which the core tier does not include. + from agentforge_graph.store.query.capability import ATTRS_ACCESS + + with pytest.raises(CapabilityError) as exc: + _v('MATCH (f:Function) WHERE f.attrs.anything = "x" RETURN f.name') + assert exc.value.capability == ATTRS_ACCESS + + +def test_attrs_property_allowed_when_backend_advertises_it() -> None: + from agentforge_graph.store.query.capability import ATTRS_ACCESS + + _v( + 'MATCH (f:Function) WHERE f.attrs.anything = "x" RETURN f.name', + capabilities=CORE_TIER | {ATTRS_ACCESS}, + ) def test_unknown_inline_property_rejected() -> None: diff --git a/tests/store/test_facade.py b/tests/store/test_facade.py index b7648dd..801f73b 100644 --- a/tests/store/test_facade.py +++ b/tests/store/test_facade.py @@ -143,3 +143,56 @@ async def test_expand_skips_absent_ref(tmp_path: Path) -> None: assert result.nodes == [] # nothing in the graph yet finally: await store.close() + + +# --- query_graph (feat-015) ------------------------------------------------- + + +async def test_query_enabled_and_capabilities_on_kuzu(tmp_path: Path) -> None: + store = await Store.open(repo_path=tmp_path) + try: + assert store.query_enabled is True + assert "core" in store.query_capabilities + finally: + await store.close() + + +async def test_query_graph_end_to_end(tmp_path: Path) -> None: + from agentforge_graph.store.query import QuerySettings + + store = await Store.open(repo_path=tmp_path) + try: + await store.graph.upsert(make_sample_subgraph()) # File ▸ Class ▸ Method + rt = await store.query_graph( + 'MATCH (c:Class) WHERE c.name = "Auth" RETURN c.name, c.path', QuerySettings() + ) + assert rt.columns == ("c.name", "c.path") + assert rt.rows == (("Auth", "src/app/auth.py"),) + assert not rt.truncated + finally: + await store.close() + + +async def test_query_graph_bad_syntax_raises_parse_error(tmp_path: Path) -> None: + from agentforge_graph.store.query import ParseError, QuerySettings + + store = await Store.open(repo_path=tmp_path) + try: + with pytest.raises(ParseError): + await store.query_graph("MATCH (c:Class) RETURN", QuerySettings()) + finally: + await store.close() + + +async def test_query_graph_attrs_needs_capability(tmp_path: Path) -> None: + from agentforge_graph.store.query import CapabilityError, QuerySettings + + store = await Store.open(repo_path=tmp_path) + try: + # Kuzu stores attrs as a JSON string and does not advertise attrs.access. + with pytest.raises(CapabilityError): + await store.query_graph( + 'MATCH (c:Class) WHERE c.attrs.x = "y" RETURN c.name', QuerySettings() + ) + finally: + await store.close() From f9b92e0611f667d5d1a83bd1d42e388617c41c93 Mon Sep 17 00:00:00 2001 From: Khemchand Joshi Date: Wed, 8 Jul 2026 12:34:34 +0530 Subject: [PATCH 04/11] feat-015: ckg query --graph / --schema / --format CLI (chunk 5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The power-user surface for the read-only query engine. The natural-language retrieval path (ckg query "") is unchanged; --graph selects the structural surface. - cli_format.py: a reusable render_table / render_json helper (introduced here, adoptable by other verbs later — not inline in the handler). - query subcommand: --graph '' runs a structural query; --schema prints the queryable vocabulary (node/edge kinds + properties); --format {table,json}; --limit caps rows (clamped to the server max). QueryError / QueryDisabled -> stderr + exit 2 (the structured "why", not a traceback). Verified live: ckg index a repo, then ckg query --graph 'MATCH (c:Class) RETURN c.name' -> aligned table; --format json -> columns/rows + truncation envelope; --schema -> vocabulary. Gate: full suite green (1064 passed, 94.72% cov); ruff + mypy --strict clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/agentforge_graph/cli.py | 98 ++++++++++++++++++++ src/agentforge_graph/cli_format.py | 56 ++++++++++++ tests/test_cli_query.py | 138 +++++++++++++++++++++++++++++ 3 files changed, 292 insertions(+) create mode 100644 src/agentforge_graph/cli_format.py create mode 100644 tests/test_cli_query.py diff --git a/src/agentforge_graph/cli.py b/src/agentforge_graph/cli.py index 51d21c4..5fa0feb 100644 --- a/src/agentforge_graph/cli.py +++ b/src/agentforge_graph/cli.py @@ -734,6 +734,16 @@ async def _map(args: argparse.Namespace) -> int: async def _query(args: argparse.Namespace) -> int: + # feat-015: --schema and --graph select the structural surface; the + # natural-language retrieval path (below) is unchanged. + if args.schema: + return await _query_schema(args) + if args.graph is not None: + return await _query_graph(args) + if args.limit is not None: + print("--limit only applies to --graph", file=sys.stderr) + return 2 + from agentforge_graph.temporal import TemporalError cg = await CodeGraph.open(repo_path=args.path, config=args.config) @@ -756,6 +766,70 @@ async def _query(args: argparse.Namespace) -> int: return 0 +async def _query_schema(args: argparse.Namespace) -> int: + """feat-015: print the queryable vocabulary (needs no index build).""" + from agentforge_graph.cli_format import render_table + + cg = await CodeGraph.open(repo_path=args.path, config=args.config) + try: + desc = cg.describe_schema() + finally: + await cg.close() + if args.format == "json": + import json + + print(json.dumps(desc.to_dict(), indent=2)) + return 0 + print(f"query language v{desc.lang_version}\n") + print("node kinds: " + ", ".join(desc.node_kinds)) + print("edge kinds: " + ", ".join(desc.edge_kinds) + "\n") + print( + render_table( + ("property", "type", "description"), + [(p.name, p.type, p.doc) for p in desc.node_properties], + ) + ) + print("\n" + desc.attrs_note) + return 0 + + +async def _query_graph(args: argparse.Namespace) -> int: + """feat-015: run a read-only structural query and render the result.""" + from agentforge_graph.cli_format import render_json, render_table + from agentforge_graph.store.query import QueryDisabled, QueryError, QuerySettings + + settings = QuerySettings() + if args.limit is not None: + settings = QuerySettings( + max_rows=min(settings.max_rows, args.limit), + timeout_ms=settings.timeout_ms, + max_expansions=settings.max_expansions, + ) + cg = await CodeGraph.open(repo_path=args.path, config=args.config) + try: + rt = await cg.query_graph(args.graph, settings) + except QueryError as exc: + print(f"query error: {exc}", file=sys.stderr) + return 2 + except QueryDisabled as exc: + print(f"query unavailable: {exc}", file=sys.stderr) + return 2 + finally: + await cg.close() + + if args.format == "json": + print( + render_json( + rt.columns, rt.rows, truncated=rt.truncated, stopped_reason=rt.stopped_reason + ) + ) + else: + print(render_table(rt.columns, rt.rows)) + if rt.truncated: + print(f"... result truncated ({rt.stopped_reason})", file=sys.stderr) + return 0 + + async def _doctor(args: argparse.Namespace) -> int: """ENH-026: validate config readiness without indexing — for one repo or a whole workspace, reporting every problem (and its fix) in one pass.""" @@ -925,6 +999,30 @@ def build_parser() -> argparse.ArgumentParser: help="reconstruct results as of a git commit (feat-009; needs temporal + backfill)", ) qry.add_argument("--config", default=None, help="path to ckg.yaml") + # feat-015: read-only structural query surface (Cypher subset). + qry.add_argument( + "--graph", + metavar="CYPHER", + default=None, + help="run a read-only structural query (Cypher subset) instead of NL retrieval", + ) + qry.add_argument( + "--schema", + action="store_true", + help="print the queryable vocabulary (node/edge kinds + properties) and exit", + ) + qry.add_argument( + "--format", + default="table", + choices=["table", "json"], + help="output format for --graph/--schema (default: table)", + ) + qry.add_argument( + "--limit", + type=int, + default=None, + help="cap rows for --graph (clamped to the server max)", + ) qry.set_defaults(func=_query) mp = sub.add_parser("map", help="print a budget-aware, centrality-ranked repo map") diff --git a/src/agentforge_graph/cli_format.py b/src/agentforge_graph/cli_format.py new file mode 100644 index 0000000..d9e43a5 --- /dev/null +++ b/src/agentforge_graph/cli_format.py @@ -0,0 +1,56 @@ +"""Reusable columnar output rendering for the CLI (feat-015). + +Introduced for ``ckg query --graph`` but written as a standalone helper other +verbs can adopt later without a rewrite — not inline in the query handler. Two +formats: an aligned text table (human) and JSON rows (machine). +""" + +from __future__ import annotations + +import json +from collections.abc import Sequence +from typing import Any + + +def _cell(value: Any) -> str: + if value is None: + return "" + if isinstance(value, bool): + return "true" if value else "false" + return str(value) + + +def render_table(columns: Sequence[str], rows: Sequence[Sequence[Any]]) -> str: + """A monospace, left-aligned table with a header underline. Empty result + still prints the header so the shape is visible.""" + headers = list(columns) + body = [[_cell(v) for v in row] for row in rows] + widths = [len(h) for h in headers] + for row in body: + for i, cell in enumerate(row): + widths[i] = max(widths[i], len(cell)) + lines = [ + " ".join(h.ljust(widths[i]) for i, h in enumerate(headers)), + " ".join("-" * widths[i] for i in range(len(headers))), + ] + lines += [" ".join(cell.ljust(widths[i]) for i, cell in enumerate(row)) for row in body] + if not body: + lines.append("(no rows)") + return "\n".join(lines) + + +def render_json( + columns: Sequence[str], + rows: Sequence[Sequence[Any]], + *, + truncated: bool = False, + stopped_reason: str | None = None, +) -> str: + """JSON with the column order, row arrays, and the truncation envelope.""" + payload = { + "columns": list(columns), + "rows": [list(row) for row in rows], + "truncated": truncated, + "stopped_reason": stopped_reason, + } + return json.dumps(payload, indent=2, default=str) diff --git a/tests/test_cli_query.py b/tests/test_cli_query.py new file mode 100644 index 0000000..dfaece1 --- /dev/null +++ b/tests/test_cli_query.py @@ -0,0 +1,138 @@ +"""feat-015 chunk 5: the `ckg query --graph / --schema / --format` CLI. + +End-to-end over a real indexed repo (embedded Kuzu), plus the reusable +cli_format helpers. +""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + +from agentforge_graph.cli import main +from agentforge_graph.cli_format import render_json, render_table +from agentforge_graph.ingest import CodeGraph + +_SRC = """\ +class Repo: + def save(self): ... + +class Cache: + def get(self): ... + +def helper(): + return Repo() +""" + + +@pytest.fixture +async def indexed(tmp_path: Path) -> Path: + (tmp_path / "app.py").write_text(_SRC) + cg = await CodeGraph.index(repo_path=tmp_path) + await cg.close() + return tmp_path + + +# --- cli_format units ------------------------------------------------------- + + +def test_render_table_aligns_and_headers() -> None: + out = render_table(("name", "kind"), [("Repo", "Class"), ("helper", "Function")]) + lines = out.splitlines() + assert lines[0].startswith("name") + assert set(lines[1]) <= {"-", " "} + assert "Repo" in out and "helper" in out + + +def test_render_table_empty_shows_header_and_no_rows() -> None: + out = render_table(("name",), []) + assert "name" in out and "(no rows)" in out + + +def test_render_json_shape() -> None: + out = render_json(("name",), [("Repo",)], truncated=True, stopped_reason="row_cap") + data = json.loads(out) + assert data == { + "columns": ["name"], + "rows": [["Repo"]], + "truncated": True, + "stopped_reason": "row_cap", + } + + +# --- CLI end-to-end --------------------------------------------------------- + + +def test_graph_query_table(indexed: Path, capsys: pytest.CaptureFixture[str]) -> None: + rc = main(["query", "--path", str(indexed), "--graph", "MATCH (c:Class) RETURN c.name"]) + out = capsys.readouterr().out + assert rc == 0 + assert "Repo" in out and "Cache" in out and "c.name" in out + + +def test_graph_query_json(indexed: Path, capsys: pytest.CaptureFixture[str]) -> None: + rc = main( + [ + "query", + "--path", + str(indexed), + "--graph", + "MATCH (m:Method) RETURN m.name", + "--format", + "json", + ] + ) + data = json.loads(capsys.readouterr().out) + assert rc == 0 + assert data["columns"] == ["m.name"] + assert {r[0] for r in data["rows"]} == {"save", "get"} + assert data["truncated"] is False + + +def test_schema_command(indexed: Path, capsys: pytest.CaptureFixture[str]) -> None: + rc = main(["query", "--path", str(indexed), "--schema"]) + out = capsys.readouterr().out + assert rc == 0 + assert "query language v1.0" in out + assert "Class" in out and "CALLS" in out + assert "sym_path" not in out # curated logical names, not physical columns + assert "path" in out + + +def test_schema_json(indexed: Path, capsys: pytest.CaptureFixture[str]) -> None: + rc = main(["query", "--path", str(indexed), "--schema", "--format", "json"]) + data = json.loads(capsys.readouterr().out) + assert rc == 0 + assert data["query_lang_version"] == "1.0" + assert "Class" in data["node_kinds"] + + +def test_bad_query_exits_2(indexed: Path, capsys: pytest.CaptureFixture[str]) -> None: + rc = main(["query", "--path", str(indexed), "--graph", "MATCH (c:Bogus) RETURN c.name"]) + assert rc == 2 + assert "query error" in capsys.readouterr().err + + +def test_limit_caps_rows(indexed: Path, capsys: pytest.CaptureFixture[str]) -> None: + rc = main( + [ + "query", + "--path", + str(indexed), + "--graph", + "MATCH (n:Method) RETURN n.name", + "--limit", + "1", + ] + ) + err = capsys.readouterr().err + assert rc == 0 + assert "truncated" in err # 2 methods, capped to 1 + + +def test_limit_without_graph_errors(indexed: Path, capsys: pytest.CaptureFixture[str]) -> None: + rc = main(["query", "--path", str(indexed), "--limit", "5"]) + assert rc == 2 + assert "--limit only applies to --graph" in capsys.readouterr().err From ed2f642ff9c9670aa8edc32de49e72f29c9b5e08 Mon Sep 17 00:00:00 2001 From: Khemchand Joshi Date: Wed, 8 Jul 2026 12:34:34 +0530 Subject: [PATCH 05/11] feat-015: ckg_query MCP tool + capability-gated registration (chunk 6) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The agent surface for the read-only query engine, plus the honest tool-gating seam. - CkgQuery tool (QueryInput {query, limit}); engine.query_graph folds the staleness envelope + tool_api_version + query_lang_version and returns a STRUCTURED error rather than raising into the tool layer. ckg_status now reports query.{enabled, lang_version, capabilities}. - Capability-driven registration (not always-register-then-error): _tools_for filters ALL_TOOLS by `tool.requires <= backend capabilities`, so ckg_query is present exactly when the backend is query-capable and cleanly absent otherwise. _store_capabilities checks the graph DRIVER CLASS for query_dialect (no index opened); federation offers ckg_query only when every member is query-capable. This is a reusable gate for future capability-scoped tools. - test_schemas is now profile-parameterized: the enabled profile includes ckg_query, the disabled profile excludes it — drift on either path fails CI. Deviation (for the spec at merge): ckg_query v1 has no `params` argument (the design's open-Q1 leaned yes). The grammar inlines and parameterizes literals internally and exposes no $placeholder syntax, so a caller-facing params map has no binding site in v1 — honest to omit it. Gate: 1064 passed, 45 skipped, 94.72% coverage; ruff + mypy --strict clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/agentforge_graph/serve/engine.py | 43 +++++++++++++++++ src/agentforge_graph/serve/server.py | 41 +++++++++++++--- src/agentforge_graph/serve/tools.py | 38 ++++++++++++++- tests/serve/test_binding.py | 1 + tests/serve/test_query_tool.py | 70 ++++++++++++++++++++++++++++ tests/serve/test_schemas.py | 34 ++++++++++---- 6 files changed, 211 insertions(+), 16 deletions(-) create mode 100644 tests/serve/test_query_tool.py diff --git a/src/agentforge_graph/serve/engine.py b/src/agentforge_graph/serve/engine.py index 4f36d23..06f4f36 100644 --- a/src/agentforge_graph/serve/engine.py +++ b/src/agentforge_graph/serve/engine.py @@ -216,7 +216,45 @@ async def history(self, symbol_id: str) -> dict[str, Any]: ) return {**body, **(await self.staleness()), "tool_api_version": TOOL_API_VERSION} + async def query_graph(self, query: str, limit: int | None = None) -> dict[str, Any]: + """feat-015: run a read-only structural query, wrapped in the staleness + envelope. A ``QueryError`` (bad syntax / vocabulary / capability / a + non-query backend) is returned as a structured ``error`` — never raised + into the tool layer.""" + from agentforge_graph.store.query import ( + QUERY_LANG_VERSION, + QueryError, + QuerySettings, + ) + + cg = await self.code_graph() + settings = QuerySettings() + if limit is not None: + settings = QuerySettings( + max_rows=min(settings.max_rows, max(0, limit)), + timeout_ms=settings.timeout_ms, + max_expansions=settings.max_expansions, + ) + envelope = { + **(await self.staleness()), + "tool_api_version": TOOL_API_VERSION, + "query_lang_version": QUERY_LANG_VERSION, + } + try: + rt = await cg.query_graph(query, settings) + except QueryError as exc: + return {"error": str(exc), **envelope} + return { + "columns": list(rt.columns), + "rows": [list(r) for r in rt.rows], + "truncated": rt.truncated, + "stopped_reason": rt.stopped_reason, + **envelope, + } + async def status(self) -> dict[str, Any]: + from agentforge_graph.store.query import QUERY_LANG_VERSION + meta = self._meta() cg = await self.code_graph() nodes = (await cg.store.graph.query(GraphQuery(limit=_ALL))).nodes @@ -235,6 +273,11 @@ async def status(self) -> dict[str, Any]: "by_kind": by_kind, "temporal": await cg.temporal_status(), "store_path": str(store_root), + "query": { + "enabled": cg.query_enabled, + "lang_version": QUERY_LANG_VERSION, + "capabilities": sorted(cg.query_capabilities), + }, "tool_api_version": TOOL_API_VERSION, } diff --git a/src/agentforge_graph/serve/server.py b/src/agentforge_graph/serve/server.py index e0f5efd..6872bb3 100644 --- a/src/agentforge_graph/serve/server.py +++ b/src/agentforge_graph/serve/server.py @@ -31,16 +31,38 @@ Transport = Literal["stdio", "http"] -def _tools_for(engine: EngineProvider) -> list[Tool]: +def _tools_for(engine: EngineProvider, capabilities: frozenset[str]) -> list[Tool]: # ALL_TOOLS holds concrete Tool subclasses; mypy joins them to the abstract - # base and can't see that, hence the abstract ignore. - return [tool_cls(engine) for tool_cls in ALL_TOOLS] # type: ignore[abstract] + # base and can't see that, hence the abstract ignore. A tool is included only + # when the backend provides every capability it `requires` (feat-015), so a + # capability-gated tool is present exactly when it is actually usable — never + # discovered as a tool that only errors. + return [ + tool_cls(engine) # type: ignore[abstract] + for tool_cls in ALL_TOOLS + if tool_cls.requires <= capabilities + ] + + +def _store_capabilities(config: ConfigSource, repo_path: str | Path) -> frozenset[str]: + """Serve-level capability markers for a store config — currently just + ``query`` when the resolved graph driver is query-capable (feat-015). + Resolved from the driver *class*, so no index is opened.""" + from agentforge_graph.config import StoreConfig, resolve_config + from agentforge_graph.store.registry import graph_driver + + try: + cfg = StoreConfig.load(resolve_config(config, repo_path)) + driver = graph_driver(cfg.graph.driver) + except Exception: + return frozenset() + return frozenset({"query"}) if hasattr(driver, "query_dialect") else frozenset() def code_graph_tools(repo_path: str | Path = ".", config: ConfigSource = None) -> list[Tool]: """The CKG toolset as native AgentForge ``Tool`` instances, sharing one lazily-opened engine. Pass straight to ``Agent(tools=code_graph_tools("."))``.""" - return _tools_for(_Engine(repo_path, config)) + return _tools_for(_Engine(repo_path, config), _store_capabilities(config, repo_path)) def federated_tools(workspace: str | Path) -> list[Tool]: @@ -48,9 +70,16 @@ def federated_tools(workspace: str | Path) -> list[Tool]: fan across every member; pinpoint tools take a ``service`` to pick one; plus the federation-only ``ckg_services_map`` (cross-service call graph). One endpoint for the whole org.""" - fed = FederatedEngine.from_workspace(WorkspaceConfig.load(workspace)) + ws = WorkspaceConfig.load(workspace) + fed = FederatedEngine.from_workspace(ws) + # A capability holds for the federation only if every member provides it, so + # ckg_query is offered only when all members are query-capable. + # resolve_member_config returns a ResolvedConfig, which resolve_config passes + # through (repo_path is unused for an already-resolved config). + member_caps = [_store_capabilities(ws.resolve_member_config(m), ".") for m in ws.members] + caps = frozenset.intersection(*member_caps) if member_caps else frozenset() # the cross-service tools are appended only here — single-repo tool set stays v1. - return [*_tools_for(fed), CkgServicesMap(fed), CkgTrace(fed)] + return [*_tools_for(fed, caps), CkgServicesMap(fed), CkgTrace(fed)] def build_mcp_server( diff --git a/src/agentforge_graph/serve/tools.py b/src/agentforge_graph/serve/tools.py index 1217669..8610bf3 100644 --- a/src/agentforge_graph/serve/tools.py +++ b/src/agentforge_graph/serve/tools.py @@ -83,12 +83,23 @@ class HistoryInput(_Fed): symbol_id: str = Field(description="exact symbol id whose git history to report") +class QueryInput(_Fed): + query: str = Field( + description="a read-only structural query in the Cypher subset " + "(MATCH … WHERE … RETURN …); see ckg_status for the language version" + ) + limit: int | None = Field(default=None, description="cap rows (clamped to the server max)") + + class EmptyInput(_Fed): pass class _CkgTool(Tool): capabilities: ClassVar[frozenset[str]] = frozenset() + # Serve-level capability gates: this tool is registered only when the backend + # provides every marker here (feat-015 capability-driven registration). + requires: ClassVar[frozenset[str]] = frozenset() def __init__(self, engine: EngineProvider) -> None: self._engine = engine @@ -469,8 +480,30 @@ async def run(self, **kwargs: Any) -> str: return json.dumps(await fn(), indent=2) -# The locked v1 tool set (single-repo). ``ckg_services_map`` is federation-only -# and appended by ``federated_tools`` (ENH-020), so a single repo is unchanged. +class CkgQuery(_CkgTool): + name: ClassVar[str] = "ckg_query" + description: ClassVar[str] = ( + "Escape hatch for PRECISE STRUCTURAL questions no typed verb covers — e.g. " + "'classes tagged Repository with no inbound CALLS', 'interfaces implemented by " + ">5 classes'. Read-only Cypher subset: MATCH patterns over the graph's node/edge " + "kinds, WHERE (comparisons, IN, STARTS/ENDS WITH, CONTAINS, pattern existence), " + "RETURN with count/min/max/avg, ORDER BY, LIMIT. For semantic 'find code about X' " + "use ckg_search instead; for 'who calls this' use ckg_impact." + ) + input_schema: ClassVar[type[BaseModel]] = QueryInput + requires: ClassVar[frozenset[str]] = frozenset({"query"}) + + async def run(self, **kwargs: Any) -> str: + eng, err = self._resolve_one(kwargs) + if eng is None: + return err or "" + result = await eng.query_graph(kwargs["query"], kwargs.get("limit")) + return json.dumps(result, indent=2) + + +# The locked v1 tool set (single-repo). ``ckg_query`` is capability-gated — +# registered only when the backend is query-capable (feat-015). ``ckg_services_map`` +# is federation-only and appended by ``federated_tools`` (ENH-020). ALL_TOOLS = [ CkgRepoMap, CkgSearch, @@ -482,4 +515,5 @@ async def run(self, **kwargs: Any) -> str: CkgDecisions, CkgExplain, CkgHistory, + CkgQuery, ] diff --git a/tests/serve/test_binding.py b/tests/serve/test_binding.py index 5d5d3e3..f4cd2c8 100644 --- a/tests/serve/test_binding.py +++ b/tests/serve/test_binding.py @@ -23,6 +23,7 @@ "ckg_decisions", "ckg_explain", "ckg_history", + "ckg_query", # feat-015: present because the default (kuzu) backend is query-capable } diff --git a/tests/serve/test_query_tool.py b/tests/serve/test_query_tool.py new file mode 100644 index 0000000..b9c2a9a --- /dev/null +++ b/tests/serve/test_query_tool.py @@ -0,0 +1,70 @@ +"""feat-015 chunk 6: the ckg_query MCP tool + engine.query_graph. + +Functional over a real indexed repo (embedded Kuzu). Asserts the staleness + +query-language envelope, structured errors (never raised into the tool layer), +and that ckg_status reports the query surface. +""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + +from agentforge_graph.ingest import CodeGraph +from agentforge_graph.serve.engine import _Engine +from agentforge_graph.serve.tools import CkgQuery, CkgStatus + +_SRC = """\ +class Repo: + def save(self): ... + +class Cache: + def get(self): ... +""" + + +@pytest.fixture +async def repo(tmp_path: Path) -> Path: + (tmp_path / "app.py").write_text(_SRC) + cg = await CodeGraph.index(repo_path=tmp_path) + await cg.close() + return tmp_path + + +def _engine(repo: Path) -> _Engine: + return _Engine(repo) + + +async def test_query_tool_returns_rows_with_envelope(repo: Path) -> None: + tool = CkgQuery(_engine(repo)) + out = json.loads(await tool.run(query="MATCH (c:Class) RETURN c.name")) + assert set(out["columns"]) == {"c.name"} + assert {r[0] for r in out["rows"]} == {"Repo", "Cache"} + assert out["truncated"] is False + # envelope: staleness + tool/query-language version + assert "indexed_commit" in out and "dirty" in out + assert out["tool_api_version"] == "1.0" + assert out["query_lang_version"] == "1.0" + + +async def test_query_tool_limit(repo: Path) -> None: + tool = CkgQuery(_engine(repo)) + out = json.loads(await tool.run(query="MATCH (c:Class) RETURN c.name", limit=1)) + assert len(out["rows"]) == 1 + assert out["truncated"] is True and out["stopped_reason"] == "row_cap" + + +async def test_query_tool_error_is_structured_not_raised(repo: Path) -> None: + tool = CkgQuery(_engine(repo)) + out = json.loads(await tool.run(query="MATCH (c:Bogus) RETURN c.name")) + assert "error" in out and "Bogus" in out["error"] + assert out["tool_api_version"] == "1.0" # envelope still attached + + +async def test_status_reports_query_surface(repo: Path) -> None: + out = json.loads(await CkgStatus(_engine(repo)).run()) + assert out["query"]["enabled"] is True + assert out["query"]["lang_version"] == "1.0" + assert "core" in out["query"]["capabilities"] diff --git a/tests/serve/test_schemas.py b/tests/serve/test_schemas.py index 4c031d5..ed99da6 100644 --- a/tests/serve/test_schemas.py +++ b/tests/serve/test_schemas.py @@ -1,13 +1,19 @@ -"""Locked tool contracts: tool names + input-schema params. Drift fails CI.""" +"""Locked tool contracts: tool names + input-schema params. Drift fails CI. + +feat-015: the tool set is capability-gated — ``ckg_query`` is present only when +the backend is query-capable. The set is asserted for both profiles (enabled / +disabled) so drift on either path fails CI, without depending on the ambient cwd. +""" from __future__ import annotations -from agentforge_graph.serve import code_graph_tools +from agentforge_graph.serve.engine import _Engine +from agentforge_graph.serve.server import _tools_for # (properties, required) per tool — the v1 contract. Every tool also carries the # optional ENH-020 ``service`` field (federated member selector; inert for a # single repo), added uniformly below. -_EXPECTED: dict[str, tuple[set[str], set[str]]] = { +_BASE: dict[str, tuple[set[str], set[str]]] = { "ckg_repo_map": ({"budget_tokens", "focus"}, set()), "ckg_search": ({"query", "k", "mode"}, {"query"}), "ckg_symbol": ({"symbol_id", "name", "path"}, set()), @@ -19,16 +25,28 @@ "ckg_explain": ({"symbol_id"}, {"symbol_id"}), "ckg_history": ({"symbol_id"}, {"symbol_id"}), # feat-009 chunk 3 } +# feat-015: ckg_query is present only in the query-capable profile. +_QUERY = {"ckg_query": ({"query", "limit"}, {"query"})} +_ALL = {**_BASE, **_QUERY} # ENH-020: the federated member selector is present on every tool, never required. -_EXPECTED = {name: (props | {"service"}, required) for name, (props, required) in _EXPECTED.items()} +_EXPECTED = {name: (props | {"service"}, required) for name, (props, required) in _ALL.items()} + +_ENABLED = frozenset({"query"}) +_DISABLED: frozenset[str] = frozenset() + + +def _names(capabilities: frozenset[str]) -> set[str]: + return {t.name for t in _tools_for(_Engine("."), capabilities)} -def test_tool_set_is_exactly_v1() -> None: - assert {t.name for t in code_graph_tools(".")} == set(_EXPECTED) +def test_query_tool_present_only_when_capable() -> None: + assert _names(_ENABLED) == set(_EXPECTED) + assert _names(_DISABLED) == set(_BASE) # ckg_query gated out + assert "ckg_query" not in _names(_DISABLED) def test_tool_schemas_match_contract() -> None: - tools = {t.name: t for t in code_graph_tools(".")} + tools = {t.name: t for t in _tools_for(_Engine("."), _ENABLED)} for name, (props, required) in _EXPECTED.items(): schema = type(tools[name]).input_schema.model_json_schema() assert set(schema.get("properties", {})) == props, name @@ -36,5 +54,5 @@ def test_tool_schemas_match_contract() -> None: def test_every_tool_has_an_llm_description() -> None: - for t in code_graph_tools("."): + for t in _tools_for(_Engine("."), _ENABLED): assert len(t.description) > 40 # written for tool-choice, not a stub From 86460ca078e0d84ee48f413cd3c851b546fa5e46 Mon Sep 17 00:00:00 2001 From: Khemchand Joshi Date: Wed, 8 Jul 2026 12:40:04 +0530 Subject: [PATCH 06/11] feat-015: Neo4j execution + read-only session (chunk 3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The second query-capable backend, proving the Cypher compiler is portable. - execute.py: pull_bounded_async + run_bounded_async — the async twin of the bounded pull, for backends whose driver is async (Neo4j, and SurrealDB next). Same row/expansion/timeout policy; hard wait_for backstop -> GuardrailError. - Neo4jGraphStore.run_query: compiles via Neo4jCypherCompiler (the shared Cypher body — Neo4j uses the identical single-table schema, so no dialect deltas were needed) and executes inside a genuine READ transaction (execute_read), so read-only is enforced by the session (gate #2) on top of the AST gate (gate #1). QueryCapable: dialect=neo4j, caps=core+agg.collect, read_only_execution=True. - Neo4j QueryConformance subclass (env-gated on CKG_NEO4J_URI). Verified against a live Neo4j 5 container: all 11 conformance tests pass — the same query set returns the same rows as Kuzu (result parity), the runaway query trips the row/expansion caps, and writes are rejected. Two of three backends now proven result-identical. Unit gate green; ruff + mypy --strict clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/agentforge_graph/store/neo4j_store.py | 47 ++++++++++++++++++++- src/agentforge_graph/store/query/execute.py | 47 ++++++++++++++++++++- tests/store/test_neo4j_query_conformance.py | 41 ++++++++++++++++++ 3 files changed, 133 insertions(+), 2 deletions(-) create mode 100644 tests/store/test_neo4j_query_conformance.py diff --git a/src/agentforge_graph/store/neo4j_store.py b/src/agentforge_graph/store/neo4j_store.py index 4fa84a7..416d44c 100644 --- a/src/agentforge_graph/store/neo4j_store.py +++ b/src/agentforge_graph/store/neo4j_store.py @@ -43,6 +43,10 @@ node_from_row, node_params, ) +from .query.ast import QueryAst +from .query.capability import AGG_COLLECT, CORE_TIER, QuerySettings, ResultTable +from .query.compile_cypher import Neo4jCypherCompiler +from .query.execute import BoundedResult, pull_bounded_async, run_bounded_async if TYPE_CHECKING: from neo4j import AsyncDriver, AsyncManagedTransaction @@ -69,7 +73,17 @@ class Neo4jGraphStore(GraphStore): - """Server graph store backed by Neo4j (Bolt).""" + """Server graph store backed by Neo4j (Bolt). + + Query-capable (feat-015): the same openCypher compiler as Kuzu (identical + single-table schema), executed inside a genuine **read transaction** + (``execute_read``) — so read-only is enforced by the session (gate #2) on top + of the AST gate (gate #1).""" + + # --- QueryCapable (feat-015) --- + query_dialect = "neo4j" + capabilities = CORE_TIER | {AGG_COLLECT} + read_only_execution = True def __init__(self, driver: AsyncDriver, database: str) -> None: self._driver = driver @@ -287,6 +301,37 @@ async def _read(self, cypher: str, params: dict[str, Any]) -> list[Any]: result = await session.run(cypher, params) return [r async for r in result] + async def run_query(self, ast: QueryAst, settings: QuerySettings) -> ResultTable: + compiler = Neo4jCypherCompiler() + compiled = compiler.compile(ast, settings) + effective_limit = compiler.effective_limit(ast, settings) + + async def _tx(tx: AsyncManagedTransaction) -> BoundedResult: + result = await tx.run(compiled.text, compiled.params) + + async def anext_row() -> tuple[Any, ...] | None: + try: + record = await result.__anext__() + except StopAsyncIteration: + return None + return tuple(record.values()) + + return await pull_bounded_async(anext_row, settings, effective_limit) + + async def pull() -> BoundedResult: + # execute_read runs a genuine READ transaction — a write would be + # refused by the session (gate #2), on top of the AST gate (gate #1). + async with self._driver.session(database=self._database) as session: + return await session.execute_read(_tx) + + bounded = await run_bounded_async(pull, settings) + return ResultTable( + columns=compiled.columns, + rows=tuple(bounded.rows), + truncated=bounded.truncated, + stopped_reason=bounded.stopped_reason, + ) + async def close(self) -> None: if self._closed: return diff --git a/src/agentforge_graph/store/query/execute.py b/src/agentforge_graph/store/query/execute.py index e2a3300..7ee94cf 100644 --- a/src/agentforge_graph/store/query/execute.py +++ b/src/agentforge_graph/store/query/execute.py @@ -21,7 +21,7 @@ import asyncio import time -from collections.abc import Callable, Iterator +from collections.abc import Awaitable, Callable, Iterator from dataclasses import dataclass from typing import Any @@ -71,6 +71,36 @@ def pull_bounded( return BoundedResult(rows=rows, truncated=reason is not None, stopped_reason=reason) +async def pull_bounded_async( + anext_row: Callable[[], Awaitable[Row | None]], + settings: QuerySettings, + effective_limit: int, + now: Callable[[], float] = time.monotonic, +) -> BoundedResult: + """Async twin of ``pull_bounded`` for backends whose driver is async (Neo4j, + SurrealDB). Same row/expansion/timeout policy; the caller supplies a hard + ``asyncio.wait_for`` backstop around the whole pull.""" + deadline = now() + settings.timeout_ms / 1000 + rows: list[Row] = [] + reason: str | None = None + while True: + if now() >= deadline: + reason = "timeout" + break + if len(rows) >= settings.max_expansions: + reason = "expansion_cap" + break + row = await anext_row() + if row is None: + break + rows.append(row) + if len(rows) > effective_limit: + reason = "row_cap" + rows = rows[:effective_limit] + break + return BoundedResult(rows=rows, truncated=reason is not None, stopped_reason=reason) + + async def run_bounded( make_source: Callable[[], Iterator[Row]], settings: QuerySettings, @@ -92,3 +122,18 @@ def work() -> BoundedResult: ) except TimeoutError as exc: # a single fetch hung past the hard backstop raise GuardrailError(f"query exceeded the {settings.timeout_ms}ms time budget") from exc + + +async def run_bounded_async( + pull: Callable[[], Awaitable[BoundedResult]], + settings: QuerySettings, +) -> BoundedResult: + """Wrap an async bounded pull (``pull_bounded_async``) with the hard + ``wait_for`` backstop, so a hung fetch raises ``GuardrailError`` instead of + hanging forever. For backends with an async driver (Neo4j, SurrealDB).""" + try: + return await asyncio.wait_for( + pull(), timeout=settings.timeout_ms / 1000 + _HARD_TIMEOUT_GRACE_S + ) + except TimeoutError as exc: + raise GuardrailError(f"query exceeded the {settings.timeout_ms}ms time budget") from exc diff --git a/tests/store/test_neo4j_query_conformance.py b/tests/store/test_neo4j_query_conformance.py new file mode 100644 index 0000000..f73bebb --- /dev/null +++ b/tests/store/test_neo4j_query_conformance.py @@ -0,0 +1,41 @@ +"""Run feat-015's QueryConformance against the Neo4j adapter (chunk 3). + +Env-gated like the storage conformance: skipped unless ``CKG_NEO4J_URI`` points +at a reachable Neo4j. Proves the Cypher compiler + bounds are backend-portable — +the same query set must return the same rows as Kuzu. + + docker run -d --name ckg-neo4j -e NEO4J_AUTH=neo4j/ckgckgckg -p 7688:7687 neo4j:5 + CKG_NEO4J_URI=bolt://localhost:7688 CKG_NEO4J_PASSWORD=ckgckgckg \\ + uv run pytest tests/store/test_neo4j_query_conformance.py +""" + +from __future__ import annotations + +import os +from collections.abc import AsyncIterator + +import pytest + +from agentforge_graph.store.query.conformance import QueryConformance + +_URI = os.environ.get("CKG_NEO4J_URI") +pytestmark = pytest.mark.skipif(not _URI, reason="set CKG_NEO4J_URI to run Neo4j query conformance") + + +class TestNeo4jQuery(QueryConformance): + @pytest.fixture + async def store(self) -> AsyncIterator: + from agentforge_graph.store.neo4j_store import Neo4jGraphStore + + config = { + "uri": _URI or "", + "user": os.environ.get("CKG_NEO4J_USER", "neo4j"), + "password": os.environ.get("CKG_NEO4J_PASSWORD", ""), + } + store = await Neo4jGraphStore.open("unused", config) + async with store._driver.session(database=store._database) as s: + await s.run("MATCH (n:CkgNode) DETACH DELETE n") # fresh graph per test + try: + yield store + finally: + await store.close() From 0d1c1c17b7c47f7f7f755aa5c55ec1a10bd4ff38 Mon Sep 17 00:00:00 2001 From: Khemchand Joshi Date: Wed, 8 Jul 2026 13:04:51 +0530 Subject: [PATCH 07/11] feat-015: portable AST interpreter + SurrealDB query support (chunk 4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The third query-capable backend — and a universal query path. SurrealDB models edges as a document table (src/dst fields) with no native graph traversal to compile to, so rather than a fragile SurrealQL translator it *interprets* the query. - interpret.py: InterpretingQueryEngine evaluates a validated QueryAst using only the locked GraphStore ABC read methods (query/adjacent/get). This makes query support universal — any GraphStore is query-capable for free — and it is inherently read-only (no write path through the ABC). Same row/expansion/ timeout bounds as the compiled path, and it passes the SAME QueryConformance suite, so results are identical to the canonical rows. "Compile where the backend has a native query language (Kuzu/Neo4j); interpret elsewhere." - SurrealGraphStore.run_query delegates to the interpreter (dialect=interpreted, read_only_execution=True — the interpreter cannot write). - Interpreter coverage without a server: test_interpret_conformance runs it over embedded Kuzu (always-on CI gate), plus test_interpret unit tests for the branches the parity suite doesn't hit (avg/min/max/collect, DISTINCT, ORDER BY a non-projected property, attrs access, timeout backstop, node_property). - Live: SurrealDB QueryConformance (env-gated) — 12 tests pass against a real SurrealDB 2 container; identical rows to Kuzu and Neo4j. Parser bugfix (shared, both paths): CONTAINS is both the string operator and an EdgeKind, so the tokenizer made it a keyword and [:CONTAINS] failed to parse. Added _name() so labels/relationship types accept reserved words; locked across all three backends with a File-[:CONTAINS]->Class conformance test. All three backends — Kuzu (compiled), Neo4j (compiled), SurrealDB (interpreted) — now return identical rows. Gate: 1084 passed, 94.33% coverage; ruff + mypy clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/agentforge_graph/store/query/__init__.py | 2 + .../store/query/conformance.py | 14 +- src/agentforge_graph/store/query/interpret.py | 401 ++++++++++++++++++ src/agentforge_graph/store/query/parser.py | 14 +- src/agentforge_graph/store/surreal_store.py | 20 +- tests/store/query/test_interpret.py | 133 ++++++ .../store/query/test_interpret_conformance.py | 55 +++ tests/store/test_surreal_query_conformance.py | 44 ++ 8 files changed, 679 insertions(+), 4 deletions(-) create mode 100644 src/agentforge_graph/store/query/interpret.py create mode 100644 tests/store/query/test_interpret.py create mode 100644 tests/store/query/test_interpret_conformance.py create mode 100644 tests/store/test_surreal_query_conformance.py diff --git a/src/agentforge_graph/store/query/__init__.py b/src/agentforge_graph/store/query/__init__.py index 1ccf0df..6ec9d0d 100644 --- a/src/agentforge_graph/store/query/__init__.py +++ b/src/agentforge_graph/store/query/__init__.py @@ -29,6 +29,7 @@ QueryError, ValidationError, ) +from .interpret import InterpretingQueryEngine from .parser import parse_query from .schema import QUERY_LANG_VERSION, SchemaDescription, describe_schema from .validator import validate_query @@ -38,6 +39,7 @@ "parse_query", "validate_query", "describe_schema", + "InterpretingQueryEngine", # types "QueryAst", "QueryCapable", diff --git a/src/agentforge_graph/store/query/conformance.py b/src/agentforge_graph/store/query/conformance.py index b0369cb..01d6b0d 100644 --- a/src/agentforge_graph/store/query/conformance.py +++ b/src/agentforge_graph/store/query/conformance.py @@ -50,6 +50,7 @@ def _node(descriptor: str, kind: NodeKind, name: str, span: tuple[int, int] = (1 def make_query_fixture() -> FileSubgraph: """A small graph exercising every core construct: classes implementing an interface, a call chain (for var-length), an orphan, and a tag.""" + file = _node("", NodeKind.FILE, "app.py") repo = _node(Descriptor.type("Repo"), NodeKind.CLASS, "Repo") cache = _node(Descriptor.type("Cache"), NodeKind.CLASS, "Cache") plain = _node(Descriptor.type("Plain"), NodeKind.CLASS, "Plain") @@ -60,8 +61,13 @@ def make_query_fixture() -> FileSubgraph: baz = _node(Descriptor.term("baz"), NodeKind.FUNCTION, "baz") qux = _node(Descriptor.term("qux"), NodeKind.FUNCTION, "qux") tag = _node(Descriptor.term("Repository"), NodeKind.PATTERN_TAG, "Repository") - nodes = [repo, cache, plain, store_if, empty_if, foo, bar, baz, qux, tag] + nodes = [file, repo, cache, plain, store_if, empty_if, foo, bar, baz, qux, tag] edges = [ + # CONTAINS is both an EdgeKind and the CONTAINS string operator — including + # it proves the parser accepts a reserved word in relationship-type position. + Edge(src=file.id, dst=repo.id, kind=EdgeKind.CONTAINS, provenance=_PROV), + Edge(src=file.id, dst=cache.id, kind=EdgeKind.CONTAINS, provenance=_PROV), + Edge(src=file.id, dst=plain.id, kind=EdgeKind.CONTAINS, provenance=_PROV), Edge(src=repo.id, dst=store_if.id, kind=EdgeKind.IMPLEMENTS, provenance=_PROV), Edge(src=cache.id, dst=store_if.id, kind=EdgeKind.IMPLEMENTS, provenance=_PROV), Edge(src=foo.id, dst=bar.id, kind=EdgeKind.CALLS, provenance=_PROV), @@ -147,6 +153,12 @@ async def test_tagged_classes(self, store: QueryCapable) -> None: ) assert self._col0(rt) == {"Repo"} + async def test_reserved_word_edge_kind(self, store: QueryCapable) -> None: + # CONTAINS is a reserved word (string operator) *and* an edge kind. + await self._load(store) + rt = await self._run(store, "MATCH (f:File)-[:CONTAINS]->(c:Class) RETURN c.name") + assert self._col0(rt) == {"Repo", "Cache", "Plain"} + # --- 2. bounded execution ---------------------------------------------- async def test_row_cap_truncates(self, store: QueryCapable) -> None: diff --git a/src/agentforge_graph/store/query/interpret.py b/src/agentforge_graph/store/query/interpret.py new file mode 100644 index 0000000..cc20563 --- /dev/null +++ b/src/agentforge_graph/store/query/interpret.py @@ -0,0 +1,401 @@ +"""A portable query interpreter over the ``GraphStore`` ABC (feat-015). + +Kuzu and Neo4j speak openCypher, so they *compile* the AST to native Cypher. +Other backends do not — SurrealDB, for instance, models edges as a document +table (``src``/``dst`` string fields), with no native graph traversal to compile +to. Rather than a fragile per-dialect translator, those backends *interpret* the +AST: this engine evaluates a validated ``QueryAst`` using only the locked +``GraphStore`` read methods (``query``/``adjacent``/``get``), in Python. + +That makes query support **universal** — any ``GraphStore`` is query-capable for +free — and it is inherently **read-only** (there is no write path through the +ABC). Both the compiled and interpreted paths pass the same ``QueryConformance`` +suite, so their results are identical to the canonical rows. + +Bounds (row cap / expansion cap / timeout) are enforced here just as the compiled +path enforces them in ``execute.py``: intermediate bindings are capped at +``max_expansions``, the wall-clock deadline is checked between steps, and the row +cap trims the final rows (all reported via ``stopped_reason``). +""" + +from __future__ import annotations + +import time +from collections.abc import Callable +from typing import Any + +from agentforge_graph.core import EdgeKind, GraphQuery, GraphStore, Node, NodeKind, SymbolID + +from .ast import ( + Aggregate, + BoolOp, + Compare, + Expr, + InList, + NodePattern, + Not, + PathPattern, + PatternExists, + PropRef, + QueryAst, + RelPattern, + ReturnExpr, + StringPred, + VarRef, +) +from .capability import QuerySettings, ResultTable + +Binding = dict[str, Node] +Row = tuple[Any, ...] + +_COMPARATORS: dict[str, Callable[[Any, Any], bool]] = { + "=": lambda a, b: a == b, + "<>": lambda a, b: a != b, + "<": lambda a, b: a < b, + "<=": lambda a, b: a <= b, + ">": lambda a, b: a > b, + ">=": lambda a, b: a >= b, +} + + +def node_property(node: Node, path: tuple[str, ...]) -> Any: + """Resolve a logical property path against a ``Node`` — the interpreter twin + of the compiler's logical-name -> physical-column map.""" + if path[0] == "attrs": + value: Any = node.attrs + for seg in path[1:]: + if not isinstance(value, dict): + return None + value = value.get(seg) + return value + name = path[0] + p = node.provenance + return { + "name": node.name, + "kind": node.kind.value, + "path": SymbolID.parse(node.id).path, + "start_line": node.span[0] if node.span else None, + "end_line": node.span[1] if node.span else None, + "source": p.source.value, + "extractor": p.extractor, + "commit": p.commit, + "confidence": p.confidence, + }.get(name) + + +class InterpretingQueryEngine: + """Evaluates a validated ``QueryAst`` against a ``GraphStore`` via the ABC.""" + + def __init__(self, store: GraphStore) -> None: + self._store = store + + async def run( + self, + ast: QueryAst, + settings: QuerySettings, + now: Callable[[], float] = time.monotonic, + ) -> ResultTable: + self._settings = settings + self._deadline = now() + settings.timeout_ms / 1000 + self._now = now + self._stopped: str | None = None + + bindings = await self._match_all(ast) + if ast.where is not None: + kept = [] + for b in bindings: + if await self._truth(ast.where, b): + kept.append(b) + bindings = kept + columns, rows = self._project(ast, bindings) + if ast.skip: + rows = rows[ast.skip :] + limit = self._effective_limit(ast) + truncated = self._stopped is not None or len(rows) > limit + reason = self._stopped or ("row_cap" if len(rows) > limit else None) + return ResultTable( + columns=tuple(columns), + rows=tuple(rows[:limit]), + truncated=truncated, + stopped_reason=reason, + ) + + def _effective_limit(self, ast: QueryAst) -> int: + if ast.limit is None: + return self._settings.max_rows + return min(ast.limit, self._settings.max_rows) + + def _over_budget(self, count: int) -> bool: + if self._now() >= self._deadline: + self._stopped = "timeout" + return True + if count > self._settings.max_expansions: + self._stopped = "expansion_cap" + return True + return False + + # --- MATCH -------------------------------------------------------------- + + async def _match_all(self, ast: QueryAst) -> list[Binding]: + bindings: list[Binding] = [{}] + for pattern in ast.match: + extended: list[Binding] = [] + for b in bindings: + for nb in await self._extend_pattern(pattern, b): + extended.append(nb) + if self._over_budget(len(extended)): + return extended[: self._settings.max_expansions] + bindings = extended + return bindings + + async def _extend_pattern(self, pattern: PathPattern, base: Binding) -> list[Binding]: + els = pattern.elements + # state: (binding, current_node) — carries the node the next rel starts from + state: list[tuple[Binding, Node]] = list(await self._bind_first(els[0], base)) + idx = 1 + while idx < len(els): + rel = els[idx] + nxt = els[idx + 1] + idx += 2 + assert isinstance(rel, RelPattern) and isinstance(nxt, NodePattern) + new_state: list[tuple[Binding, Node]] = [] + for b, cur in state: + for tid in await self._reachable(cur.id, rel): + target = await self._store.get(tid) + if target is None: + continue + bound = self._bind_specific(nxt, b, target) + if bound is not None: + new_state.append((bound, target)) + state = new_state + return [b for b, _ in state] + + async def _bind_first( + self, node: NodePattern | RelPattern, base: Binding + ) -> list[tuple[Binding, Node]]: + assert isinstance(node, NodePattern) + if node.var is not None and node.var in base: # already bound upstream + fixed = base[node.var] + return [(base, fixed)] if self._matches(node, fixed) else [] + out: list[tuple[Binding, Node]] = [] + for cand in await self._candidates(node): + b = dict(base) + if node.var is not None: + b[node.var] = cand + out.append((b, cand)) + return out + + def _bind_specific(self, node: NodePattern, base: Binding, target: Node) -> Binding | None: + if not self._matches(node, target): + return None + if node.var is not None: + if node.var in base: + return base if base[node.var].id == target.id else None + b = dict(base) + b[node.var] = target + return b + return base + + async def _candidates(self, node: NodePattern) -> list[Node]: + name = next((lit.value for k, lit in node.props if k == "name"), None) + gq = GraphQuery( + kinds=[NodeKind(node.label)] if node.label else None, + name=name if isinstance(name, str) else None, + limit=self._settings.max_expansions + 1, + ) + result = await self._store.query(gq) + return [n for n in result.nodes if self._matches(node, n)] + + def _matches(self, node: NodePattern, n: Node) -> bool: + if node.label is not None and n.kind.value != node.label: + return False + return all(node_property(n, (key,)) == lit.value for key, lit in node.props) + + async def _reachable(self, start_id: str, rel: RelPattern) -> set[str]: + if (rel.min_hops, rel.max_hops) == (1, 1): + return set(await self._one_hop(start_id, rel)) + results: set[str] = set() + frontier = {start_id} + visited = {start_id} + for depth in range(1, (rel.max_hops or 1) + 1): + nxt: set[str] = set() + for nid in frontier: + nxt.update(await self._one_hop(nid, rel)) + if depth >= rel.min_hops: + results |= nxt + frontier = nxt - visited + visited |= nxt + if not frontier: + break + return results + + async def _one_hop(self, cur_id: str, rel: RelPattern) -> list[str]: + kinds = [EdgeKind(rel.kind)] if rel.kind else None + edges = await self._store.adjacent(cur_id, kinds, rel.direction) + out: list[str] = [] + for e in edges: + if rel.direction == "out": + out.append(e.dst) + elif rel.direction == "in": + out.append(e.src) + else: + out.append(e.dst if e.src == cur_id else e.src) + return out + + # --- WHERE -------------------------------------------------------------- + + async def _truth(self, expr: Expr, b: Binding) -> bool: + match expr: + case Compare(lhs, op, rhs): + left = node_property(b[lhs.var], lhs.path) + if left is None: + return False + try: + return _COMPARATORS[op](left, rhs.value) + except TypeError: + return False + case InList(lhs, values): + return node_property(b[lhs.var], lhs.path) in [v.value for v in values] + case StringPred(lhs, op, rhs): + left = node_property(b[lhs.var], lhs.path) + if left is None: + return False + s = str(left) + if op == "STARTS_WITH": + return s.startswith(rhs) + if op == "ENDS_WITH": + return s.endswith(rhs) + return rhs in s + case Not(operand): + return not await self._truth(operand, b) + case BoolOp(op, operands): + if op == "AND": + for o in operands: + if not await self._truth(o, b): + return False + return True + for o in operands: + if await self._truth(o, b): + return True + return False + case PatternExists(pattern): + return bool(await self._extend_pattern(pattern, b)) + raise AssertionError(f"unhandled expr: {type(expr).__name__}") # pragma: no cover + + # --- RETURN / ORDER BY -------------------------------------------------- + + def _project(self, ast: QueryAst, bindings: list[Binding]) -> tuple[list[str], list[Row]]: + columns = [item.alias or self._default_col(item.expr) for item in ast.returns] + if not any(isinstance(it.expr, Aggregate) for it in ast.returns): + # Order the bindings first, so ORDER BY can reference any property of a + # bound variable — even one not projected (matching the compiled path). + bindings = self._order_bindings(ast, bindings) + rows = [tuple(self._value(it.expr, b) for it in ast.returns) for b in bindings] + if ast.distinct: + rows = list(dict.fromkeys(rows)) + return columns, rows + # grouped aggregation: group by the non-aggregate return values + non_agg = [it for it in ast.returns if not isinstance(it.expr, Aggregate)] + groups: dict[Row, list[Binding]] = {} + for b in bindings: + key = tuple(self._value(it.expr, b) for it in non_agg) + groups.setdefault(key, []).append(b) + rows = [] + for key, members in groups.items(): + row: list[Any] = [] + ki = 0 + for it in ast.returns: + if isinstance(it.expr, Aggregate): + row.append(self._aggregate(it.expr, members)) + else: + row.append(key[ki]) + ki += 1 + rows.append(tuple(row)) + return columns, self._order_rows(ast, columns, rows) + + def _value(self, expr: ReturnExpr, b: Binding) -> Any: + if isinstance(expr, PropRef): + return node_property(b[expr.var], expr.path) + if isinstance(expr, VarRef): + return b[expr.var].id + raise AssertionError("aggregate handled in grouping") # pragma: no cover + + def _default_col(self, expr: ReturnExpr) -> str: + if isinstance(expr, PropRef): + return f"{expr.var}.{'.'.join(expr.path)}" + if isinstance(expr, VarRef): + return expr.var + star = "*" if expr.arg is None else self._agg_label(expr.arg) + return f"{expr.func}({star})" + + def _agg_label(self, arg: PropRef | VarRef) -> str: + return arg.var if isinstance(arg, VarRef) else f"{arg.var}.{'.'.join(arg.path)}" + + def _aggregate(self, agg: Aggregate, members: list[Binding]) -> Any: + if agg.arg is None: # count(*) + return len(members) + raw = [self._value(agg.arg, b) for b in members] + values = [v for v in raw if v is not None] + if agg.distinct: + values = list(dict.fromkeys(values)) + if agg.func == "count": + return len(values) + if agg.func == "collect": + return sorted(values, key=_sort_key) + if not values: + return None + if agg.func == "min": + return min(values, key=_sort_key) + if agg.func == "max": + return max(values, key=_sort_key) + return sum(values) / len(values) # avg + + def _order_bindings(self, ast: QueryAst, bindings: list[Binding]) -> list[Binding]: + if not ast.order_by: + return bindings + alias_expr = {it.alias: it.expr for it in ast.returns if it.alias} + ordered = list(bindings) + for ok in reversed(ast.order_by): # stable, least-significant first + ordered.sort(key=self._binding_key(ok.ref, alias_expr), reverse=ok.descending) + return ordered + + def _binding_key( + self, ref: PropRef | VarRef, alias_expr: dict[str, Any] + ) -> Callable[[Binding], tuple[int, Any]]: + return lambda b: _sort_key(self._order_value(ref, b, alias_expr)) + + def _order_value(self, ref: PropRef | VarRef, b: Binding, alias_expr: dict[str, Any]) -> Any: + if isinstance(ref, PropRef): + return node_property(b[ref.var], ref.path) + if ref.var in alias_expr: # a RETURN alias + return self._value(alias_expr[ref.var], b) + return b[ref.var].id # a bound variable + + def _order_rows(self, ast: QueryAst, columns: list[str], rows: list[Row]) -> list[Row]: + # Aggregate results are rows, not bindings, so ORDER BY must name a column + # (a RETURN alias or grouping key), which the validator guarantees. + if not ast.order_by: + return rows + keys: list[tuple[int, bool]] = [] + for ok in ast.order_by: + col = ( + ok.ref.var + if isinstance(ok.ref, VarRef) + else f"{ok.ref.var}.{'.'.join(ok.ref.path)}" + ) + if col in columns: + keys.append((columns.index(col), ok.descending)) + ordered = list(rows) + for idx, descending in reversed(keys): # stable, least-significant first + ordered.sort(key=self._row_key(idx), reverse=descending) + return ordered + + def _row_key(self, idx: int) -> Callable[[Row], tuple[int, Any]]: + return lambda r: _sort_key(r[idx]) + + +def _sort_key(value: Any) -> tuple[int, Any]: + """None-safe, type-stable sort key: None first, then by (type-name, value).""" + if value is None: + return (0, "") + return (1, (type(value).__name__, value)) diff --git a/src/agentforge_graph/store/query/parser.py b/src/agentforge_graph/store/query/parser.py index bbc3ce3..509ba20 100644 --- a/src/agentforge_graph/store/query/parser.py +++ b/src/agentforge_graph/store/query/parser.py @@ -158,6 +158,16 @@ def _expect(self, type_: str, what: str) -> _Token: raise ParseError(f"expected {what}, found {found}", position=tok.pos, query=self.text) return self._advance() + def _name(self, what: str) -> str: + # A label / relationship type may be a reserved word (e.g. the CONTAINS + # edge kind clashes with the CONTAINS string operator), so accept an + # identifier *or* any keyword token in name position. + tok = self.toks[self.i] + if tok.type == "IDENT" or tok.type in _KEYWORDS: + return self._advance().value + found = "end of input" if tok.type == "EOF" else repr(tok.value) + raise ParseError(f"expected {what}, found {found}", position=tok.pos, query=self.text) + # query := MATCH pattern (, pattern)* [WHERE expr] RETURN [DISTINCT] item (, item)* # [ORDER BY key (, key)*] [SKIP int] [LIMIT int] def parse(self) -> QueryAst: @@ -213,7 +223,7 @@ def _node_pattern(self) -> NodePattern: var = self._accept("IDENT") label = None if self._accept(":"): - label = self._expect("IDENT", "a node label").value + label = self._name("a node label") props: list[tuple[str, Lit]] = [] if self._accept("{"): props.append(self._prop_eq()) @@ -242,7 +252,7 @@ def _rel_pattern(self) -> RelPattern: v = self._accept("IDENT") var = v.value if v else None if self._accept(":"): - kind = self._expect("IDENT", "a relationship type").value + kind = self._name("a relationship type") if self._accept("*"): min_hops, max_hops = self._varlen() self._expect("]", "']'") diff --git a/src/agentforge_graph/store/surreal_store.py b/src/agentforge_graph/store/surreal_store.py index 562bc15..d3bac08 100644 --- a/src/agentforge_graph/store/surreal_store.py +++ b/src/agentforge_graph/store/surreal_store.py @@ -61,6 +61,9 @@ node_from_row, node_params, ) +from .query.ast import QueryAst +from .query.capability import AGG_COLLECT, CORE_TIER, QuerySettings, ResultTable +from .query.interpret import InterpretingQueryEngine _VEC_FILTERABLE = ("ref", "kind", "path") @@ -146,12 +149,27 @@ def _rows(result: Any) -> list[dict[str, Any]]: class SurrealGraphStore(GraphStore): - """Graph store backed by SurrealDB (document tables ``ckg_node``/``ckg_edge``).""" + """Graph store backed by SurrealDB (document tables ``ckg_node``/``ckg_edge``). + + Query-capable (feat-015) via the portable AST *interpreter*: SurrealDB models + edges as a document table with no native graph traversal to compile to, so it + evaluates the query over the ``GraphStore`` ABC (``query``/``adjacent``/ + ``get``) instead of a SurrealQL translation. Inherently read-only (no write + path through the ABC); passes the same conformance suite as the compiled + backends.""" + + # --- QueryCapable (feat-015) --- + query_dialect = "interpreted" + capabilities = CORE_TIER | {AGG_COLLECT} + read_only_execution = True def __init__(self, db: Any) -> None: self._db = db self._closed = False + async def run_query(self, ast: QueryAst, settings: QuerySettings) -> ResultTable: + return await InterpretingQueryEngine(self).run(ast, settings) + @classmethod async def open( cls, path: str | Path, config: dict[str, Any] | None = None diff --git a/tests/store/query/test_interpret.py b/tests/store/query/test_interpret.py new file mode 100644 index 0000000..ee70814 --- /dev/null +++ b/tests/store/query/test_interpret.py @@ -0,0 +1,133 @@ +"""Targeted unit tests for the AST interpreter (feat-015 chunk 4). + +The conformance suite covers the core parity path; these cover the branches it +doesn't: avg/min/max/collect aggregates, DISTINCT, ORDER BY a property, attrs +access, node_property fields, and the timeout backstop. Driven over an embedded +Kuzu store as the data source. +""" + +from __future__ import annotations + +from collections.abc import AsyncIterator +from pathlib import Path + +import pytest + +from agentforge_graph.core import ( + Descriptor, + Edge, + EdgeKind, + FileSubgraph, + Node, + NodeKind, + Provenance, + SymbolID, +) +from agentforge_graph.store import KuzuGraphStore +from agentforge_graph.store.query import parse_query, validate_query +from agentforge_graph.store.query.capability import ATTRS_ACCESS, CORE_TIER, QuerySettings +from agentforge_graph.store.query.interpret import InterpretingQueryEngine, node_property + +_PROV = Provenance.parsed("interp-test", "c0") +_CAPS = CORE_TIER | {ATTRS_ACCESS, "agg.collect"} + + +def _n(desc: str, kind: NodeKind, name: str, span: tuple[int, int], **attrs: object) -> Node: + return Node( + id=SymbolID.for_symbol("py", "r", "a.py", desc), + kind=kind, + name=name, + span=span, + attrs=attrs, + provenance=_PROV, + ) + + +@pytest.fixture +async def store(tmp_path: Path) -> AsyncIterator[KuzuGraphStore]: + s = await KuzuGraphStore.open(tmp_path / "g.kuzu") + cls = _n(Descriptor.type("Svc"), NodeKind.CLASS, "Svc", (1, 40), layer="core") + other = _n(Descriptor.type("Web"), NodeKind.CLASS, "Web", (2, 5), layer="ui") + m1 = _n(Descriptor.type("Svc") + Descriptor.method("a"), NodeKind.METHOD, "a", (3, 8)) + m2 = _n(Descriptor.type("Svc") + Descriptor.method("b"), NodeKind.METHOD, "b", (10, 20)) + nodes = [cls, other, m1, m2] + edges = [ + Edge(src=cls.id, dst=m1.id, kind=EdgeKind.CONTAINS, provenance=_PROV), + Edge(src=cls.id, dst=m2.id, kind=EdgeKind.CONTAINS, provenance=_PROV), + ] + await s.upsert(FileSubgraph(path="a.py", content_hash="h", nodes=nodes, edges=edges)) + try: + yield s + finally: + await s.close() + + +async def _run(store: KuzuGraphStore, text: str, settings: QuerySettings | None = None): + ast = validate_query(parse_query(text), _CAPS) + return await InterpretingQueryEngine(store).run(ast, settings or QuerySettings()) + + +async def test_avg_min_max_aggregates(store: KuzuGraphStore) -> None: + rt = await _run( + store, + "MATCH (m:Method) RETURN min(m.start_line) AS lo, max(m.end_line) AS hi, " + "avg(m.start_line) AS mid", + ) + assert rt.columns == ("lo", "hi", "mid") + assert rt.rows == ((3, 20, 6.5),) + + +async def test_collect_is_sorted(store: KuzuGraphStore) -> None: + rt = await _run( + store, + "MATCH (c:Class)-[:CONTAINS]->(m:Method) RETURN c.name, collect(m.name) AS ms", + ) + assert rt.rows == (("Svc", ["a", "b"]),) + + +async def test_distinct(store: KuzuGraphStore) -> None: + rt = await _run(store, "MATCH (c:Class) RETURN DISTINCT c.kind") + assert rt.rows == (("Class",),) + + +async def test_order_by_property_desc(store: KuzuGraphStore) -> None: + rt = await _run(store, "MATCH (m:Method) RETURN m.name ORDER BY m.start_line DESC") + assert [r[0] for r in rt.rows] == ["b", "a"] # start_line 10 then 3 + + +async def test_attrs_access_when_permitted(store: KuzuGraphStore) -> None: + rt = await _run(store, 'MATCH (c:Class) WHERE c.attrs.layer = "core" RETURN c.name') + assert {r[0] for r in rt.rows} == {"Svc"} + + +async def test_timeout_backstop_reports_partial(store: KuzuGraphStore) -> None: + # a clock that jumps past the deadline after the first binding is appended + import contextlib + + ticks = iter([0.0, 100.0, 100.0, 100.0, 100.0]) + last = [0.0] + + def now() -> float: + with contextlib.suppress(StopIteration): + last[0] = next(ticks) + return last[0] + + ast = validate_query(parse_query("MATCH (c:Class) RETURN c.name"), _CAPS) + rt = await InterpretingQueryEngine(store).run(ast, QuerySettings(timeout_ms=1000), now=now) + assert rt.truncated and rt.stopped_reason == "timeout" + + +def test_node_property_covers_all_fields() -> None: + n = _n(Descriptor.term("f"), NodeKind.FUNCTION, "f", (2, 9), role="svc") + assert node_property(n, ("name",)) == "f" + assert node_property(n, ("kind",)) == "Function" + assert node_property(n, ("path",)) == "a.py" + assert node_property(n, ("start_line",)) == 2 + assert node_property(n, ("end_line",)) == 9 + assert node_property(n, ("source",)) == "parsed" + assert node_property(n, ("extractor",)) == "interp-test" + assert node_property(n, ("commit",)) == "c0" + assert node_property(n, ("confidence",)) == 1.0 + assert node_property(n, ("attrs", "role")) == "svc" + assert node_property(n, ("attrs", "missing")) is None + assert node_property(n, ("bogus",)) is None diff --git a/tests/store/query/test_interpret_conformance.py b/tests/store/query/test_interpret_conformance.py new file mode 100644 index 0000000..d366cb6 --- /dev/null +++ b/tests/store/query/test_interpret_conformance.py @@ -0,0 +1,55 @@ +"""Run feat-015's QueryConformance through the AST interpreter (chunk 4). + +The interpreter is the query path for backends with no native query language +(SurrealDB). Here it runs over an *embedded Kuzu* store as the data source, so +the interpreter logic is proven in CI with no server — the same query set must +return the same canonical rows as the compiled Kuzu path. The live-SurrealDB +subclass (env-gated) then confirms it against the real backend. +""" + +from __future__ import annotations + +from collections.abc import AsyncIterator +from pathlib import Path + +import pytest + +from agentforge_graph.core import GraphStore +from agentforge_graph.store import KuzuGraphStore +from agentforge_graph.store.query.ast import QueryAst +from agentforge_graph.store.query.capability import ( + AGG_COLLECT, + CORE_TIER, + QuerySettings, + ResultTable, +) +from agentforge_graph.store.query.conformance import QueryConformance +from agentforge_graph.store.query.interpret import InterpretingQueryEngine + + +class _InterpretedStore: + """A QueryCapable store that stores via a real backend but *interprets* + queries over the ABC — exercises the interpreter without a live server.""" + + query_dialect = "interpreted" + capabilities = CORE_TIER | {AGG_COLLECT} + read_only_execution = True + + def __init__(self, backing: GraphStore) -> None: + self._backing = backing + + async def upsert(self, subgraph: object) -> None: + await self._backing.upsert(subgraph) # type: ignore[arg-type] + + async def run_query(self, ast: QueryAst, settings: QuerySettings) -> ResultTable: + return await InterpretingQueryEngine(self._backing).run(ast, settings) + + +class TestInterpretedQuery(QueryConformance): + @pytest.fixture + async def store(self, tmp_path: Path) -> AsyncIterator[_InterpretedStore]: + backing = await KuzuGraphStore.open(tmp_path / "graph.kuzu") + try: + yield _InterpretedStore(backing) + finally: + await backing.close() diff --git a/tests/store/test_surreal_query_conformance.py b/tests/store/test_surreal_query_conformance.py new file mode 100644 index 0000000..39f7664 --- /dev/null +++ b/tests/store/test_surreal_query_conformance.py @@ -0,0 +1,44 @@ +"""Run feat-015's QueryConformance against the SurrealDB adapter (chunk 4). + +SurrealDB has no native graph traversal, so it executes queries via the portable +AST interpreter over the GraphStore ABC. This env-gated live test confirms the +interpreter returns the same canonical rows against the real SurrealDB backend +that it does over Kuzu — result parity across all three backends. + + docker run -d --name ckg-surreal -p 8000:8000 \\ + surrealdb/surrealdb:latest start --user root --pass root + CKG_SURREALDB_URL=ws://localhost:8000/rpc \\ + uv run pytest tests/store/test_surreal_query_conformance.py +""" + +from __future__ import annotations + +import os +from collections.abc import AsyncIterator + +import pytest + +from agentforge_graph.store.query.conformance import QueryConformance + +_URL = os.environ.get("CKG_SURREALDB_URL") +pytestmark = pytest.mark.skipif( + not _URL, reason="set CKG_SURREALDB_URL to run SurrealDB query conformance" +) + + +class TestSurrealQuery(QueryConformance): + @pytest.fixture + async def store(self) -> AsyncIterator: + from agentforge_graph.store.surreal_store import SurrealGraphStore + + config = { + "url": _URL or "", + "username": os.environ.get("CKG_SURREALDB_USER", "root"), + "password": os.environ.get("CKG_SURREALDB_PASS", "root"), + } + store = await SurrealGraphStore.open("unused", config) + await store._db.query("DELETE ckg_node; DELETE ckg_edge") # fresh graph per test + try: + yield store + finally: + await store.close() From 3202c1b7741258614ef01826dafac9a65b4c3b93 Mon Sep 17 00:00:00 2001 From: Khemchand Joshi Date: Wed, 8 Jul 2026 13:12:19 +0530 Subject: [PATCH 08/11] feat-015: query: config block + docs (chunk 7) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Final chunk — make the query surface configurable and documented. - QueryConfig(_Block, KEY="query"): enabled / max_rows / timeout_ms / max_expansions / allow_in_mcp, with to_settings(limit). Auto-discovered by block_keys(); read framework-free (ADR-0001). - Wired through the surfaces: the CLI (`_query_graph`) and engine (`query_graph`) build QuerySettings from the block and refuse when enabled=false; the serve capability gate offers ckg_query only when the backend is query-capable AND enabled AND allow_in_mcp. - Docs: guide 13 (ad-hoc structural queries) + guides index; README tool list now notes ckg_query; CHANGELOG [Unreleased] entry; feat-015 Implementation- status section recording the (deliberate, documented) deviations — chiefly the portable AST interpreter for SurrealDB in place of a native SurrealQL compiler. feat-015 is now complete across all 7 chunks. Gate: 1091 passed, 69 skipped, 94.33% coverage; ruff + mypy --strict clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 25 ++++ README.md | 8 +- .../feat-015-read-only-graph-query.md | 44 +++++++ docs/guides/10-using-over-mcp.md | 4 +- docs/guides/13-graph-query.md | 116 ++++++++++++++++++ docs/guides/README.md | 1 + src/agentforge_graph/cli.py | 16 ++- src/agentforge_graph/config.py | 26 ++++ src/agentforge_graph/serve/engine.py | 19 +-- src/agentforge_graph/serve/server.py | 17 +-- tests/test_query_config.py | 82 +++++++++++++ 11 files changed, 325 insertions(+), 33 deletions(-) create mode 100644 docs/guides/13-graph-query.md create mode 100644 tests/test_query_config.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e85aa6..ff23004 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,31 @@ on a schema mismatch is **rebuild** (ADR-0006). ## [Unreleased] +### Added + +- **feat-015 — read-only graph query surface (`ckg query --graph` / `ckg_query`).** + An escape hatch for precise **structural** questions no typed verb covers + (e.g. "classes tagged `Repository` with no inbound `CALLS`", "interfaces + implemented by >5 classes"). A bounded, read-only **Cypher subset**: + - **CLI:** `ckg query --graph ''` with `--format table|json`, + `--limit`, and `ckg query --schema` (the queryable vocabulary). + - **MCP:** a `ckg_query` tool, registered only when the backend is + query-capable and `query.allow_in_mcp` is on; results carry the staleness + + `query_lang_version` envelope. + - **Safety by design:** caller text is never executed — it is parsed into a + validated AST (the single trust boundary), then **compiled** to native + Cypher (Kuzu, Neo4j) or **interpreted** over the storage API (SurrealDB and + any backend without a native query language). All three backends return + identical rows (a shared conformance suite proves it). Writes/DDL, procedure + calls, unbounded paths, and un-joined Cartesian products are rejected with a + clear reason. + - **Bounded, no silent caps:** `query.max_rows` / `timeout_ms` / + `max_expansions` are enforced on every backend and reported via `truncated` + + `stopped_reason`. `ckg status` reports the query-language version and whether + the active backend is query-capable. + - New `query:` config block; guide 13. `NodeKind`/`EdgeKind` vocabulary + unchanged. + ## [0.6.3] — 2026-07-07 ### Added diff --git a/README.md b/README.md index 1ed84f3..4383484 100644 --- a/README.md +++ b/README.md @@ -185,9 +185,11 @@ provenance. ### Serve it to an agent -Read-only over MCP — **10 tools**: `ckg_repo_map`, `ckg_search`, `ckg_symbol`, -`ckg_impact`, `ckg_neighbors`, `ckg_status`, `ckg_routes`, `ckg_decisions`, -`ckg_explain`, `ckg_history`: +Read-only over MCP — **10 typed tools**: `ckg_repo_map`, `ckg_search`, +`ckg_symbol`, `ckg_impact`, `ckg_neighbors`, `ckg_status`, `ckg_routes`, +`ckg_decisions`, `ckg_explain`, `ckg_history` — plus `ckg_query` (a read-only +[structural query](docs/guides/13-graph-query.md), added when the backend is +query-capable): ```bash ckg setup # wire your agent for you (writes .mcp.json) diff --git a/docs/features/feat-015-read-only-graph-query.md b/docs/features/feat-015-read-only-graph-query.md index e8d9061..8d67ee9 100644 --- a/docs/features/feat-015-read-only-graph-query.md +++ b/docs/features/feat-015-read-only-graph-query.md @@ -266,6 +266,50 @@ Accepted cost: we own a **small, read-only** Cypher-subset parser (the validatio trust boundary), bounded deliberately — not full Cypher — with the query-conformance suite keeping backends result-identical. +## Implementation status + +**Shipped (0.6.4).** All three backends query-capable and result-identical, +proven by a shared conformance suite (Kuzu compiled + Neo4j compiled verified +against live servers; SurrealDB via the interpreter, verified live). Built in 7 +chunks on `feat/015-ckg-query`: (1) AST + parser + validator + schema + +capability; (2) Cypher compiler + Kuzu execution + bounded executor + facade/ +CodeGraph wiring + conformance; (3) Neo4j execution (read-only session); (4) +portable AST interpreter + SurrealDB; (5) CLI `--graph/--schema/--format/--limit`; +(6) `ckg_query` tool + capability-gated registration; (7) `query:` config block + +docs (guide 13). + +**Deviations from design-015 (all deliberate, documented):** + +- **SurrealDB uses a portable AST *interpreter* over the `GraphStore` ABC, not a + native SurrealQL compiler.** Discovered during implementation that SurrealDB + models edges as a document table (no native graph traversal), making a faithful + SurrealQL translator of the core tier infeasible without a fragile workaround. + The interpreter (`store/query/interpret.py`) is a superior universal + alternative — *compile where the backend has a native query language + (Kuzu/Neo4j), interpret elsewhere* — so **any** `GraphStore` is query-capable + for free, and it passes the same conformance suite. User-approved. +- **Compiler expression visitor uses `match` statements, not + `functools.singledispatchmethod`** — same additive property (one arm per AST + node), cleaner under `mypy --strict`. +- **`QueryConformance` lives in `store/query/`, not `core/conformance.py`** — core + must not import `store` (where the query types live). +- **`attrs.*` access is an optional `attrs.access` capability that no v1 backend + advertises** — Kuzu/Neo4j store `attrs` as a JSON string, not destructurable in + native Cypher without a workaround that breaks under aggregation. The curated + columns cover the real use cases; the capability seam reports `attrs.*` as + unsupported honestly. (The interpreter *can* read `attrs`; a future backend or a + follow-up may advertise the capability.) +- **`ckg_query` v1 exposes `{query, limit}` only — no `params` argument** (design + open-Q1 leaned yes). The grammar inlines and parameterizes literals internally + and has no `$placeholder` syntax, so a caller-facing params map has no binding + site in v1. +- **CLI formatter is a flat `cli_format.py` module** (the repo has no `cli/` + package), introduced as a reusable seam wired only to `query` this PR. + +**Parser note:** `CONTAINS` is both the string operator and an `EdgeKind`; labels +and relationship types accept reserved words in name position (a shared-parser fix +locked across all three backends). + ## 11. References - [FA-003](../feature-analysis/FA-003-read-only-graph-query-language.md) — source analysis. diff --git a/docs/guides/10-using-over-mcp.md b/docs/guides/10-using-over-mcp.md index 815eeee..27a97fa 100644 --- a/docs/guides/10-using-over-mcp.md +++ b/docs/guides/10-using-over-mcp.md @@ -1,6 +1,8 @@ # Using the CKG from an agent (MCP) or in-process -> **TL;DR:** Serve the graph over **10 read-only tools** — an MCP server +> **TL;DR:** Serve the graph over **10 read-only tools** (plus `ckg_query`, the +> [structural-query](13-graph-query.md) escape hatch, when the backend supports +> it) — an MCP server > (`ckg serve-mcp`, stdio or HTTP) for external clients like Claude Code, or the > in-process AgentForge toolset for framework agents. Every response carries a > staleness envelope. diff --git a/docs/guides/13-graph-query.md b/docs/guides/13-graph-query.md new file mode 100644 index 0000000..a32119e --- /dev/null +++ b/docs/guides/13-graph-query.md @@ -0,0 +1,116 @@ +# Ad-hoc structural queries (`ckg query --graph` / `ckg_query`) + +> **TL;DR:** For precise **structural** questions no typed verb answers — "public +> methods on classes tagged `Repository` with no inbound `CALLS`", "interfaces +> implemented by >5 classes" — run a **read-only Cypher subset** query: +> `ckg query --graph 'MATCH … WHERE … RETURN …'` or the `ckg_query` MCP tool. It is +> the escape hatch; for semantic "find code about X" use `ckg_search`. + +The typed verbs (`ckg search`, `ckg impact`, `ckg routes`, …) each answer one +question well, but they are a **closed set**. When a question doesn't map to a +verb, the query surface lets you ask it directly against the graph — without us +shipping a new verb — while keeping the same read-only safety and staleness +guarantees (feat-015). + +Retrieval vs query, at a glance: **retrieval** (`ckg_search`) discovers code by +meaning (vectors + ranking); **query** interrogates the graph with exact +predicates. Use retrieval to find, query to filter/aggregate. + +--- + +## 1. The surface + +```bash +# structural query (read-only) +ckg query --graph 'MATCH (f:Function) WHERE NOT (f)<-[:CALLS]-() RETURN f.name, f.path LIMIT 50' + +# output formats +ckg query --graph '' --format table # default, aligned columns +ckg query --graph '' --format json # {columns, rows, truncated, stopped_reason} + +# see the queryable vocabulary (node/edge kinds + properties) +ckg query --schema +``` + +The natural-language path — `ckg query "how does auth work"` (hybrid retrieval) — +is unchanged; `--graph` selects the structural surface. + +Over MCP, the `ckg_query` tool takes `{ query, limit? }` and returns the columnar +result plus the usual staleness envelope (`indexed_commit`, `dirty`, `truncated`, +`query_lang_version`). It is registered only when the backend is query-capable and +`query.allow_in_mcp` is on. + +--- + +## 2. The language (a bounded, read-only Cypher subset) + +Supported (see `ckg query --schema` for the live vocabulary): + +- **`MATCH`** node/relationship patterns over the locked node/edge kinds: + `(c:Class)-[:IMPLEMENTS]->(i:Interface)`, directions `->`/`<-`/`-`, bounded + variable-length `[:CALLS*1..3]`. +- **`WHERE`** — comparisons (`= <> < <= > >=`), `AND`/`OR`/`NOT`, `IN […]`, string + predicates (`STARTS WITH` / `ENDS WITH` / `CONTAINS`), and pattern existence + (`(a)-[:KIND]->(b)`). +- **`RETURN`** — property projections, aliases (`AS`), `DISTINCT`, and aggregates + `count` / `min` / `max` / `avg` / `collect`. +- **`ORDER BY … [ASC|DESC]`, `SKIP`, `LIMIT`.** + +Queryable node properties (mapped identically on every backend): `name`, `kind`, +`path`, `start_line`, `end_line`, `source`, `extractor`, `commit`, `confidence`. + +**Rejected** (with a clear reason, never a stack trace): any write/DDL +(`CREATE/MERGE/SET/DELETE/…`), procedure `CALL`, unbounded paths (`[*]`), and +disconnected patterns that would form a Cartesian product. + +### Examples + +```cypher +-- classes tagged "Repository" +MATCH (c:Class)-[:TAGGED]->(t:PatternTag) WHERE t.name = "Repository" RETURN c.name + +-- interfaces by how many classes implement them +MATCH (c:Class)-[:IMPLEMENTS]->(i:Interface) +RETURN i.name, count(c) AS impls ORDER BY impls DESC + +-- functions nothing calls (dead-ish code) +MATCH (f:Function) WHERE NOT (f)<-[:CALLS]-() RETURN f.name, f.path + +-- what a function reaches within 3 call hops +MATCH (f:Function {name: "handle_login"})-[:CALLS*1..3]->(g:Function) RETURN DISTINCT g.name +``` + +--- + +## 3. Safety & bounds + +The query is never executed as caller text: it is parsed into a validated +internal AST (the single trust boundary), then either **compiled** to the +backend's native Cypher (Kuzu, Neo4j) or **interpreted** over the storage API +(SurrealDB and any backend without a native query language). Every backend +returns identical rows — proven by a shared conformance suite. + +Every run is bounded and the caps are reported (never silently applied): + +```yaml +query: + enabled: true # disable the structural surface wholesale + max_rows: 1000 # row cap; a clipped result sets truncated=true + timeout_ms: 5000 # per-query wall-clock budget + max_expansions: 50000 # intermediate-row / traversal ceiling + allow_in_mcp: true # expose ckg_query as an MCP tool (vs CLI-only) +``` + +When a bound trips, the result carries `truncated: true` and a `stopped_reason` +(`row_cap` / `expansion_cap` / `timeout`), so a partial answer is never mistaken +for a complete one. `ckg status` reports the query-language version and whether +the active backend is query-capable. + +--- + +## 4. When to use a query vs a verb + +Reach for `ckg_query` when the question is **structural and exact** and no verb +fits. If you find yourself writing the *same* query repeatedly, that is a signal +it should become a first-class verb — tell us; the common ad-hoc queries are the +best candidates to promote. diff --git a/docs/guides/README.md b/docs/guides/README.md index 3c64d59..2f51e4c 100644 --- a/docs/guides/README.md +++ b/docs/guides/README.md @@ -32,6 +32,7 @@ setup. They build on each other in order. | 10 | [Using over MCP](10-using-over-mcp.md) | Serve the CKG to an agent over 10 read-only tools (stdio/HTTP) or in-process. | | 11 | [Agent auto-configuration](11-agent-auto-configuration.md) | `ckg setup` writes your agent's MCP config (repo `.mcp.json` / `--scope user`) + `--hooks` nudge; reversible. | | 12 | [Watch mode & CI indexing](12-watch-and-ci-indexing.md) | Keep the graph fresh: `ckg watch` (local, conditional triggers) + `ckg ci init` (central, CI-as-sole-writer). | +| 13 | [Ad-hoc structural queries](13-graph-query.md) | Escape hatch for structural questions no verb covers: a read-only Cypher subset via `ckg query --graph` / the `ckg_query` tool. | > New here? Start with **[Getting started → a single repo](getting-started/1-single-repo.md)**. > Building an agent on top? Jump to **[10 — Using over MCP](10-using-over-mcp.md)**. diff --git a/src/agentforge_graph/cli.py b/src/agentforge_graph/cli.py index 5fa0feb..0b53771 100644 --- a/src/agentforge_graph/cli.py +++ b/src/agentforge_graph/cli.py @@ -796,18 +796,16 @@ async def _query_schema(args: argparse.Namespace) -> int: async def _query_graph(args: argparse.Namespace) -> int: """feat-015: run a read-only structural query and render the result.""" from agentforge_graph.cli_format import render_json, render_table - from agentforge_graph.store.query import QueryDisabled, QueryError, QuerySettings + from agentforge_graph.config import QueryConfig, resolve_config + from agentforge_graph.store.query import QueryDisabled, QueryError - settings = QuerySettings() - if args.limit is not None: - settings = QuerySettings( - max_rows=min(settings.max_rows, args.limit), - timeout_ms=settings.timeout_ms, - max_expansions=settings.max_expansions, - ) + qcfg = QueryConfig.load(resolve_config(args.config, args.path)) + if not qcfg.enabled: + print("the query surface is disabled (query.enabled=false)", file=sys.stderr) + return 2 cg = await CodeGraph.open(repo_path=args.path, config=args.config) try: - rt = await cg.query_graph(args.graph, settings) + rt = await cg.query_graph(args.graph, qcfg.to_settings(args.limit)) except QueryError as exc: print(f"query error: {exc}", file=sys.stderr) return 2 diff --git a/src/agentforge_graph/config.py b/src/agentforge_graph/config.py index 9b9ca9f..395cf27 100644 --- a/src/agentforge_graph/config.py +++ b/src/agentforge_graph/config.py @@ -308,6 +308,32 @@ class ServeConfig(_Block): refresh_on_call: bool = False +class QueryConfig(_Block): + """The ``query:`` block of ckg.yaml (feat-015 — read-only structural query). + + Bounds the guard-railed query surface: ``enabled`` can disable it wholesale; + ``max_rows``/``timeout_ms``/``max_expansions`` are the resource caps every + backend enforces; ``allow_in_mcp`` gates whether ``ckg_query`` is exposed as + an MCP tool (vs CLI-only).""" + + KEY: ClassVar[str] = "query" + enabled: bool = True + max_rows: int = 1000 + timeout_ms: int = 5000 + max_expansions: int = 50_000 + allow_in_mcp: bool = True + + def to_settings(self, limit: int | None = None) -> Any: + """Build the engine-layer ``QuerySettings`` from this block. ``limit`` (a + caller's ``--limit`` / tool ``limit``) further lowers the row cap.""" + from agentforge_graph.store.query import QuerySettings + + max_rows = self.max_rows if limit is None else min(self.max_rows, max(0, limit)) + return QuerySettings( + max_rows=max_rows, timeout_ms=self.timeout_ms, max_expansions=self.max_expansions + ) + + class SetupConfig(_Block): """The ``setup:`` block (feat-013 — agent auto-configuration). diff --git a/src/agentforge_graph/serve/engine.py b/src/agentforge_graph/serve/engine.py index 06f4f36..09a9ba4 100644 --- a/src/agentforge_graph/serve/engine.py +++ b/src/agentforge_graph/serve/engine.py @@ -221,27 +221,20 @@ async def query_graph(self, query: str, limit: int | None = None) -> dict[str, A envelope. A ``QueryError`` (bad syntax / vocabulary / capability / a non-query backend) is returned as a structured ``error`` — never raised into the tool layer.""" - from agentforge_graph.store.query import ( - QUERY_LANG_VERSION, - QueryError, - QuerySettings, - ) + from agentforge_graph.config import QueryConfig + from agentforge_graph.store.query import QUERY_LANG_VERSION, QueryError + qcfg = QueryConfig.load(self.config) cg = await self.code_graph() - settings = QuerySettings() - if limit is not None: - settings = QuerySettings( - max_rows=min(settings.max_rows, max(0, limit)), - timeout_ms=settings.timeout_ms, - max_expansions=settings.max_expansions, - ) envelope = { **(await self.staleness()), "tool_api_version": TOOL_API_VERSION, "query_lang_version": QUERY_LANG_VERSION, } + if not qcfg.enabled: + return {"error": "the query surface is disabled (query.enabled=false)", **envelope} try: - rt = await cg.query_graph(query, settings) + rt = await cg.query_graph(query, qcfg.to_settings(limit)) except QueryError as exc: return {"error": str(exc), **envelope} return { diff --git a/src/agentforge_graph/serve/server.py b/src/agentforge_graph/serve/server.py index 6872bb3..e398139 100644 --- a/src/agentforge_graph/serve/server.py +++ b/src/agentforge_graph/serve/server.py @@ -45,18 +45,21 @@ def _tools_for(engine: EngineProvider, capabilities: frozenset[str]) -> list[Too def _store_capabilities(config: ConfigSource, repo_path: str | Path) -> frozenset[str]: - """Serve-level capability markers for a store config — currently just - ``query`` when the resolved graph driver is query-capable (feat-015). - Resolved from the driver *class*, so no index is opened.""" - from agentforge_graph.config import StoreConfig, resolve_config + """Serve-level capability markers for a store config — ``query`` when the + resolved graph driver is query-capable *and* the ``query:`` block permits it + over MCP (``enabled`` + ``allow_in_mcp``, feat-015). Resolved from the driver + *class* + config, so no index is opened.""" + from agentforge_graph.config import QueryConfig, StoreConfig, resolve_config from agentforge_graph.store.registry import graph_driver try: - cfg = StoreConfig.load(resolve_config(config, repo_path)) - driver = graph_driver(cfg.graph.driver) + resolved = resolve_config(config, repo_path) + driver = graph_driver(StoreConfig.load(resolved).graph.driver) + qcfg = QueryConfig.load(resolved) except Exception: return frozenset() - return frozenset({"query"}) if hasattr(driver, "query_dialect") else frozenset() + capable = hasattr(driver, "query_dialect") and qcfg.enabled and qcfg.allow_in_mcp + return frozenset({"query"}) if capable else frozenset() def code_graph_tools(repo_path: str | Path = ".", config: ConfigSource = None) -> list[Tool]: diff --git a/tests/test_query_config.py b/tests/test_query_config.py new file mode 100644 index 0000000..11f041e --- /dev/null +++ b/tests/test_query_config.py @@ -0,0 +1,82 @@ +"""feat-015 chunk 7: the query: config block and its wiring. + +Covers QueryConfig parsing/defaults/to_settings and that disabling the surface +(query.enabled / query.allow_in_mcp) is honoured at the CLI, the engine, and the +MCP tool-registration gate. +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from agentforge_graph.cli import main +from agentforge_graph.config import QueryConfig +from agentforge_graph.ingest import CodeGraph + +_SRC = "class Repo:\n def save(self): ...\n" + + +def test_defaults() -> None: + q = QueryConfig.load(None) + assert q.enabled is True and q.allow_in_mcp is True + assert (q.max_rows, q.timeout_ms, q.max_expansions) == (1000, 5000, 50_000) + + +def test_parses_block(tmp_path: Path) -> None: + y = tmp_path / "ckg.yaml" + y.write_text("query:\n enabled: false\n max_rows: 25\n allow_in_mcp: false\n") + q = QueryConfig.load(y) + assert q.enabled is False and q.allow_in_mcp is False and q.max_rows == 25 + + +def test_to_settings_applies_limit() -> None: + q = QueryConfig.load(None) + assert q.to_settings().max_rows == 1000 + assert q.to_settings(10).max_rows == 10 # caller limit lowers the cap + assert q.to_settings(9999).max_rows == 1000 # but never raises it + + +def test_block_key_is_discovered() -> None: + from agentforge_graph.config import block_keys + + assert "query" in block_keys() + + +@pytest.fixture +async def indexed(tmp_path: Path) -> Path: + (tmp_path / "app.py").write_text(_SRC) + cg = await CodeGraph.index(repo_path=tmp_path) + await cg.close() + return tmp_path + + +def test_cli_refuses_when_disabled(indexed: Path, capsys: pytest.CaptureFixture[str]) -> None: + cfg = indexed / "ckg.yaml" + cfg.write_text("query:\n enabled: false\n") + rc = main(["query", "--path", str(indexed), "--config", str(cfg), + "--graph", "MATCH (c:Class) RETURN c.name"]) + assert rc == 2 + assert "disabled" in capsys.readouterr().err + + +async def test_engine_reports_disabled(indexed: Path) -> None: + from agentforge_graph.config import resolve_config + from agentforge_graph.serve.engine import _Engine + + cfg = indexed / "ckg.yaml" + cfg.write_text("query:\n enabled: false\n") + eng = _Engine(indexed, resolve_config(str(cfg), str(indexed))) + out = await eng.query_graph("MATCH (c:Class) RETURN c.name") + assert "disabled" in out["error"] + + +def test_tool_gated_out_when_allow_in_mcp_false(tmp_path: Path) -> None: + from agentforge_graph.serve import code_graph_tools + + (tmp_path / "app.py").write_text(_SRC) + (tmp_path / "ckg.yaml").write_text("query:\n allow_in_mcp: false\n") + tools = {t.name for t in code_graph_tools(str(tmp_path), str(tmp_path / "ckg.yaml"))} + assert "ckg_query" not in tools # capable backend, but MCP exposure is off + assert "ckg_search" in tools # other tools unaffected From 11515de6a6f802ea23e11433b5d02c87eb1a92fb Mon Sep 17 00:00:00 2001 From: Khemchand Joshi Date: Wed, 8 Jul 2026 14:19:05 +0530 Subject: [PATCH 09/11] fix(resolver): resolve re-exported package symbols as base classes [BUG-CARRIED feat-015] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Surfaced during feat-015 end-to-end validation: `class KuzuGraphStore(GraphStore)` produced no INHERITS edge. `GraphStore` is imported `from agentforge_graph.core import GraphStore` but *defined* in `core.contracts` and only re-exported by `core/__init__.py` — so it was absent from the package module's def-`exports`, the base stayed unresolved, and no edge was emitted. Fix: compute a per-package `reexports` namespace from each `pkg/__init__.py`'s own `from .sub import Name` imports (resolved against `exports` via a fixpoint, so a re-export chained through several `__init__` files still resolves), and consult it when binding `from pkg import Name`. So an absolute import of a re-exported symbol links to its real definition. On this project's own core+store package: INHERITS 11→17 (all GraphStore / VectorStore subclasses now link to their ABC), CALLS 373→395 (functions imported via a package re-export and called by name now resolve too). Improves INHERITS/CALLS recall for every consumer, not only the query surface. 2 new tests (absolute + relative re-export). Gate: 1093 passed, 94.34% coverage; ruff + mypy --strict clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/agentforge_graph/ingest/resolver.py | 40 ++++++++++++++++++++++++- tests/ingest/test_inherits.py | 31 +++++++++++++++++++ 2 files changed, 70 insertions(+), 1 deletion(-) diff --git a/src/agentforge_graph/ingest/resolver.py b/src/agentforge_graph/ingest/resolver.py index a74f11c..9bd4e4d 100644 --- a/src/agentforge_graph/ingest/resolver.py +++ b/src/agentforge_graph/ingest/resolver.py @@ -190,6 +190,42 @@ def _external(slug: str, repo: str, module: str) -> str: def _is_target(path: str) -> bool: return changed_files is None or path in changed_files + # --- package re-exports (Python): a `pkg/__init__.py` doing + # `from .sub import Name` makes `Name` importable as `pkg.Name` even though + # it is *defined* in `pkg.sub`. Resolve these so an absolute import of a + # re-exported symbol (e.g. a base class `class C(GraphStore)` where + # `GraphStore` is re-exported by the package) links to the real definition. + # Computed from `exports` (defs) via a fixpoint, so a re-export chained + # through several `__init__` files still resolves (BUG-CARRIED feat-015). --- + reexports: dict[str, dict[str, str]] = {} + init_reexports: list[tuple[str, str, list[str]]] = [] + for f in files: + ps = SymbolID.parse(f.id) + if posixpath.basename(ps.path) not in _INIT_FILES: + continue + ipack = self.registry.for_slug(ps.lang) + if ipack is None: + continue + pkg_module = file_module.get(f.id, "") + for imp in f.attrs.get("imports", []): + module = imp.get("module", "") + names = imp.get("names", []) + if module and names: + sub_key = ipack.resolve_import(ps.path, module, pkg_module) + init_reexports.append((pkg_module, sub_key, names)) + for _ in range(len(init_reexports) + 1): + changed = False + for pkg_module, sub_key, names in init_reexports: + src_ns = exports.get(sub_key, {}) + chained = reexports.get(sub_key, {}) + for nm in names: + tgt = src_ns.get(nm) or chained.get(nm) + if tgt and reexports.setdefault(pkg_module, {}).get(nm) != tgt: + reexports[pkg_module][nm] = tgt + changed = True + if not changed: + break + # --- imports -> IMPORTS edges + per-file name bindings --- for f in files: ps = SymbolID.parse(f.id) @@ -276,7 +312,9 @@ def _is_target(path: str) -> bool: stats.imports_resolved += 1 sep = "/" if pack is not None and pack.module_style != "dotted" else "." for nm in names: - tgt = exports.get(key, {}).get(nm) + # a def of the target module, or a name it re-exports + # (Python `pkg/__init__.py` re-export, BUG-CARRIED feat-015) + tgt = exports.get(key, {}).get(nm) or reexports.get(key, {}).get(nm) if tgt: binding[nm] = tgt continue diff --git a/tests/ingest/test_inherits.py b/tests/ingest/test_inherits.py index 281aed7..d2cd64e 100644 --- a/tests/ingest/test_inherits.py +++ b/tests/ingest/test_inherits.py @@ -66,6 +66,37 @@ async def test_external_base_not_guessed(tmp_path: Path) -> None: await store.close() +async def test_reexported_base_via_package_init(tmp_path: Path) -> None: + # A base class defined in pkg/contracts.py, re-exported by pkg/__init__.py, + # and inherited via the *package* import `from pkg import Base` — the real + # `class KuzuGraphStore(GraphStore)` shape (BUG-CARRIED feat-015). + files = { + "pkg/__init__.py": "from pkg.contracts import Base\n", + "pkg/contracts.py": "class Base:\n pass\n", + "pkg/impl.py": "from pkg import Base\n\n\nclass Impl(Base):\n pass\n", + } + store = await _resolve(tmp_path, files) + try: + assert await _supers(store, "Impl#") == {"Base#"} # re-exported base resolves + finally: + await store.close() + + +async def test_reexported_base_relative_init(tmp_path: Path) -> None: + # Same, but the __init__ uses a *relative* re-export (`from .contracts …`), + # matching this project's own core/__init__.py. + files = { + "pkg/__init__.py": "from .contracts import Base\n", + "pkg/contracts.py": "class Base:\n pass\n", + "pkg/impl.py": "from pkg import Base\n\n\nclass Impl(Base):\n pass\n", + } + store = await _resolve(tmp_path, files) + try: + assert await _supers(store, "Impl#") == {"Base#"} + finally: + await store.close() + + async def test_inherits_is_idempotent(tmp_path: Path) -> None: store = await _resolve(tmp_path, {"m.py": "class A:\n pass\n\n\nclass B(A):\n pass\n"}) try: From 776d7e383a09036ce36e83d68ff76c13c159f875 Mon Sep 17 00:00:00 2001 From: Khemchand Joshi Date: Wed, 8 Jul 2026 15:24:18 +0530 Subject: [PATCH 10/11] style: ruff format codegraph.py + test_query_config.py (CI format check) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Formatting only — the feat-015 edits to CodeGraph.query_graph and the config tests weren't run through `ruff format`. No behaviour change. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/agentforge_graph/ingest/codegraph.py | 4 +--- tests/test_query_config.py | 13 +++++++++++-- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/agentforge_graph/ingest/codegraph.py b/src/agentforge_graph/ingest/codegraph.py index f2d05db..0a1f48d 100644 --- a/src/agentforge_graph/ingest/codegraph.py +++ b/src/agentforge_graph/ingest/codegraph.py @@ -496,9 +496,7 @@ async def retrieve( allow_ids=allow_ids, ) - async def query_graph( - self, text: str, settings: QuerySettings | None = None - ) -> ResultTable: + async def query_graph(self, text: str, settings: QuerySettings | None = None) -> ResultTable: """Execute a read-only structural query (feat-015). ``settings`` bounds the run (row cap / timeout / expansions); defaults are used if omitted. Raises ``QueryError`` on bad input, ``QueryDisabled`` on a non-query diff --git a/tests/test_query_config.py b/tests/test_query_config.py index 11f041e..130ab35 100644 --- a/tests/test_query_config.py +++ b/tests/test_query_config.py @@ -55,8 +55,17 @@ async def indexed(tmp_path: Path) -> Path: def test_cli_refuses_when_disabled(indexed: Path, capsys: pytest.CaptureFixture[str]) -> None: cfg = indexed / "ckg.yaml" cfg.write_text("query:\n enabled: false\n") - rc = main(["query", "--path", str(indexed), "--config", str(cfg), - "--graph", "MATCH (c:Class) RETURN c.name"]) + rc = main( + [ + "query", + "--path", + str(indexed), + "--config", + str(cfg), + "--graph", + "MATCH (c:Class) RETURN c.name", + ] + ) assert rc == 2 assert "disabled" in capsys.readouterr().err From 859437a92b2eaccd6162cd6b1399f1fe2ff3803a Mon Sep 17 00:00:00 2001 From: Khemchand Joshi Date: Wed, 8 Jul 2026 15:26:23 +0530 Subject: [PATCH 11/11] fix(neo4j): type run_query pull result (mypy no-any-return) neo4j's session.execute_read returns Any; assign to a typed local so mypy 2.1.0's no-any-return is satisfied. No behaviour change. (A stale local mypy incremental cache masked this; a --no-incremental run reproduces CI.) Co-Authored-By: Claude Opus 4.8 (1M context) --- src/agentforge_graph/store/neo4j_store.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/agentforge_graph/store/neo4j_store.py b/src/agentforge_graph/store/neo4j_store.py index 416d44c..b345438 100644 --- a/src/agentforge_graph/store/neo4j_store.py +++ b/src/agentforge_graph/store/neo4j_store.py @@ -322,7 +322,8 @@ async def pull() -> BoundedResult: # execute_read runs a genuine READ transaction — a write would be # refused by the session (gate #2), on top of the AST gate (gate #1). async with self._driver.session(database=self._database) as session: - return await session.execute_read(_tx) + result: BoundedResult = await session.execute_read(_tx) + return result bounded = await run_bounded_async(pull, settings) return ResultTable(