fix(ogar-adapter-surrealql): quote non-bare identifiers in emitted DDL

Closes Codex P2 on PR #33 (now merged).

# The bug

When `surrealql-hint` is enabled (PR #33), `register_class_knowable_from`
renders the schema DDL for every registered Class via
`emit_surrealql_ddl`. For Odoo-style class names — which legitimately
use the dotted form (`sale.order`, `account.move.line`,
`res.partner`) per `ogar-ontology::class_identity` — the emitter was
producing raw `DEFINE TABLE sale.order …`. That's invalid SurrealQL:
the dot terminates the bare identifier, so the parser either rejects
the DDL or mis-parses the dot as a path operator.

Codex flagged this on PR #33 line 272: "enabling this feature for
Odoo-style classes stores an invalid schema hint instead of a self-
describing registry entry".

# The fix

New helper in `ogar-adapter-surrealql`:

    fn surrealql_ident(name: &str) -> String

Returns the bare identifier if `name` matches `[A-Za-z_][A-Za-z0-9_]*`,
or backtick-quoted otherwise (with embedded backticks doubled per
SurrealQL convention). Examples:

    surrealql_ident("widget")             -> "widget"
    surrealql_ident("sale.order")         -> "`sale.order`"
    surrealql_ident("account.move.line")  -> "`account.move.line`"
    surrealql_ident("kebab-case")         -> "`kebab-case`"
    surrealql_ident("weird`name")         -> "`weird``name`"

Applied at every identifier-emission site:

  - `emit_class` — DEFINE TABLE <name> (once at the top; `class.name`
    pre-quoted into `table_ident` and reused as `table` parameter for
    the field emitters).
  - `emit_field_attr` — DEFINE FIELD <name> ON <table>; quotes
    `attr.name` locally.
  - `emit_field_assoc` — DEFINE FIELD <name> ON <table> TYPE
    record<<target>>; quotes `assoc.name` + record target locally
    (so `record<\`res.partner\`>` lands correctly).
  - `emit_field_enum` — DEFINE FIELD <column> ON <table> TYPE …;
    quotes `enum_decl.column` locally.

The comment-marker line for non-owning sides (`-- {table} HasMany …`)
is left un-quoted — it's not parsed; readable text is more useful.

# Walker side is automatically correct

`parse_surrealql_ddl` extracts identifiers via
`Expr::Path { start: Ident { text }, parts: None }` -> `ast[ident.text]`.
The SurrealDB parser unwraps backtick-quoting at the lexer level, so
the walker yields `"sale.order"` (unquoted text) whether the DDL had
backticks or not. Round-trip works without walker changes — verified
by the new round-trip tests.

# Tests added (8 new; 20 default + 33 with parser feature)

Helper unit tests:
  - `bare_identifier_passes_through_unquoted`
  - `non_bare_identifier_gets_backtick_quoted` (covers `sale.order`,
    `account.move.line`, `res.partner`, kebab-case, leading digit,
    space)
  - `embedded_backticks_get_doubled`
  - `empty_string_is_not_a_bare_identifier`

Emit-site tests:
  - `emit_class_with_odoo_dotted_name_quotes_the_table` — asserts
    both DEFINE TABLE and DEFINE FIELD ON clauses use the quoted form.
  - `emit_class_with_odoo_belongs_to_quotes_record_target` — asserts
    `record<\`res.partner\`>` lands correctly.

Round-trip (feature-on) tests — the load-bearing ones:
  - `round_trip_odoo_dotted_table_name` — builds IR with
    `sale.order` + belongs_to → `res.partner`, emits, parses,
    asserts the IR survives the trip with names byte-identical.
  - `round_trip_three_segment_odoo_name` — same for
    `account.move.line`.

# Verification

  cargo test -p ogar-adapter-surrealql                          -> 20/20
  cargo test -p ogar-adapter-surrealql --features surrealdb-parser -> 33/33
  cargo test -p ogar-knowable-from --features surrealql-hint    -> 10/10
  cargo check --workspace --all-targets                         -> clean

PII abort-guard (word-boundary): CLEAN.

# Note on scope

This is a fix for the merged PR #33 — adding the missing identifier
quoting at the emit layer. Same change-policy as ADR-023 (the IR is
fixed; adapter emit logic is where source-dialect concerns get
absorbed). No `Class` IR change; no adapter signature change;
existing call sites unaffected (all the bare identifiers they use
pass through `surrealql_ident` unchanged).

https://claude.ai/code/session_01PBTGaPCSnnt6u3pjXpbLwY

Author: claude

crates/ogar-adapter-surrealql/src/lib.rs

@@ -499,26 +499,31 @@ fn emit_class(class: &Class, out: &mut String) {
     // SCHEMAFULL is the safe default for OGAR-produced classes — they have
     // a typed shape. If a producer needs SCHEMALESS it can wire it through
     // a Class decorator and we extend here.
-    out.push_str(&format!("DEFINE TABLE {} SCHEMAFULL", class.name));
+    let table_ident = surrealql_ident(&class.name);
+    out.push_str(&format!("DEFINE TABLE {table_ident} SCHEMAFULL"));
     if let Some(desc) = &class.description {
         out.push_str(&format!(" COMMENT {}", surreal_string_literal(desc)));
     }
     out.push_str(";\n");
 
     // DEFINE FIELD per Attribute
     for attr in &class.attributes {
-        emit_field_attr(&class.name, attr, out);
+        emit_field_attr(&table_ident, attr, out);
     }
     // DEFINE FIELD per Association (renders as record<target>)
     for assoc in &class.associations {
-        emit_field_assoc(&class.name, assoc, out);
+        emit_field_assoc(&table_ident, assoc, out);
     }
     // DEFINE FIELD per EnumDecl (renders as string ASSERT $value IN [...])
     for enum_decl in &class.enums {
-        emit_field_enum(&class.name, enum_decl, out);
+        emit_field_enum(&table_ident, enum_decl, out);
     }
 }
 
+// NOTE: `table` arrives already passed through `surrealql_ident` at the
+// `emit_class` call site (each identifier quoted once at its source).
+// Field-side identifiers (`attr.name`, `assoc.name`, target record name,
+// enum column) are quoted locally here.
 fn emit_field_attr(table: &str, attr: &Attribute, out: &mut String) {
     let surreal_type = attr
         .type_name
@@ -537,7 +542,9 @@ fn emit_field_attr(table: &str, attr: &Attribute, out: &mut String) {
     };
     out.push_str(&format!(
         "DEFINE FIELD {} ON {} TYPE {};\n",
-        attr.name, table, wrapped
+        surrealql_ident(&attr.name),
+        table,
+        wrapped
     ));
 }
 
@@ -552,20 +559,27 @@ fn emit_field_assoc(table: &str, assoc: &Association, out: &mut String) {
                 .class_name
                 .as_deref()
                 .unwrap_or(&assoc.name); // fallback: relation name as target
+            // The record target is a SurrealQL identifier — quote if
+            // non-bare (Odoo `res.partner` → `` `res.partner` ``).
+            let target_ident = surrealql_ident(target);
             let ty = if assoc.optional.unwrap_or(false) {
-                format!("option<record<{target}>>")
+                format!("option<record<{target_ident}>>")
             } else {
-                format!("record<{target}>")
+                format!("record<{target_ident}>")
             };
             out.push_str(&format!(
                 "DEFINE FIELD {} ON {} TYPE {};\n",
-                assoc.name, table, ty
+                surrealql_ident(&assoc.name),
+                table,
+                ty
             ));
         }
         AssociationKind::HasOne | AssociationKind::HasMany | AssociationKind::HasAndBelongsToMany => {
             // Non-owning / join-table sides: no field on this table.
             // Roundtrip note for unmap: the inverse side reconstructs from
             // the owning side's `record<X>` field on the target table.
+            // The comment body isn't parsed; leave names un-quoted for
+            // readability.
             out.push_str(&format!(
                 "-- {} {:?} {} (no DEFINE FIELD — non-owning / join side)\n",
                 table, assoc.kind, assoc.name
@@ -582,6 +596,7 @@ fn emit_field_assoc(table: &str, assoc: &Association, out: &mut String) {
 fn emit_field_enum(table: &str, enum_decl: &EnumDecl, out: &mut String) {
     // Per `ogar-vocab::EnumDecl`: `column` names the field; `source`
     // carries the variant list (Static / Computed / Add).
+    let column_ident = surrealql_ident(&enum_decl.column);
     match &enum_decl.source {
         ogar_vocab::EnumSource::Static(items) => {
             let variants = items
@@ -590,8 +605,7 @@ fn emit_field_enum(table: &str, enum_decl: &EnumDecl, out: &mut String) {
                 .collect::<Vec<_>>()
                 .join(", ");
             out.push_str(&format!(
-                "DEFINE FIELD {} ON {} TYPE string ASSERT $value IN [{}];\n",
-                enum_decl.column, table, variants
+                "DEFINE FIELD {column_ident} ON {table} TYPE string ASSERT $value IN [{variants}];\n"
             ));
         }
         ogar_vocab::EnumSource::Add { items, parent_selection } => {
@@ -603,8 +617,7 @@ fn emit_field_enum(table: &str, enum_decl: &EnumDecl, out: &mut String) {
                 .collect::<Vec<_>>()
                 .join(", ");
             out.push_str(&format!(
-                "DEFINE FIELD {} ON {} TYPE string /* selection_add from {} */ ASSERT $value IN [{}];\n",
-                enum_decl.column, table, parent_selection, variants
+                "DEFINE FIELD {column_ident} ON {table} TYPE string /* selection_add from {parent_selection} */ ASSERT $value IN [{variants}];\n"
             ));
         }
         ogar_vocab::EnumSource::Computed(body) => {
@@ -613,8 +626,7 @@ fn emit_field_enum(table: &str, enum_decl: &EnumDecl, out: &mut String) {
             // side to reconstitute.
             let escaped = body.replace("*/", "* /");
             out.push_str(&format!(
-                "DEFINE FIELD {} ON {} TYPE string /* computed: {} */;\n",
-                enum_decl.column, table, escaped
+                "DEFINE FIELD {column_ident} ON {table} TYPE string /* computed: {escaped} */;\n"
             ));
         }
         // `EnumSource` is `#[non_exhaustive]` in `ogar-vocab`; the three arms
@@ -655,6 +667,39 @@ fn surreal_string_literal(s: &str) -> String {
     format!("'{escaped}'")
 }
 
+/// Render a name as a SurrealQL identifier: bare if it matches the
+/// `[A-Za-z_][A-Za-z0-9_]*` bare-identifier subset, backtick-quoted
+/// otherwise.
+///
+/// OGAR class names can carry dots (Odoo dotted names like
+/// `sale.order` / `account.move.line`), hyphens, or other characters
+/// that aren't valid as SurrealQL bare identifiers. Closes Codex P2
+/// on PR #33: enabling `surrealql-hint` for Odoo-style classes was
+/// rendering invalid DDL (`DEFINE TABLE sale.order …` — the dot
+/// terminates the identifier).
+///
+/// The backtick-quoted form is the canonical SurrealQL non-bare
+/// identifier syntax; embedded backticks are escaped by doubling
+/// (`` `foo``bar` `` for an identifier containing one backtick).
+/// Round-trip safe: `parse_surrealql_ddl` reads the backtick-quoted
+/// form back into the same `Ident.text`.
+fn surrealql_ident(name: &str) -> String {
+    if is_bare_surrealql_identifier(name) {
+        name.to_string()
+    } else {
+        format!("`{}`", name.replace('`', "``"))
+    }
+}
+
+fn is_bare_surrealql_identifier(s: &str) -> bool {
+    let mut chars = s.chars();
+    match chars.next() {
+        Some(c) if c.is_ascii_alphabetic() || c == '_' => {}
+        _ => return false,
+    }
+    chars.all(|c| c.is_ascii_alphanumeric() || c == '_')
+}
+
 // ─────────────────────────────────────────────────────────────────────
 // Tests — lock the emit shape; parse_surrealql_ddl tests land with the
 // parser wiring.
@@ -1083,4 +1128,124 @@ mod tests {
             other => panic!("expected Unimplemented(feature off), got: {other:?}"),
         }
     }
+
+    // ── Codex P2 fix (PR #33) ──────────────────────────────────────────
+    // OGAR class names like `sale.order` (Odoo dotted form) aren't
+    // valid SurrealQL bare identifiers — the dot terminates the
+    // identifier. `surrealql_ident()` quotes them with backticks; the
+    // emit sites use it everywhere a Class/Attribute/Association/Enum
+    // name lands in DDL.
+    // ───────────────────────────────────────────────────────────────────
+
+    #[test]
+    fn bare_identifier_passes_through_unquoted() {
+        assert_eq!(surrealql_ident("widget"), "widget");
+        assert_eq!(surrealql_ident("WorkPackage"), "WorkPackage");
+        assert_eq!(surrealql_ident("work_package"), "work_package");
+        assert_eq!(surrealql_ident("_internal"), "_internal");
+        assert_eq!(surrealql_ident("a1b2c3"), "a1b2c3");
+    }
+
+    #[test]
+    fn non_bare_identifier_gets_backtick_quoted() {
+        // Odoo dotted names — the Codex P2 motivating case.
+        assert_eq!(surrealql_ident("sale.order"), "`sale.order`");
+        assert_eq!(surrealql_ident("account.move.line"), "`account.move.line`");
+        assert_eq!(surrealql_ident("res.partner"), "`res.partner`");
+        // Hyphens, leading digits, spaces — all need quoting.
+        assert_eq!(surrealql_ident("kebab-case"), "`kebab-case`");
+        assert_eq!(surrealql_ident("3leading"), "`3leading`");
+        assert_eq!(surrealql_ident("with space"), "`with space`");
+    }
+
+    #[test]
+    fn embedded_backticks_get_doubled() {
+        // Pathological case: ident contains a backtick. Double it
+        // per SurrealQL convention.
+        assert_eq!(surrealql_ident("weird`name"), "`weird``name`");
+    }
+
+    #[test]
+    fn empty_string_is_not_a_bare_identifier() {
+        assert!(!is_bare_surrealql_identifier(""));
+        assert_eq!(surrealql_ident(""), "``");
+    }
+
+    #[test]
+    fn emit_class_with_odoo_dotted_name_quotes_the_table() {
+        let mut c = Class::new("sale.order");
+        let mut amount = Attribute::new("amount_total");
+        amount.type_name = Some("decimal".into());
+        c.attributes.push(amount);
+        let ddl = emit_surrealql_ddl(&[c]);
+        // Critical: the table identifier is backtick-quoted in BOTH
+        // the DEFINE TABLE and the DEFINE FIELD ON clause.
+        assert!(
+            ddl.contains("DEFINE TABLE `sale.order` SCHEMAFULL"),
+            "expected backtick-quoted DEFINE TABLE, got: {ddl}"
+        );
+        assert!(
+            ddl.contains("DEFINE FIELD amount_total ON `sale.order` TYPE decimal;"),
+            "expected backtick-quoted ON clause, got: {ddl}"
+        );
+    }
+
+    #[test]
+    fn emit_class_with_odoo_belongs_to_quotes_record_target() {
+        // The record target name also needs quoting: record<`res.partner`>
+        let mut c = Class::new("sale.order");
+        let mut partner = Association::new(AssociationKind::BelongsTo, "partner_id");
+        partner.class_name = Some("res.partner".into());
+        c.associations.push(partner);
+        let ddl = emit_surrealql_ddl(&[c]);
+        assert!(
+            ddl.contains("DEFINE FIELD partner_id ON `sale.order` TYPE record<`res.partner`>;"),
+            "expected quoted target in record<…>, got: {ddl}"
+        );
+    }
+
+    #[cfg(feature = "surrealdb-parser")]
+    #[test]
+    fn round_trip_odoo_dotted_table_name() {
+        // Build IR with an Odoo dotted name + a dotted-name record
+        // target. Emit → parse → assert the IR survives the trip.
+        // Without `surrealql_ident`, emit would produce invalid DDL
+        // (`DEFINE TABLE sale.order …`) and the parser would either
+        // reject it or mis-parse the dot as a path operator — the
+        // bug Codex P2 flagged on PR #33.
+        let mut order = Class::new("sale.order");
+        let mut partner_id = Association::new(AssociationKind::BelongsTo, "partner_id");
+        partner_id.class_name = Some("res.partner".into());
+        order.associations.push(partner_id);
+        let mut amount = Attribute::new("amount_total");
+        amount.type_name = Some("decimal".into());
+        order.attributes.push(amount);
+
+        let ddl = emit_surrealql_ddl(&[order]);
+        let recovered = parse_surrealql_ddl(&ddl).expect("parse OK");
+        assert_eq!(recovered.len(), 1);
+        let r = &recovered[0];
+        assert_eq!(r.name, "sale.order");
+        assert_eq!(r.attributes.len(), 1);
+        assert_eq!(r.attributes[0].name, "amount_total");
+        assert_eq!(r.attributes[0].type_name.as_deref(), Some("decimal"));
+        assert_eq!(r.associations.len(), 1);
+        assert_eq!(r.associations[0].name, "partner_id");
+        assert_eq!(r.associations[0].class_name.as_deref(), Some("res.partner"));
+    }
+
+    #[cfg(feature = "surrealdb-parser")]
+    #[test]
+    fn round_trip_three_segment_odoo_name() {
+        // `account.move.line` — three-segment dotted name.
+        let c = Class::new("account.move.line");
+        let ddl = emit_surrealql_ddl(&[c]);
+        assert!(
+            ddl.contains("`account.move.line`"),
+            "three-segment name must be quoted, got: {ddl}"
+        );
+        let recovered = parse_surrealql_ddl(&ddl).expect("parse OK");
+        assert_eq!(recovered.len(), 1);
+        assert_eq!(recovered[0].name, "account.move.line");
+    }
 }