mirror of
https://github.com/ModernRelay/omnigraph.git
synced 2026-06-21 02:28:07 +02:00
feat(cli): surface stored-query @description/@instruction in queries list (#280)
* test: e2e coverage for @description/@instruction surfaces Add end-to-end tests pinning the two annotation surfaces as they exist today, at their real boundaries: - engine (lifecycle.rs): schema-level @description (node/edge/property) and @instruction (node/edge) persist verbatim into the on-disk _schema.ir.json through Omnigraph::init; property-level @instruction aborts init and writes no schema IR. - server (stored_queries.rs): query-level @description/@instruction on a stored query surface as typed QueryCatalogEntry fields over GET /queries, and a query declaring neither omits both fields. No behavior change — these document the current contract. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(cli): surface stored-query @description/@instruction in `queries list` A stored query's @description/@instruction are its catalog metadata — what it does and how to invoke it. The HTTP GET /queries catalog already carries them, but `omnigraph queries list` dropped both fields in human and --json output even though they were available on the registry entry. Carry description/instruction on QueriesListItem (Option, skipped when None) and copy them from the query decl. Human output prints an indented `description:` / `instruction:` line per query when present; --json includes the fields when present and omits them otherwise — matching the HTTP catalog shape documented in docs/user/operations/server.md. Tests (cli_queries.rs): a query with both annotations surfaces them in human + --json; a query with neither prints no annotation lines and omits both JSON fields. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(cli): document `queries list` output incl. description/instruction Per AGENTS.md maintenance Rule 1, document the user-visible `queries list` output alongside the field addition. The `queries` command family had no row in the CLI reference top-level table; add one covering `list` (human + --json shapes, with description/instruction shown only when declared, matching the HTTP GET /queries catalog) and `validate`. Addresses the Greptile P2 review finding on PR #280. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cli): indent multiline stored-query annotations in `queries list` A `@description`/`@instruction` value can be multiline (GQ string literals admit newlines), which made the human `queries list` output break back to the left margin on continuation lines. Indent continuation lines to align under the first via a `print_query_annotation` helper. Addresses review feedback from @martin-g on PR #280. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Ragnor Comerford <ragnor.comerford@gmail.com>
This commit is contained in:
parent
7fd23c54a3
commit
3feb23af05
6 changed files with 300 additions and 0 deletions
|
|
@ -875,6 +875,25 @@ pub(crate) async fn execute_queries_validate(
|
|||
Ok(())
|
||||
}
|
||||
|
||||
/// Print a stored-query annotation under its `queries list` entry. A
|
||||
/// `@description`/`@instruction` value may be multiline (GQ string literals
|
||||
/// admit newlines); continuation lines are indented to align under the first
|
||||
/// so the catalog stays readable instead of breaking the left margin.
|
||||
fn print_query_annotation(label: &str, value: &str) {
|
||||
let prefix = format!(" {label}: ");
|
||||
let continuation = " ".repeat(prefix.len());
|
||||
let mut lines = value.split('\n');
|
||||
match lines.next() {
|
||||
Some(first) => {
|
||||
println!("{prefix}{first}");
|
||||
for line in lines {
|
||||
println!("{continuation}{line}");
|
||||
}
|
||||
}
|
||||
None => println!("{prefix}"),
|
||||
}
|
||||
}
|
||||
|
||||
/// `queries list --cluster <dir>` (RFC-011): list the catalog's stored queries.
|
||||
/// With `--graph`, scope to one graph.
|
||||
pub(crate) async fn execute_queries_list(
|
||||
|
|
@ -893,6 +912,8 @@ pub(crate) async fn execute_queries_list(
|
|||
mcp_expose: q.expose,
|
||||
tool_name: q.tool_name.clone(),
|
||||
mutation: q.is_mutation(),
|
||||
description: q.decl.description.clone(),
|
||||
instruction: q.decl.instruction.clone(),
|
||||
params: q
|
||||
.decl
|
||||
.params
|
||||
|
|
@ -933,6 +954,12 @@ pub(crate) async fn execute_queries_list(
|
|||
String::new()
|
||||
};
|
||||
println!("{kind} {}({params}){mcp}", q.name);
|
||||
if let Some(description) = &q.description {
|
||||
print_query_annotation("description", description);
|
||||
}
|
||||
if let Some(instruction) = &q.instruction {
|
||||
print_query_annotation("instruction", instruction);
|
||||
}
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
|
|
|
|||
|
|
@ -849,6 +849,13 @@ pub(crate) struct QueriesListItem {
|
|||
pub(crate) mcp_expose: bool,
|
||||
pub(crate) tool_name: Option<String>,
|
||||
pub(crate) mutation: bool,
|
||||
/// `@description` from the query declaration — what the query is for.
|
||||
/// Carried so the CLI catalog matches the HTTP `GET /queries` surface.
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub(crate) description: Option<String>,
|
||||
/// `@instruction` from the query declaration — how/when to invoke it.
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub(crate) instruction: Option<String>,
|
||||
pub(crate) params: Vec<QueriesParam>,
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -231,6 +231,125 @@ fn queries_list_prints_registered_query() {
|
|||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn queries_list_surfaces_description_and_instruction() {
|
||||
// `@description`/`@instruction` are the whole point of a stored query in a
|
||||
// catalog — they tell an agent/operator what it does and how to invoke it.
|
||||
// The CLI catalog must surface them in both human and --json output, to
|
||||
// match the HTTP `GET /queries` surface.
|
||||
let cluster = converged_cluster_with_query(
|
||||
"described.gq",
|
||||
"query described($name: String) \
|
||||
@description(\"Find a person by exact name.\") \
|
||||
@instruction(\"Use for exact lookups; prefer search for fuzzy matches.\") \
|
||||
{ match { $p: Person { name: $name } } return { $p.age } }",
|
||||
" described:\n file: ./described.gq\n",
|
||||
);
|
||||
|
||||
// Human output.
|
||||
let output = output_success(
|
||||
cli().arg("queries").arg("list").arg("--cluster").arg(cluster.path()),
|
||||
);
|
||||
let stdout = stdout_string(&output);
|
||||
assert!(
|
||||
stdout.contains("description: Find a person by exact name."),
|
||||
"human list must show @description; stdout:\n{stdout}"
|
||||
);
|
||||
assert!(
|
||||
stdout.contains("instruction: Use for exact lookups; prefer search for fuzzy matches."),
|
||||
"human list must show @instruction; stdout:\n{stdout}"
|
||||
);
|
||||
|
||||
// --json output.
|
||||
let output = output_success(
|
||||
cli()
|
||||
.arg("queries")
|
||||
.arg("list")
|
||||
.arg("--cluster")
|
||||
.arg(cluster.path())
|
||||
.arg("--json"),
|
||||
);
|
||||
let body: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap();
|
||||
let entry = body["queries"]
|
||||
.as_array()
|
||||
.unwrap()
|
||||
.iter()
|
||||
.find(|q| q["name"] == "described")
|
||||
.unwrap();
|
||||
assert_eq!(entry["description"], "Find a person by exact name.");
|
||||
assert_eq!(
|
||||
entry["instruction"],
|
||||
"Use for exact lookups; prefer search for fuzzy matches."
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn queries_list_indents_multiline_annotation_continuation() {
|
||||
// GQ string literals admit newlines, so a `@description`/`@instruction`
|
||||
// can be multiline. Human output must indent continuation lines to align
|
||||
// under the first rather than breaking back to the left margin.
|
||||
let cluster = converged_cluster_with_query(
|
||||
"multi.gq",
|
||||
"query multi($name: String) \
|
||||
@description(\"line one\\nline two\") \
|
||||
{ match { $p: Person { name: $name } } return { $p.age } }",
|
||||
" multi:\n file: ./multi.gq\n",
|
||||
);
|
||||
let output = output_success(
|
||||
cli().arg("queries").arg("list").arg("--cluster").arg(cluster.path()),
|
||||
);
|
||||
let stdout = stdout_string(&output);
|
||||
// " description: " is 17 chars wide; the continuation aligns under it.
|
||||
assert!(
|
||||
stdout.contains(" description: line one\n line two"),
|
||||
"multiline annotation must indent the continuation; stdout:\n{stdout}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn queries_list_omits_annotations_when_absent() {
|
||||
// The other half of the contract: a query that declares neither annotation
|
||||
// prints no extra lines and omits both JSON fields entirely. This keeps the
|
||||
// catalog clean rather than echoing empty `description:`/`instruction:`.
|
||||
let cluster = converged_cluster_with_query(
|
||||
"bare.gq",
|
||||
"query bare() { match { $p: Person } return { $p.name } }",
|
||||
" bare:\n file: ./bare.gq\n",
|
||||
);
|
||||
|
||||
// Human output: the query is listed, but no annotation lines.
|
||||
let output = output_success(
|
||||
cli().arg("queries").arg("list").arg("--cluster").arg(cluster.path()),
|
||||
);
|
||||
let stdout = stdout_string(&output);
|
||||
assert!(stdout.contains("bare()"), "stdout:\n{stdout}");
|
||||
assert!(
|
||||
!stdout.contains("description:") && !stdout.contains("instruction:"),
|
||||
"a query without annotations prints no annotation lines; stdout:\n{stdout}"
|
||||
);
|
||||
|
||||
// --json output: both fields omitted (not present as null).
|
||||
let output = output_success(
|
||||
cli()
|
||||
.arg("queries")
|
||||
.arg("list")
|
||||
.arg("--cluster")
|
||||
.arg(cluster.path())
|
||||
.arg("--json"),
|
||||
);
|
||||
let body: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap();
|
||||
let entry = body["queries"]
|
||||
.as_array()
|
||||
.unwrap()
|
||||
.iter()
|
||||
.find(|q| q["name"] == "bare")
|
||||
.unwrap();
|
||||
assert!(
|
||||
entry.get("description").is_none() && entry.get("instruction").is_none(),
|
||||
"a query without annotations omits both JSON fields: {entry}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn queries_validate_requires_a_cluster() {
|
||||
// RFC-011: with no --cluster (and no cluster profile), the command errors
|
||||
|
|
|
|||
|
|
@ -369,6 +369,47 @@ async fn list_queries_is_read_gated_so_a_non_invoker_can_list() {
|
|||
);
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn list_queries_surfaces_query_description_and_instruction() {
|
||||
// E2e for the query-level `.gq` surface: `@description`/`@instruction` on
|
||||
// a stored query declaration are carried through to clients via the typed
|
||||
// `QueryCatalogEntry` fields over `GET /queries`. A query without them
|
||||
// omits both fields (serde `skip_serializing_if = "Option::is_none"`).
|
||||
let described = "query described($name: String) \
|
||||
@description(\"Find a person by exact name.\") \
|
||||
@instruction(\"Use for exact lookups; prefer search for fuzzy matches.\") \
|
||||
{ match { $p: Person { name: $name } } return { $p.age } }";
|
||||
let (_temp, app) = app_with_stored_queries(
|
||||
&[
|
||||
("described", described, true),
|
||||
("bare", "query bare() { match { $p: Person } return { $p.name } }", true),
|
||||
],
|
||||
&[("act-invoke", "t-invoke")],
|
||||
INVOKE_POLICY_YAML,
|
||||
)
|
||||
.await;
|
||||
let (status, body) = json_response(&app, get_request(&g("/queries"), "t-invoke")).await;
|
||||
assert_eq!(status, StatusCode::OK, "body: {body}");
|
||||
let entries = body["queries"].as_array().unwrap();
|
||||
|
||||
let described = entries.iter().find(|q| q["name"] == "described").unwrap();
|
||||
assert_eq!(
|
||||
described["description"], "Find a person by exact name.",
|
||||
"query @description surfaces over GET /queries: {described}"
|
||||
);
|
||||
assert_eq!(
|
||||
described["instruction"],
|
||||
"Use for exact lookups; prefer search for fuzzy matches.",
|
||||
"query @instruction surfaces over GET /queries: {described}"
|
||||
);
|
||||
|
||||
let bare = entries.iter().find(|q| q["name"] == "bare").unwrap();
|
||||
assert!(
|
||||
bare.get("description").is_none() && bare.get("instruction").is_none(),
|
||||
"a query without the annotations omits both fields: {bare}"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn list_queries_is_empty_when_no_registry() {
|
||||
let (_temp, app) = app_for_loaded_graph_with_auth("demo-token").await;
|
||||
|
|
|
|||
|
|
@ -304,3 +304,108 @@ async fn init_with_force_recovers_from_orphan_schema_files() {
|
|||
"force-recovered graph must have full schema state written"
|
||||
);
|
||||
}
|
||||
|
||||
/// E2e for the schema-level `.pg` surface: `@description` (node / edge /
|
||||
/// property) and `@instruction` (node / edge only) parse, validate, and
|
||||
/// persist verbatim into the on-disk `_schema.ir.json` through `Omnigraph::init`
|
||||
/// — the contract that surfaces them in catalog metadata for tooling.
|
||||
#[tokio::test]
|
||||
async fn schema_annotations_persist_into_ir_json_on_init() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let uri = dir.path().to_str().unwrap();
|
||||
|
||||
let schema = r#"
|
||||
node Task @description("Tracked work item") @instruction("Prefer querying by slug") {
|
||||
slug: String @key @description("Stable external identifier")
|
||||
}
|
||||
|
||||
edge DependsOn: Task -> Task @description("Hard dependency") @instruction("Use only for blockers")
|
||||
"#;
|
||||
|
||||
Omnigraph::init(uri, schema).await.unwrap();
|
||||
|
||||
let ir_json = fs::read_to_string(dir.path().join("_schema.ir.json")).unwrap();
|
||||
let ir: serde_json::Value = serde_json::from_str(&ir_json).unwrap();
|
||||
|
||||
// Helper: collect the {name -> value} map of annotations that carry a
|
||||
// string value. Value-less annotations (e.g. `@key`, which also desugars
|
||||
// to a constraint) are skipped — they aren't what this test asserts.
|
||||
let anns = |v: &serde_json::Value| -> std::collections::BTreeMap<String, String> {
|
||||
v["annotations"]
|
||||
.as_array()
|
||||
.unwrap()
|
||||
.iter()
|
||||
.filter_map(|a| {
|
||||
Some((
|
||||
a["name"].as_str()?.to_string(),
|
||||
a["value"].as_str()?.to_string(),
|
||||
))
|
||||
})
|
||||
.collect()
|
||||
};
|
||||
|
||||
let node = ir["nodes"]
|
||||
.as_array()
|
||||
.unwrap()
|
||||
.iter()
|
||||
.find(|n| n["name"] == "Task")
|
||||
.unwrap();
|
||||
let node_anns = anns(node);
|
||||
assert_eq!(node_anns.get("description").map(String::as_str), Some("Tracked work item"));
|
||||
assert_eq!(
|
||||
node_anns.get("instruction").map(String::as_str),
|
||||
Some("Prefer querying by slug"),
|
||||
"node @instruction persists into _schema.ir.json"
|
||||
);
|
||||
|
||||
let prop = node["properties"]
|
||||
.as_array()
|
||||
.unwrap()
|
||||
.iter()
|
||||
.find(|p| p["name"] == "slug")
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
anns(prop).get("description").map(String::as_str),
|
||||
Some("Stable external identifier"),
|
||||
"property @description persists into _schema.ir.json"
|
||||
);
|
||||
|
||||
let edge = ir["edges"]
|
||||
.as_array()
|
||||
.unwrap()
|
||||
.iter()
|
||||
.find(|e| e["name"] == "DependsOn")
|
||||
.unwrap();
|
||||
let edge_anns = anns(edge);
|
||||
assert_eq!(edge_anns.get("description").map(String::as_str), Some("Hard dependency"));
|
||||
assert_eq!(edge_anns.get("instruction").map(String::as_str), Some("Use only for blockers"));
|
||||
}
|
||||
|
||||
/// `@instruction` is rejected on a property at compile time, so init aborts
|
||||
/// before any graph state is written (mirrors the parser-level rejection from
|
||||
/// the full engine boundary).
|
||||
#[tokio::test]
|
||||
async fn init_rejects_instruction_on_property() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let uri = dir.path().to_str().unwrap();
|
||||
|
||||
let schema = r#"
|
||||
node Task {
|
||||
slug: String @key @instruction("bad")
|
||||
}
|
||||
"#;
|
||||
|
||||
// `Omnigraph` is not `Debug`, so match rather than `unwrap_err`.
|
||||
let err = match Omnigraph::init(uri, schema).await {
|
||||
Ok(_) => panic!("property-level @instruction must abort init"),
|
||||
Err(err) => err,
|
||||
};
|
||||
assert!(
|
||||
err.to_string().contains("@instruction is only supported on node and edge types"),
|
||||
"property-level @instruction must abort init: {err}"
|
||||
);
|
||||
assert!(
|
||||
!dir.path().join("_schema.ir.json").exists(),
|
||||
"rejected init must not persist a schema IR"
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -26,6 +26,7 @@ Top-level command families and subcommands. Graph-targeting commands accept a po
|
|||
| `cleanup --keep N --older-than 7d --confirm` | destructive version GC (`--confirm` to execute; also needs `--yes` against a non-local `s3://` target — see *Write diagnostics & destructive confirmation*) |
|
||||
| `embed` | offline JSONL embedding pipeline |
|
||||
| `policy validate \| test \| explain` | Cedar tooling against a cluster's applied policies (`--cluster <dir>`; `--graph <id>` picks a graph's bundle when several apply). `test` takes `--tests <file>`; `explain` takes `--actor`/`--action`/`--branch`/`--target-branch` |
|
||||
| `queries list \| validate` | inspect a cluster's applied stored-query registry (`--cluster <dir\|uri>`; `--graph <id>` to scope one graph). `list` prints each query's kind (read/mutation), name, typed params, and `[mcp: …]` exposure; a query's `@description`/`@instruction` are shown as indented `description:` / `instruction:` lines when declared (omitted otherwise). `--json` emits `{name, mcp_expose, tool_name, mutation, params}` plus `description`/`instruction` **only when present** — matching the HTTP `GET /queries` catalog ([server.md](../operations/server.md)). `validate` type-checks the registry and exits non-zero on a broken query |
|
||||
| `profile list \| show [<name>]` | read-only inspection of `~/.omnigraph/config.yaml` profiles. `list` shows each profile's binding (server/cluster/store) + default graph and marks the `$OMNIGRAPH_PROFILE`-active one; JSON keeps `binding` and adds `scope_kind`, `target`, `valid`, and `error`; `show` resolves one profile's scope (endpoint + default graph), defaulting to the active profile, else the flat operator defaults |
|
||||
| `version` / `-v` | print `omnigraph 0.3.x` |
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue