---
phase: 64
plan: 03
type: execute
wave: 2
depends_on: [01]
files_modified:
  - src/expand/resolution.rs
autonomous: true
requirements:
  - QID-04
must_haves:
  truths:
    - "quote_table_ref on an already-quoted FQN emits exactly ONE pair of quotes per part (no `\"\"\"triple\"\"\"` strings)"
    - "quote_table_ref on a bare or partially-quoted FQN still emits one pair of quotes per part (unchanged behaviour for the legacy unquoted case)"
    - "quote_table_ref preserves embedded `\"` characters via standard `\"\"` escape on emit"
    - "qualify_and_quote_table_ref uses STRUCTURAL part-count (not raw `.contains('.')`) to detect already-qualified inputs"
  artifacts:
    - path: "src/expand/resolution.rs"
      provides: "quote_table_ref delegates to ident::parse_qualified_identifier; qualify_and_quote_table_ref uses parts.len() for qualification detection"
      contains: "ident::parse_qualified_identifier"
  key_links:
    - from: "src/expand/resolution.rs::quote_table_ref"
      to: "src/ident.rs::parse_qualified_identifier"
      via: "function call; falls back to quote_ident(table) on parse error"
      pattern: "ident::parse_qualified_identifier"
    - from: "src/expand/resolution.rs::qualify_and_quote_table_ref"
      to: "src/ident.rs::parse_qualified_identifier"
      via: "structural is-qualified test via parts.len() > 1"
      pattern: "parts.len\\(\\)"
---

<objective>
Fix the secondary expansion-side bug: `quote_table_ref(s)` currently `s.split('.').map(quote_ident).join(".")` — which RE-QUOTES already-quoted input, producing `"""memory"""."""main"""."""orders_sv"""` for the bug-report reproduction. Replace with a delegate to `ident::parse_qualified_identifier` so we operate on UNQUOTED parts and emit deterministically.

Also fix `qualify_and_quote_table_ref`'s `.contains('.')` heuristic (Pitfall 4 from RESEARCH.md), which trips on names whose quoted parts contain `.` (e.g. `"a.b"`).

Coverage: QID-04 (no triple-quoting in expanded SQL).

This plan touches `src/expand/resolution.rs` ONLY and depends ONLY on 64-01's `src/ident.rs`. It runs in Wave 2 IN PARALLEL with 64-02 (which touches `src/parse.rs` + `src/query/table_function.rs`) — no file overlap.
</objective>

<execution_context>
@$HOME/.claude/get-shit-done/workflows/execute-plan.md
@$HOME/.claude/get-shit-done/templates/summary.md
</execution_context>

<context>
@.planning/phases/64-fix-quoted-identifier-handling/64-RESEARCH.md
@.planning/phases/64-fix-quoted-identifier-handling/64-01-SUMMARY.md

<interfaces>
<!-- Helpers from 64-01. -->

```rust
// src/ident.rs (created in 64-01)
pub fn parse_qualified_identifier(input: &str) -> Result<Vec<String>, String>;
```

<!-- Current code being replaced. -->

```rust
// src/expand/resolution.rs:16
#[must_use]
pub fn quote_ident(ident: &str) -> String {
    format!("\"{}\"", ident.replace('"', "\"\""))
}

// src/expand/resolution.rs:29 — THE BUG
#[must_use]
pub fn quote_table_ref(table: &str) -> String {
    table.split('.').map(quote_ident).collect::<Vec<_>>().join(".")
}

// src/expand/resolution.rs:47 — Pitfall 4
#[must_use]
pub fn qualify_and_quote_table_ref(table: &str, def: &SemanticViewDefinition) -> String {
    if table.contains('.') {                  // <-- structural test should use parse result
        return quote_table_ref(table);
    }
    // ... existing prepend-database-and-schema logic
}
```

<!-- Existing tests live at src/expand/resolution.rs:126+ in mod tests { mod quote_table_ref_tests {...} }. Extend that module — DO NOT scatter new tests. -->
</interfaces>
</context>

<tasks>

<task type="auto" tdd="true">
  <name>Task 1: Rewrite quote_table_ref to delegate to ident::parse_qualified_identifier + extend tests</name>
  <files>src/expand/resolution.rs</files>
  <read_first>
    - src/expand/resolution.rs (whole file — it's ~250 LOC)
    - src/ident.rs (helper module — confirm parse_qualified_identifier signature)
    - .planning/phases/64-fix-quoted-identifier-handling/64-RESEARCH.md §Example B, §Anti-Patterns (rewriting strategy), §Pitfall 4 (qualify_and_quote_table_ref)
  </read_first>
  <behavior>
    New `quote_table_ref` (replaces current lines 28–35):

    ```rust
    /// Quote a potentially dot-qualified table reference, normalising already-quoted input.
    ///
    /// Behaviour:
    /// - Bare:                `orders`               -> `"orders"`
    /// - Two-part bare:       `jaffle.raw_orders`    -> `"jaffle"."raw_orders"`
    /// - Three-part bare:     `catalog.schema.table` -> `"catalog"."schema"."table"`
    /// - Already quoted:      `"memory"."main"."v"`  -> `"memory"."main"."v"`  (idempotent)
    /// - Mixed quoting:       `main."v"`             -> `"main"."v"`
    /// - Embedded `""` escape: `"with""quote"`       -> `"with""quote"`        (preserved)
    ///
    /// Inputs that fail to parse as a SQL identifier (legacy / malformed strings) are
    /// emitted verbatim wrapped in a single pair of quotes via [`quote_ident`]; this
    /// preserves the BARE-name case behaviour and never produces double-quoting.
    #[must_use]
    pub fn quote_table_ref(table: &str) -> String {
        match crate::ident::parse_qualified_identifier(table) {
            Ok(parts) => parts.iter().map(|p| quote_ident(p)).collect::<Vec<_>>().join("."),
            Err(_) => quote_ident(table),
        }
    }
    ```

    Equivalence for the legacy case: for any bare unquoted dotted input like `jaffle.raw_orders`, `parse_qualified_identifier` returns `Ok(vec!["jaffle", "raw_orders"])`, which emits `"jaffle"."raw_orders"` — same as today.

    New tests to ADD to `mod tests::quote_table_ref_tests` (do NOT remove or rename existing tests):

    - already_quoted_simple: `quote_table_ref("\"orders\"") == "\"orders\""`
    - already_quoted_two_part: `quote_table_ref("\"jaffle\".\"raw_orders\"") == "\"jaffle\".\"raw_orders\""`
    - already_quoted_three_part: `quote_table_ref("\"memory\".\"main\".\"orders\"") == "\"memory\".\"main\".\"orders\""`
    - mixed_quoting_first_quoted: `quote_table_ref("\"main\".orders") == "\"main\".\"orders\""`
    - mixed_quoting_last_quoted: `quote_table_ref("main.\"orders\"") == "\"main\".\"orders\""`
    - mixed_quoting_middle_quoted: `quote_table_ref("db.\"schema\".table") == "\"db\".\"schema\".\"table\""`
    - embedded_double_quote_in_quoted_part: `quote_table_ref("\"with\"\"q\"") == "\"with\"\"q\""`
    - dot_inside_quoted_part: `quote_table_ref("\"a.b\"") == "\"a.b\""`  (single part — `.` is data, not separator)
    - whitespace_inside_quoted_part: `quote_table_ref("\"my table\"") == "\"my table\""`
    - idempotent_property: `let once = quote_table_ref("orders"); let twice = quote_table_ref(&once); assert_eq!(once, twice);` — covers the regression directly.
    - idempotent_fqn: same for `quote_table_ref("memory.main.orders")` → run through twice, must equal once.
    - malformed_falls_back: `quote_table_ref("\"unterminated") == "\"\"\"unterminated\""`  (single-quote fallback escapes the lone `"` and wraps; this is the documented Err fallback path — verify by reading the implementation).

    Pre-existing tests `simple_table_name`, `catalog_qualified`, `fully_qualified`, `reserved_word_parts` (at lines 158+) MUST continue to pass unchanged.
  </behavior>
  <action>
    1. Open `src/expand/resolution.rs`.

    2. Replace the body of `quote_table_ref` (lines 28–35) with the new implementation under <behavior>.
       - Keep the function name, visibility (`pub`), `#[must_use]`, and outer signature identical.
       - Update the doc comment block above the function to describe the new idempotency behaviour.

    3. Add the eleven new tests above to `mod tests::quote_table_ref_tests`. Locate the closing `}` of that submodule (around line 200) and insert the new `#[test] fn ...` items BEFORE it.

    4. Run `cargo test --lib expand::resolution::tests` — verify ALL tests (pre-existing + new) pass.

    5. Run `cargo test --lib` (full suite) — verify no regressions. This is essential because `quote_table_ref` is called from MANY expansion call sites; if any of them feeds in already-quoted input we didn't expect, behaviour may shift.

    Do NOT touch `qualify_and_quote_table_ref` in this task — that's Task 2 of this plan (separated so the diff stays small and reviewable).
  </action>
  <verify>
    <automated>cargo test --lib expand::resolution::tests -- --nocapture</automated>
  </verify>
  <acceptance_criteria>
    - `grep -n "parse_qualified_identifier" src/expand/resolution.rs` returns at least one match inside the body of `quote_table_ref`.
    - `grep -nE "fn already_quoted_|fn idempotent_|fn mixed_quoting_|fn embedded_double_quote_in_quoted_part|fn dot_inside_quoted_part|fn whitespace_inside_quoted_part|fn malformed_falls_back" src/expand/resolution.rs` returns at least 10 new test function definitions.
    - `cargo test --lib expand::resolution::tests` exits 0.
    - `cargo test --lib` exits 0.
    - No occurrence of `table.split('.').map(quote_ident)` remains in `src/expand/resolution.rs` (verified by `grep -n "split('.')" src/expand/resolution.rs` returning at most matches that aren't inside `quote_table_ref`).
  </acceptance_criteria>
  <done>
    quote_table_ref is idempotent on already-quoted input and unchanged on bare input. 10+ new tests cover the QID-04 surface. Full lib test suite green.
  </done>
</task>

<task type="auto" tdd="true">
  <name>Task 2: Fix qualify_and_quote_table_ref to use structural is-qualified test (parts.len() &gt; 1) instead of contains('.')</name>
  <files>src/expand/resolution.rs</files>
  <read_first>
    - src/expand/resolution.rs lines 40–95 (qualify_and_quote_table_ref full body)
    - src/ident.rs (parse_qualified_identifier signature)
    - .planning/phases/64-fix-quoted-identifier-handling/64-RESEARCH.md §Pitfall 4 (full text — explains why `.contains('.')` is wrong)
  </read_first>
  <behavior>
    Current code (around src/expand/resolution.rs:47):
      if table.contains('.') {
          return quote_table_ref(table);
      }
      // ... fallthrough: prepend db_name + schema_name from `def`

    Problem: `table = "\"a.b\""` (one quoted part containing a literal `.`) trips `.contains('.')` even though it's a SINGLE-part identifier. Then `quote_table_ref` re-parses it (correctly) as one part — net behaviour wrong: the function declines to prepend db/schema even though the user's input was a bare single-part name.

    Fix:
      let parts_result = crate::ident::parse_qualified_identifier(table);
      let is_qualified = matches!(parts_result, Ok(ref parts) if parts.len() > 1);
      if is_qualified {
          return quote_table_ref(table);
      }
      // ... existing prepend-db-and-schema logic continues

    Edge case: if `parse_qualified_identifier` returns `Err`, fall through to the existing prepend logic (treat as if it's a bare malformed name; the existing path will quote it via quote_ident and prepend db/schema if available). This matches "principle of least surprise" — `qualify_and_quote_table_ref` should never panic on input.

    Also scrub the `.contains('.')` anti-pattern from the entire production-code surface of `src/expand/` — the substring `.contains('.')` is exactly the heuristic we are replacing structurally, and any lingering production-code use is a regression waiting to happen. (Test-code uses are fine; they test fixed string literals.)

    Tests to ADD to `mod tests` in resolution.rs (create a new `mod qualify_and_quote_table_ref_tests` if one doesn't exist):

    - bare_name_gets_db_schema_prepended (sanity check — uses a SemanticViewDefinition fixture with db_name + schema_name set; assert output is "db"."schema"."t")
    - quoted_bare_name_with_dot_inside_treated_as_single_part: `qualify_and_quote_table_ref("\"a.b\"", &def_with_db_schema)` should return `"db"."schema"."a.b"` (NOT `"a"."b"` and NOT `"a.b"` standalone).
    - already_qualified_two_part: `"jaffle.raw_orders"` returns `"jaffle"."raw_orders"` (no prepend).
    - already_qualified_quoted_two_part: `"\"jaffle\".\"raw_orders\""` returns `"jaffle"."raw_orders"` (no prepend, single quote pair).
    - already_qualified_three_part: `"a.b.c"` returns `"a"."b"."c"` (no prepend).
    - malformed_falls_through_to_prepend: `qualify_and_quote_table_ref("\"unterminated", &def)` doesn't panic; returns SOMETHING (assert non-empty); document exact behaviour by reading the implementation.

    Construct a minimal `SemanticViewDefinition` fixture for the tests — the existing tests in `resolution.rs` should already have one or a builder. Look around the `mod tests` block.
  </behavior>
  <action>
    1. Open `src/expand/resolution.rs` and locate `qualify_and_quote_table_ref` (around line 47).

    2. Replace the `if table.contains('.')` line + brace with the parts-based structural test described in <behavior>. Preserve the rest of the function body (the db_name/schema_name prepend logic).

    3. Add a small doc-comment explaining: `// We use structural part-count from the identifier parser rather than a raw `.contains('.')`, so quoted parts that contain a literal `.` (e.g. `"a.b"`) are correctly recognised as single-part bare names that should receive the db/schema prefix.`

    4. Scrub-check: run `grep -rn "\\.contains('\\.')" src/expand/` and inspect every match. Any match in production code (i.e. NOT inside `#[cfg(test)]` or `mod tests`) must be rewritten to the structural parts-based check. The expectation post-fix is: zero production-code matches in `src/expand/`.

    5. Find or construct a `SemanticViewDefinition` test fixture builder. If existing tests at `src/expand/resolution.rs:200+` already build one, copy that pattern. If not, build a minimal one inline.

    6. Add the six new tests under <behavior>. If a `qualify_and_quote_table_ref_tests` submodule doesn't yet exist, create it next to `quote_table_ref_tests`.

    7. Run `cargo test --lib expand::resolution::tests` — verify all green.

    8. Run `cargo test --lib` — verify no regressions in the broader expansion path (this function is hot in expansion; check fact / join expansion tests pass).
  </action>
  <verify>
    <automated>cargo test --lib expand:: -- --nocapture 2>&1 | tail -30</automated>
  </verify>
  <acceptance_criteria>
    - `grep -n "parts.len()" src/expand/resolution.rs` returns at least one match inside `qualify_and_quote_table_ref`.
    - `grep -nE "fn quoted_bare_name_with_dot_inside_treated_as_single_part|fn already_qualified_quoted_two_part|fn malformed_falls_through_to_prepend" src/expand/resolution.rs` returns at least 3 matches.
    - `grep -n "table.contains('.')" src/expand/resolution.rs` returns 0 matches (the exact original anti-pattern is gone).
    - `grep -n "contains('\\.')" src/expand/resolution.rs` returns 0 matches (catches variable-name variants of the anti-pattern in this file — e.g. `name.contains('.')`, `s.contains('.')`).
    - `grep -rn "\\.contains('\\.')" src/expand/` returns no matches outside `#[cfg(test)]` / `mod tests` blocks. Verify by running the grep and visually inspecting any hits — every hit must be inside a test module, or the task is not done. (If a non-test hit exists that genuinely cannot be rewritten — e.g. a comment — note it explicitly in the SUMMARY; expectation is zero such cases.)
    - `cargo test --lib expand::` exits 0.
    - `cargo test --lib` exits 0.
  </acceptance_criteria>
  <done>
    qualify_and_quote_table_ref uses parts.len() for structural qualification detection. The `.contains('.')` anti-pattern is fully scrubbed from `src/expand/` production code. New tests cover the dots-in-quoted-parts edge case. Cargo test suite green.
  </done>
</task>

</tasks>

<verification>
- `cargo test --lib` exits 0.
- `cargo test --lib expand::resolution::tests` exits 0 with at least 13 new test cases (10 in Task 1 + 3+ in Task 2) plus all pre-existing tests.
- `grep -c "ident::parse_qualified_identifier\|parse_qualified_identifier" src/expand/resolution.rs` >= 2 (one per modified function).
- No `table.split('.').map` pattern remains; no `table.contains('.')` remains; no production-code `.contains('.')` remains anywhere under `src/expand/`.
- File modified: exactly one (`src/expand/resolution.rs`).
- No changes to `src/parse.rs` or `src/query/table_function.rs` (that's 64-02 — Wave-2 parallel).
</verification>

<success_criteria>
- `quote_table_ref` idempotent on already-quoted input; identical behaviour on bare input.
- `qualify_and_quote_table_ref` uses structural is-qualified test.
- 13+ new unit tests cover the regression surface for QID-04.
- Full lib test suite green.
- This plan is parallel-safe with 64-02 (zero file overlap).
</success_criteria>

<output>
After completion, create `.planning/phases/64-fix-quoted-identifier-handling/64-03-SUMMARY.md` with:

- Updated signatures of `quote_table_ref` + `qualify_and_quote_table_ref` (if changed beyond body).
- Number of new tests added in each submodule.
- Confirmation that the "idempotent on already-quoted input" property holds.
- Confirmation that `grep -rn ".contains('.')" src/expand/` returns zero production-code hits post-fix.
- Any surprises encountered if a caller was relying on the old buggy behaviour (don't expect any, but record).
</output>
