---
phase: 65.1-phase-65-code-review-remediation
plan: 04
type: execute
wave: 2
depends_on:
  - 65.1-01
files_modified:
  - src/parse.rs
  - test/integration/test_drop_on_fresh_readonly_clear_error.py
autonomous: true
requirements:
  - REM-WR-03

must_haves:
  truths:
    - "`DROP SEMANTIC VIEW nonexistent` on a read-only DB whose `semantic_layer._definitions` table was never created returns the wording `semantic view 'nonexistent' does not exist` — not the lower-level `Catalog Error: Table _definitions does not exist`"
    - "`ALTER SEMANTIC VIEW nonexistent SET COMMENT 'x'` on the same DB shape returns the same wording"
    - "`existence_guard_select` emits a SQL expression with two short-circuited WHEN branches: outer guard checks information_schema.tables for `_definitions`; inner guard checks `semantic_layer._definitions.name`"
    - "Integration test `test_drop_on_fresh_readonly_clear_error.py` (populated from the Plan 01 stub) exercises both DROP and ALTER and pins the expected wording substrings"
  artifacts:
    - path: src/parse.rs
      provides: "Updated `existence_guard_select` (and symmetric `rename_collision_guard_select`) with outer information_schema guard"
      contains: "information_schema.tables"
    - path: test/integration/test_drop_on_fresh_readonly_clear_error.py
      provides: "Populated integration test (no longer a skip-stub) asserting clear error wording for DROP and ALTER on never-bootstrapped RO DB"
      contains: "semantic view 'nonexistent' does not exist"
  key_links:
    - from: src/parse.rs::existence_guard_select
      to: information_schema.tables
      via: "outer CASE WHEN short-circuits before inner SELECT touches _definitions"
      pattern: "information_schema.tables.*WHERE table_schema = 'semantic_layer'"
---

<objective>
Wrap the existing `existence_guard_select` SQL (and paired `rename_collision_guard_select`) in `src/parse.rs` with an outer CASE WHEN that checks `information_schema.tables` for `semantic_layer._definitions` BEFORE the inner SELECT touches the catalog table. SQL `CASE` expressions short-circuit in evaluation order, so the outer WHEN protects the inner from `Catalog Error: Table _definitions does not exist` on a read-only DB that was never bootstrapped.

Populate the Wave 0 integration stub `test/integration/test_drop_on_fresh_readonly_clear_error.py` with concrete assertions for DROP and ALTER on a never-bootstrapped RO DB. Both must error with substring `semantic view 'nonexistent' does not exist` (canonical wording pinned across phase20/phase34_1/phase45/65_alter_* sqllogictests).

Purpose: WR-03 finding — today the user sees `Catalog Error: Table _definitions does not exist` which is an internal implementation leak. The fix is local (SQL composition in the guard generator); no `init_catalog` rework needed per D-17.

This plan is independent of all C++ shim changes and can run in parallel with Plans 02, 03, 05, 06 in Wave 1.

Implements D-17, D-18 (WR-03).
</objective>

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

<context>
@.planning/PROJECT.md
@.planning/STATE.md
@.planning/phases/65.1-phase-65-code-review-remediation/65.1-CONTEXT.md
@.planning/phases/65.1-phase-65-code-review-remediation/65.1-RESEARCH.md
@.planning/phases/65.1-phase-65-code-review-remediation/65.1-PATTERNS.md
@src/parse.rs
@test/integration/test_readonly_load.py
@test/integration/test_concurrent_reads_per_call_conn.py
</context>

<interfaces>
<!-- Current existence_guard_select shape — src/parse.rs:2119-2126 -->

Current:
```
fn existence_guard_select(name_escaped: &str) -> String {
    format!(
        "SELECT CASE WHEN NOT EXISTS \
                   (SELECT 1 FROM semantic_layer._definitions WHERE name = '{name_escaped}') \
                THEN error('semantic view ''{name_escaped}'' does not exist') \
                ELSE TRUE END"
    )
}
```

Target (full target in PATTERNS.md lines 525-544):
```
fn existence_guard_select(name_escaped: &str) -> String {
    format!(
        "SELECT CASE \
              WHEN NOT EXISTS (SELECT 1 FROM information_schema.tables \
                                WHERE table_schema = 'semantic_layer' \
                                  AND table_name = '_definitions') \
                THEN error('semantic view ''{name_escaped}'' does not exist') \
              WHEN NOT EXISTS (SELECT 1 FROM semantic_layer._definitions \
                                WHERE name = '{name_escaped}') \
                THEN error('semantic view ''{name_escaped}'' does not exist') \
              ELSE TRUE \
            END"
    )
}
```

The paired `rename_collision_guard_select` at src/parse.rs:2134-2141 follows the same pattern — apply the symmetric outer wrap.

DROP/ALTER call sites (verified existence_guard usage):
- src/parse.rs:2162 (DROP plain) — fixes WR-03's primary case
- src/parse.rs:2184 (ALTER plain)
- src/parse.rs:2198, 2199 (ALTER RENAME)
- src/parse.rs:2275 (additional DROP path)
</interfaces>

<tasks>

<task type="auto" tdd="true">
  <name>Task 1: Populate test_drop_on_fresh_readonly_clear_error.py with concrete assertions (RED first)</name>
  <behavior>
    - On a fresh RO-reopened DB that was never bootstrapped (no `CREATE SEMANTIC VIEW` ran on the RW phase), `conn.execute("DROP SEMANTIC VIEW nonexistent")` raises an exception whose message contains the substring `semantic view 'nonexistent' does not exist`
    - On the same DB shape, `conn.execute("ALTER SEMANTIC VIEW nonexistent SET COMMENT 'x'")` raises an exception with the same substring
    - The error message does NOT contain `Catalog Error: Table` or `_definitions does not exist` (the leakage we are fixing)
    - Test runs via `uv run test/integration/test_drop_on_fresh_readonly_clear_error.py` and exits 0
    - Test must FAIL on the pre-fix codebase (i.e. before Task 2 lands) — capture this failure in the task summary
  </behavior>
  <read_first>
    - test/integration/test_concurrent_reads_per_call_conn.py (PEP-723 header, EXT_DIR/EXT_PATH helper paths, open_master fixture)
    - test/integration/test_readonly_load.py (the RW→close→RO-reopen pattern for the same project — find the test that closes the RW connection then reopens read_only=True)
    - .planning/phases/65.1-phase-65-code-review-remediation/65.1-PATTERNS.md lines 692-705 (target test shape)
    - .planning/phases/65.1-phase-65-code-review-remediation/65.1-CONTEXT.md (D-17, D-18)
    - test/integration/test_drop_on_fresh_readonly_clear_error.py (the Plan 01 stub — to replace)
  </read_first>
  <files>test/integration/test_drop_on_fresh_readonly_clear_error.py</files>
  <action>
Replace the Plan 01 skip-stub body with the real test. The test must:

1. Keep the PEP-723 header from the stub.
2. `import duckdb`, `import pytest`, `import tempfile`, `import os`.
3. Import the helpers from `test_ducklake_helpers` (`get_ext_dir`, `get_extension_path`) using the same `sys.path.insert(0, os.path.dirname(__file__))` boilerplate used in `test_concurrent_reads_per_call_conn.py`.
4. Define `def test_drop_on_fresh_readonly_clear_error():`. Body:
   a. Create a `tempfile.TemporaryDirectory()`; build `db_path = os.path.join(tmp, "test.duckdb")`.
   b. Open a RW connection at `db_path` with `config={"allow_unsigned_extensions": "true", "extension_directory": EXT_DIR}`.
   c. `FORCE INSTALL` the extension via `EXT_PATH`; `LOAD semantic_views`.
   d. Do NOT create any semantic view; `close()` the RW connection.
   e. Reopen `db_path` with `read_only=True` + same config; `LOAD semantic_views`.
   f. Assert: `pytest.raises(Exception)` around `conn.execute("DROP SEMANTIC VIEW nonexistent")` — capture the exception, assert `"semantic view 'nonexistent' does not exist"` in `str(exc)` AND assert `"_definitions" not in str(exc)`.
   g. Assert: same shape around `conn.execute("ALTER SEMANTIC VIEW nonexistent SET COMMENT 'x'")`.
   h. Add a third sub-assertion: `DROP SEMANTIC VIEW IF EXISTS nonexistent` on the same RO DB MUST succeed silently (no exception). This verifies that the `IF EXISTS` short-circuit path also tolerates the never-bootstrapped state. W-06 fix per plan-checker: this is REQUIRED, not optional. If the existing `IF EXISTS` rewrite path bypasses `existence_guard_select` entirely (no SQL emitted that touches `_definitions`), the assertion still holds naturally — the test pins the behaviour either way. If during implementation the assertion fails because `IF EXISTS` routes through `existence_guard_select` without a soft-fail branch, Task 2 must extend the guard to include an IF-EXISTS-aware variant. DO NOT skip this assertion.
5. The test must call `pytest.main([__file__, "-v"])` from `if __name__ == "__main__":` so `uv run` works.

RED ASSERTION: Run the test BEFORE Task 2 lands and confirm it fails with the leaked `Catalog Error: Table _definitions does not exist` wording. Capture the failure output in the eventual SUMMARY for that task.
  </action>
  <verify>
    <automated>uv run test/integration/test_drop_on_fresh_readonly_clear_error.py &gt; /tmp/claude/p04_red.log 2>&amp;1; RC=$?; tail -40 /tmp/claude/p04_red.log; if [ $RC -ne 0 ]; then echo "RED phase expected — Task 2 will turn this green"; else echo "WARN: test passes pre-fix; outer-CASE wrap may already be in effect"; fi; true</automated>
  </verify>
  <acceptance_criteria>
    - `test/integration/test_drop_on_fresh_readonly_clear_error.py` no longer contains `pytest.skip(...)` for the main test body
    - File contains the PEP-723 header from Plan 01
    - One top-level `def test_drop_on_fresh_readonly_clear_error():` function
    - Function asserts on substring `semantic view 'nonexistent' does not exist`
    - Function asserts the absence of `_definitions` from the error message
    - Both DROP and ALTER are exercised
    - `DROP SEMANTIC VIEW IF EXISTS nonexistent` succeeds silently on the never-bootstrapped RO DB (W-06 pinned assertion)
    - Running pre-fix: test fails with leaked Catalog Error (RED proven)
  </acceptance_criteria>
  <done>Test populated and fails on pre-fix codebase, demonstrating WR-03 reproduction.</done>
</task>

<task type="auto">
  <name>Task 2: Update existence_guard_select + rename_collision_guard_select in src/parse.rs (GREEN)</name>
  <read_first>
    - src/parse.rs (full file or at least lines 2100-2300 covering existence_guard_select, rename_collision_guard_select, and all DROP/ALTER call sites)
    - .planning/phases/65.1-phase-65-code-review-remediation/65.1-PATTERNS.md lines 508-548 (target shape + symmetry rule for rename_collision_guard_select)
    - .planning/phases/65.1-phase-65-code-review-remediation/65.1-RESEARCH.md lines 602-645 (call-site enumeration + composition check)
  </read_first>
  <files>src/parse.rs</files>
  <action>
1. Replace `existence_guard_select` body (lines ~2119-2126) with the target shape from `<interfaces>` above. Add a comment block explaining the short-circuit guarantee: "CASE expressions short-circuit in evaluation order, so the first WHEN protects the second from `Catalog Error: Table _definitions does not exist` on a read-only DB that was never bootstrapped. Both branches emit the same canonical 'does not exist' wording pinned by phase20/phase34_1/phase45/65_alter_* sqllogictests — do not change the message text without auditing those tests."

2. Apply the symmetric outer wrap to `rename_collision_guard_select` (lines ~2134-2141): outer WHEN checks `information_schema.tables` for `_definitions`; inner check follows whatever the existing collision logic is. This is the hygiene fix per researcher recommendation — it's not strictly needed for reachability (rename collision check always runs AFTER existence guard has succeeded), but keeps the pair symmetric and prevents a future caller from invoking `rename_collision_guard_select` without `existence_guard_select` first.

3. DO NOT change any DROP/ALTER rewrite SQL composition (the two-statement guard shape from Phase 60 is preserved). The outer CASE wraps the inner SELECT; the subsequent `DELETE/UPDATE ... RETURNING` follows unchanged via the existing `;` statement separator.

4. DO NOT change the error message wording — substring `"semantic view 'X' does not exist"` is pinned by multiple existing sqllogictests AND by the new integration test from Task 1. Verify by `grep -rE "semantic view '.*' does not exist" test/sql/` before editing to see the pinning surface.

5. After the source change, re-run the integration test from Task 1 — it must now pass.

6. Run the existing `just test-sql` to confirm no sqllogictest regresses on the new SQL shape. The outer-CASE wrap should be transparent to all existing tests since `_definitions` already exists in their setup.

7. **W-06 IF EXISTS coverage (required by plan-checker)**: Task 1 sub-step (h) pins that `DROP SEMANTIC VIEW IF EXISTS nonexistent` on a never-bootstrapped RO DB must succeed silently. Two implementation options for keeping that assertion green:
   - **Option A (preferred if it already works)**: The existing `IF EXISTS` rewrite path bypasses `existence_guard_select` entirely (no SQL touches `_definitions`). Verify by grepping `src/parse.rs` for `IF EXISTS` and the DROP rewrite around lines ~2160-2275 — if the IF-EXISTS path emits `DELETE FROM semantic_layer._definitions WHERE name = 'X'` with no guard, AND that DELETE is itself soft on missing table, no change is needed. Confirm by running Task 1's test before any extra change.
   - **Option B (if Option A is not the case)**: Extend `existence_guard_select` to accept an `if_exists: bool` parameter. When true, the inner `THEN error(...)` branch becomes `THEN TRUE` (treat missing as a no-op for both branches: missing `_definitions` table AND missing row). Update DROP/ALTER call sites that pass `IF EXISTS` to use the new variant. The Task 1 integration assertion remains the contract guard.
   
   Do NOT defer or skip the IF EXISTS assertion. If Option B is required, expand acceptance criteria below to include the new param.
  </action>
  <verify>
    <automated>cargo build &gt; /tmp/claude/p04_build.log 2>&amp;1; RC=$?; tail -40 /tmp/claude/p04_build.log; test $RC -eq 0 &amp;&amp; just build &gt; /tmp/claude/p04_jbuild.log 2>&amp;1; RC2=$?; tail -30 /tmp/claude/p04_jbuild.log; test $RC2 -eq 0 &amp;&amp; just test-sql &gt; /tmp/claude/p04_sql.log 2>&amp;1; RC3=$?; tail -40 /tmp/claude/p04_sql.log; test $RC3 -eq 0 &amp;&amp; uv run test/integration/test_drop_on_fresh_readonly_clear_error.py &gt; /tmp/claude/p04_green.log 2>&amp;1; RC4=$?; tail -30 /tmp/claude/p04_green.log; test $RC4 -eq 0</automated>
  </verify>
  <acceptance_criteria>
    - `grep -nE "information_schema.tables" src/parse.rs` returns at least 2 matches (one per modified function)
    - `existence_guard_select` body contains both an outer WHEN against `information_schema.tables` and an inner WHEN against `semantic_layer._definitions`
    - `rename_collision_guard_select` carries the same outer wrap (hygiene)
    - Error message wording is exactly `semantic view 'X' does not exist` (unchanged)
    - `just build` exits 0
    - `just test-sql` exits 0 — no existing sqllogictest regresses
    - `uv run test/integration/test_drop_on_fresh_readonly_clear_error.py` exits 0 — DROP + ALTER on never-bootstrapped RO DB return clear wording
  </acceptance_criteria>
  <done>WR-03 resolved: clear error wording on fresh RO DROP/ALTER; integration test green; no sqllogictest regression.</done>
</task>

</tasks>

<threat_model>
## Trust Boundaries

| Boundary | Description |
|----------|-------------|
| User DDL ↔ catalog persistence | DROP/ALTER on read-only DB without `_definitions` table |

## STRIDE Threat Register

| Threat ID | Category | Component | Disposition | Mitigation Plan |
|-----------|----------|-----------|-------------|-----------------|
| T-65.1-04-01 | Information disclosure | Internal `_definitions` table name leaked in error message | mitigate | Outer-CASE wrap surfaces the canonical `semantic view 'X' does not exist` wording instead of the lower-level Catalog Error (D-17) |
| T-65.1-04-02 | Repudiation | Confusing error wording could lead the user to believe DDL is broken vs. simply unbootstrapped | mitigate | Wording is the same canonical phrase used everywhere else in the project, pinned by multiple sqllogictests |
</threat_model>

<verification>
  <automated>just test-all &gt; /tmp/claude/p04_all.log 2>&amp;1; RC=$?; tail -100 /tmp/claude/p04_all.log; test $RC -eq 0</automated>
</verification>

<success_criteria>
- `existence_guard_select` carries the outer information_schema guard
- `rename_collision_guard_select` symmetric outer wrap applied
- Error wording unchanged (canonical `semantic view 'X' does not exist`)
- New integration test `test_drop_on_fresh_readonly_clear_error.py` populated, RED→GREEN
- `just test-all` green
</success_criteria>

<output>
Create `.planning/phases/65.1-phase-65-code-review-remediation/65.1-04-SUMMARY.md` when done.
</output>
