---
phase: 21-error-location-reporting
plan: 03
type: tdd
wave: 1
depends_on: ["21-01", "21-02"]
files_modified:
  - src/parse.rs
  - test/sql/phase21_error_reporting.test
autonomous: true
gap_closure: true
requirements: [ERR-01, ERR-02, ERR-03]

must_haves:
  truths:
    - "scan_clause_keywords recognizes KEYWORD (...) syntax as clause delimiter"
    - "All unit tests use ( syntax — no := syntax remains in validation tests"
    - "All integration tests use ( syntax — no := syntax remains"
    - "CREATE OR REPLACE, CREATE IF NOT EXISTS validated and tested"
    - "Unknown keyword far from any known produces 'Expected one of' (not 'Did you mean')"
    - "Case-insensitive clause keywords: TABLES, Tables, tables all recognized"
    - "tables + metrics (no dimensions) validates successfully"
    - "relationships clause recognized as valid"
  artifacts:
    - path: "src/parse.rs"
      provides: "scan_clause_keywords with dual-delimiter recognition, full test coverage"
      contains: "starts_with('(')"
    - path: "test/sql/phase21_error_reporting.test"
      provides: "Integration tests using ( syntax throughout"
  key_links:
    - from: "scan_clause_keywords()"
      to: "validate_clauses()"
      via: "found_clauses Vec<String>"
      pattern: "scan_clause_keywords.*body.*body_offset"
---

<objective>
Fix scan_clause_keywords() delimiter gate, migrate all validation tests from obsolete := syntax to ( syntax, and close coverage gaps.

Purpose: UAT tests 2 and 6 fail because scan_clause_keywords() only recognizes := delimiters. Beyond fixing the bug, the entire test suite uses := syntax that no user will ever encounter through the DDL validator — every test must be rewritten to use ( syntax. Additionally, several validation paths have zero test coverage.

Output: Fixed delimiter gate, all validation tests migrated to ( syntax, coverage gaps closed, integration tests updated.
</objective>

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

<context>
@.planning/PROJECT.md
@.planning/ROADMAP.md
@.planning/STATE.md
@.planning/phases/21-error-location-reporting/21-01-SUMMARY.md
@.planning/phases/21-error-location-reporting/21-UAT.md
@.planning/debug/validate-clauses-tables-not-detected.md

<interfaces>
```rust
const CLAUSE_KEYWORDS: &[&str] = &["tables", "relationships", "dimensions", "metrics"];

pub struct ParseError {
    pub message: String,
    pub position: Option<usize>,
}

// BUG: Line 420 — starts_with(":=") must also accept starts_with('(')
fn scan_clause_keywords(body: &str, body_offset: usize) -> Result<Vec<String>, ParseError>;
fn suggest_clause_keyword(word: &str) -> Option<&'static str>;
pub fn validate_clauses(body: &str, body_offset: usize, _original_query: &str) -> Result<(), ParseError>;
pub fn validate_and_rewrite(query: &str) -> Result<Option<String>, ParseError>;

// DDL variants that route to validate_create_body:
// DdlKind::Create, DdlKind::CreateOrReplace, DdlKind::CreateIfNotExists
```
</interfaces>
</context>

<tasks>

<task type="auto" tdd="true">
  <name>Task 1: Fix delimiter gate and rewrite all unit tests to ( syntax</name>
  <files>src/parse.rs</files>
  <behavior>
    <!-- Migrated from := (existing tests rewritten) -->
    - Test: validate_and_rewrite on valid DDL with TABLES (...), DIMENSIONS (...) → Ok(Some(rewritten)) starting with "SELECT * FROM create_semantic_view("
    - Test: validate_and_rewrite "SELECT 1" → Ok(None) [unchanged, no := involved]
    - Test: validate_and_rewrite empty body "()" → Err mentioning tables, dimensions, metrics
    - Test: validate_and_rewrite missing TABLES (only DIMENSIONS (...)) → Err mentioning "tables"
    - Test: validate_and_rewrite tables only, no dims/metrics → Err mentioning "dimensions" or "metrics"
    - Test: validate_and_rewrite clause typo "tbles (...)" → Err with "Did you mean" + "tables", position present
    - Test: validate_and_rewrite unbalanced brackets in ( syntax → Err with "bracket"/"Unbalanced", position present
    - Test: validate_and_rewrite DROP → Ok(Some) [unchanged]
    - Test: validate_and_rewrite SHOW → Ok(Some) [unchanged]
    - Test: validate_clauses empty body → Err mentioning required clauses
    - Test: validate_clauses unknown keyword "tbles (...)" → Err with "Did you mean" + "tables", position at "tbles"
    - Test: validate_clauses missing tables (dims + metrics only) → Err mentioning "tables"
    - Test: validate_clauses missing dims and metrics (tables only) → Err
    - Test: validate_clauses valid body with ( syntax → Ok
    - Test: validate_clauses unbalanced brackets → Err with position
    - Test: parse error position for clause typo points at "tbles" in ( syntax
    - Test: parse error position with leading whitespace still correct
    - Test: parse error position structural (missing name) [unchanged]
    <!-- New coverage gap tests -->
    - Test: validate_and_rewrite CREATE OR REPLACE SEMANTIC VIEW with valid ( body → Ok(Some)
    - Test: validate_and_rewrite CREATE IF NOT EXISTS SEMANTIC VIEW with valid ( body → Ok(Some)
    - Test: validate_and_rewrite with `relationships` clause: tables + relationships + dimensions → Ok(Some)
    - Test: validate_clauses unknown keyword far from any ("foobar (...)") → Err with "Expected one of: tables, relationships, dimensions, metrics" (NOT "Did you mean")
    - Test: validate_clauses case insensitivity — body with "TABLES (...), DIMENSIONS (...)" (uppercase) → Ok
    - Test: validate_and_rewrite tables + metrics only (no dimensions) → Ok(Some) — confirms "at least one of dims or metrics" accepts metrics alone
    - Test: validate_and_rewrite DESCRIBE SEMANTIC VIEW x → Ok(Some)
    - Test: validate_and_rewrite DROP IF EXISTS SEMANTIC VIEW x → Ok(Some)
  </behavior>
  <action>
**RED phase:** First, rewrite all 12 existing unit tests that use `:=` syntax to use `(` syntax instead. These tests will fail because scan_clause_keywords doesn't recognize `(` yet. Also add the 8 new coverage gap tests. Commit RED.

**Tests to REWRITE** (change `:=` to `(` in DDL strings):

1. `test_validate_and_rewrite_success` — change `tables := [{...}], dimensions := [{...}]` to `tables ([{...}]), dimensions ([{...}])`
2. `test_validate_and_rewrite_missing_tables` — change `dimensions := [{...}]` to `dimensions ([{...}])`
3. `test_validate_and_rewrite_missing_dims_and_metrics` — change `tables := [{...}]` to `tables ([{...}])`
4. `test_validate_and_rewrite_clause_typo` — change `tbles := [{...}], dimensions := [{...}]` to `tbles ([{...}]), dimensions ([{...}])`
5. `test_validate_and_rewrite_unbalanced_brackets` — change `tables := [{...}, dimensions := [{...}]` to `tables ([{...}), dimensions ([{...}])` (note: keep the unbalanced bracket)
6. `test_validate_clauses_unknown_keyword` — change `tbles := []` to `tbles ([])`
7. `test_validate_clauses_missing_tables` — change `dimensions := [], metrics := []` to `dimensions ([]), metrics ([])`
8. `test_validate_clauses_missing_dims_and_metrics` — change `tables := []` to `tables ([])`
9. `test_validate_clauses_valid` — change `tables := [{...}], dimensions := [{...}]` to `tables ([{...}]), dimensions ([{...}])`
10. `test_validate_clauses_unbalanced_brackets` — change `tables := [{...}, dimensions := []` to `tables ([{...}), dimensions ([])`
11. `test_parse_error_position_clause_typo` — change `tbles := []` to `tbles ([])`
12. `test_parse_error_position_with_leading_whitespace` — change `tbles := []` to `tbles ([])`

**New tests to ADD** (coverage gaps):

13. `test_validate_and_rewrite_create_or_replace` — `CREATE OR REPLACE SEMANTIC VIEW sv1 (tables ([{...}]), dimensions ([{...}]))` → Ok(Some)
14. `test_validate_and_rewrite_create_if_not_exists` — `CREATE IF NOT EXISTS SEMANTIC VIEW sv1 (tables ([{...}]), dimensions ([{...}]))` → Ok(Some)
15. `test_validate_and_rewrite_relationships_clause` — `CREATE SEMANTIC VIEW sv1 (tables ([{...}]), relationships ([{...}]), dimensions ([{...}]))` → Ok(Some)
16. `test_validate_clauses_unknown_keyword_far` — body: `foobar ([])` → Err with "Expected one of: tables, relationships, dimensions, metrics" and NOT containing "Did you mean"
17. `test_validate_clauses_case_insensitive` — body: `TABLES ([{...}]), DIMENSIONS ([{...}])` → Ok (verifies to_ascii_lowercase works)
18. `test_validate_and_rewrite_tables_and_metrics_only` — `CREATE SEMANTIC VIEW sv1 (tables ([{...}]), metrics ([{...}]))` → Ok(Some) (no dimensions, still valid)
19. `test_validate_and_rewrite_describe` — `DESCRIBE SEMANTIC VIEW sv1` → Ok(Some)
20. `test_validate_and_rewrite_drop_if_exists` — `DROP IF EXISTS SEMANTIC VIEW sv1` → Ok(Some)

Commit RED tests (12 rewritten + 8 new = 20 tests that should fail or be new).

**GREEN phase:** Fix `scan_clause_keywords` at line 420 in src/parse.rs. Change:

```rust
if after_trimmed.starts_with(":=") {
```

to:

```rust
if after_trimmed.starts_with(":=") || after_trimmed.starts_with('(') {
```

Update doc comment on scan_clause_keywords (line 382) from:
`/// Scan for 'word :=' patterns at the top level of the body`
to:
`/// Scan for clause keywords at the top level of the body (recognized by ':=' or '(' after the word)`

All tests must pass. Commit GREEN.

**Important:** Do NOT touch any other logic in scan_clause_keywords — only the delimiter gate on line 420 needs the additional condition.
  </action>
  <verify>
    <automated>cd /Users/paul/Documents/Dev/Personal/duckdb-semantic-views && cargo test -- --test-threads=1 2>&1 | tail -30</automated>
  </verify>
  <done>All 12 rewritten tests pass with ( syntax. All 8 new gap tests pass. No := syntax remains in validation test section. Total ~63 unit tests green.</done>
</task>

<task type="auto">
  <name>Task 2: Migrate integration tests to ( syntax and add coverage</name>
  <files>test/sql/phase21_error_reporting.test</files>
  <action>
Rewrite ALL sqllogictest cases in `test/sql/phase21_error_reporting.test` that use `:=` syntax to use `(` syntax. Also add new integration tests for coverage gaps.

**REWRITE existing tests** (change `:=` to `(` throughout):

Section 1 (ERR-01):
- "Missing tables clause" — change `dimensions := [...]` to `dimensions ([...])`
- "Missing dimensions and metrics" — change `tables := [...]` to `tables ([...])`
- "Typo in clause name" — change `tbles := [...], dimensions := [...]` to `tbles ([...]), dimensions ([...])`

Section 3 (ERR-03):
- "Typo in DDL prefix" — change `tables := []` to `tables ([])`

Section 4 (Non-interference):
- "Valid CREATE still works" — change `tables := [...], dimensions := [...], metrics := [...]` to `tables ([...]), dimensions ([...]), metrics ([...])`

**ADD new integration tests:**

In Section 1 (after existing clause tests):
```
# Unknown keyword far from any known -- "Expected one of" message
statement error
CREATE SEMANTIC VIEW err_test (
    foobar ([]),
    tables ([{'alias': 't', 'table': 't'}])
);
----
Expected one of
```

```
# Case insensitivity -- uppercase clause keywords work
statement ok
CREATE SEMANTIC VIEW err_case_test (
    TABLES ([{'alias': 'err_source', 'table': 'err_source'}]),
    DIMENSIONS ([{'name': 'id', 'expr': 'id', 'source_table': 'err_source'}]),
    METRICS ([{'name': 'total', 'expr': 'SUM(val)', 'source_table': 'err_source'}])
);

statement ok
DROP SEMANTIC VIEW err_case_test;
```

In Section 4 (after valid CREATE):
```
# CREATE OR REPLACE works
statement ok
CREATE OR REPLACE SEMANTIC VIEW err_valid (
    tables ([{'alias': 'err_source', 'table': 'err_source'}]),
    dimensions ([{'name': 'id', 'expr': 'id', 'source_table': 'err_source'}]),
    metrics ([{'name': 'total', 'expr': 'SUM(val)', 'source_table': 'err_source'}])
);
```

```
# Tables + metrics only (no dimensions) -- valid
statement ok
CREATE SEMANTIC VIEW err_metrics_only (
    tables ([{'alias': 'err_source', 'table': 'err_source'}]),
    metrics ([{'name': 'total', 'expr': 'SUM(val)', 'source_table': 'err_source'}])
);

statement ok
DROP SEMANTIC VIEW err_metrics_only;
```

Update cleanup section to drop any new views created.

Run `just build && just test-all` to confirm full quality gate.
  </action>
  <verify>
    <automated>cd /Users/paul/Documents/Dev/Personal/duckdb-semantic-views && just build && just test-all 2>&1 | tail -30</automated>
  </verify>
  <done>All sqllogictest cases pass with ( syntax. No := syntax remains in phase21_error_reporting.test. New coverage tests (unknown keyword, case insensitivity, CREATE OR REPLACE, metrics-only) all pass. `just test-all` green.</done>
</task>

</tasks>

<verification>
Both UAT gaps traced to scan_clause_keywords line 420 `:=`-only gate.

After completion:
1. `cargo test` — all Rust unit tests pass (12 rewritten + 8 new + existing non-validation tests)
2. `just build && just test-sql` — all sqllogictest files pass (all migrated to ( syntax + new coverage tests)
3. `just test-all` — full quality gate green
4. Zero `:=` syntax remains in validation test code
5. Coverage gaps closed: CREATE OR REPLACE, CREATE IF NOT EXISTS, relationships, unknown-far keyword, case insensitivity, metrics-only, DESCRIBE, DROP IF EXISTS
</verification>

<success_criteria>
- scan_clause_keywords recognizes both `:=` and `(` as clause-keyword delimiters
- All 12 existing validation unit tests rewritten from := to ( syntax
- 8 new unit tests closing coverage gaps
- All integration tests rewritten from := to ( syntax
- New integration tests for: unknown-far keyword, case insensitivity, CREATE OR REPLACE, metrics-only
- No := syntax remains in any validation test
- `just test-all` passes completely
</success_criteria>

<output>
After completion, create `.planning/phases/21-error-location-reporting/21-03-SUMMARY.md`
</output>
