From 8d2128438e8123afbc588bf3a70309305a8ccbe8 Mon Sep 17 00:00:00 2001 From: aaltshuler Date: Tue, 16 Jun 2026 01:25:27 +0300 Subject: [PATCH] fix(cli): quote @embed annotation values in schema show so they round-trip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `render_annotations` emitted `@embed` values unquoted — `@embed(title, model=openai/text-embedding-3-large)`. The parser stores values via `decode_string_literal` (quotes stripped) and `annotation_kwarg` requires a quoted `literal`, so the rendered output did not re-parse: a `model` containing `/` or `-` is not a valid bare token. `schema show` therefore produced schema text that `schema apply`/lint would reject. Re-quote the positional value and every kwarg value as string literals, so `schema show` reproduces `@embed("title", model="openai/text-embedding-3-large")` and round-trips. Regression: `render_annotations_quotes_values_so_embed_round_trips` parses the rendered form back through the schema grammar. Addresses the bot-review finding on #248. Co-Authored-By: Claude Opus 4.8 --- crates/omnigraph-cli/src/output.rs | 49 ++++++++++++++++++++++++++++-- 1 file changed, 47 insertions(+), 2 deletions(-) diff --git a/crates/omnigraph-cli/src/output.rs b/crates/omnigraph-cli/src/output.rs index 7f2be2d..8352426 100644 --- a/crates/omnigraph-cli/src/output.rs +++ b/crates/omnigraph-cli/src/output.rs @@ -698,11 +698,16 @@ pub(crate) fn render_annotations(annotations: &[omnigraph_compiler::schema::ast: .iter() .map(|annotation| { let mut args: Vec = Vec::new(); + // Values are parsed via `decode_string_literal` (quotes stripped), so + // re-quote them as string literals on render — otherwise a value with + // non-ident chars (e.g. `model=openai/text-embedding-3-large`) fails to + // round-trip back through the schema parser (`annotation_kwarg` wants a + // quoted `literal`, not a bare token). if let Some(value) = &annotation.value { - args.push(value.clone()); + args.push(format!("\"{}\"", value)); } for (key, val) in &annotation.kwargs { - args.push(format!("{}={}", key, val)); + args.push(format!("{}=\"{}\"", key, val)); } if args.is_empty() { format!("@{}", annotation.name) @@ -919,3 +924,43 @@ pub(crate) fn resolve_table_render_options(config: &OmnigraphConfig) -> ReadRend .unwrap_or_default(), } } + +#[cfg(test)] +mod tests { + use omnigraph_compiler::schema::ast::Annotation; + use omnigraph_compiler::schema::parser::parse_schema; + use std::collections::BTreeMap; + + use super::render_annotations; + + #[test] + fn render_annotations_quotes_values_so_embed_round_trips() { + let mut kwargs = BTreeMap::new(); + kwargs.insert( + "model".to_string(), + "openai/text-embedding-3-large".to_string(), + ); + let embed = Annotation { + name: "embed".to_string(), + value: Some("title".to_string()), + kwargs, + }; + + let rendered = render_annotations(std::slice::from_ref(&embed)); + assert_eq!( + rendered, + r#"@embed("title", model="openai/text-embedding-3-large")"# + ); + + // The bug: an unquoted `model=openai/text-embedding-3-large` is not a + // valid `annotation_kwarg` literal, so `schema show` output did not + // re-parse. The rendered form must round-trip through the grammar. + let schema = format!("node Doc {{\ntitle: String\nembedding: Vector(3) {rendered}\n}}\n"); + let parsed = parse_schema(&schema); + assert!( + parsed.is_ok(), + "rendered @embed must re-parse: {:?}", + parsed.err() + ); + } +}