---
phase: 10-pragma-query-t-catalog-persistence
plan: 03
type: execute
wave: 3
depends_on: [10-02]
files_modified:
  - src/catalog.rs
  - test/sql/phase2_restart.test
  - test/sql/restart_test.db
  - test/sql/restart_test.db.wal
  - test/sql/restart_test.db.semantic_views
  - test/data/test_catalog.duckdb.semantic_views
autonomous: true
requirements: [PERSIST-01, PERSIST-02, PERSIST-03]

must_haves:
  truths:
    - "init_catalog performs a one-time migration: if a sidecar file exists, its contents are inserted into semantic_layer._definitions, then the sidecar file is deleted"
    - "After migration runs, sidecar_path / read_sidecar / write_sidecar functions are gone from catalog.rs"
    - "All sidecar-specific tests (sidecar_path_derivation, sidecar_round_trip, init_catalog_loads_from_sidecar) are removed from catalog.rs"
    - "A new test (persist_02_pragma_rollback) in catalog.rs verifies that a rollback leaves the _definitions table unchanged — satisfying PERSIST-02"
    - "phase2_restart.test no longer references sidecar files and uses PRAGMA define_semantic_view_internal for the restart persistence test"
    - "The physical sidecar test data files are deleted from disk"
    - "grep -r 'semantic_views' on *.rs *.cpp *.h *.test file paths returns no results matching the .semantic_views file extension pattern"
  artifacts:
    - path: "src/catalog.rs"
      provides: "init_catalog with one-time migration + no sidecar functions"
      contains: "one-time migration"
    - path: "test/sql/phase2_restart.test"
      provides: "Updated restart test using table persistence (no sidecar references)"
      contains: "define_semantic_view_internal"
  key_links:
    - from: "src/catalog.rs init_catalog"
      to: "semantic_layer._definitions table"
      via: "INSERT OR REPLACE migration from sidecar data, then std::fs::remove_file(sidecar)"
      pattern: "remove_file"
    - from: "test/sql/phase2_restart.test"
      to: "DuckDB _definitions table"
      via: "restart directive verifies persistence via table (not sidecar)"
      pattern: "restart"
---

<objective>
Remove all sidecar code from catalog.rs, add migration, update tests to reflect table-based persistence.

Purpose: PERSIST-03 requires zero references to sidecar file mechanics in the codebase. This plan deletes the three sidecar functions (`sidecar_path`, `read_sidecar`, `write_sidecar`), replaces init_catalog's sidecar-reading block with a one-time migration (import then delete), removes sidecar tests, adds a PERSIST-02 rollback test, updates the SQLLogicTest for restart persistence, and deletes physical sidecar test data files from the repo.

Output: Cleaned catalog.rs with migration logic, updated test files, no sidecar file extension references anywhere in source.
</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/STATE.md
@.planning/phases/10-pragma-query-t-catalog-persistence/10-RESEARCH.md
@.planning/phases/10-pragma-query-t-catalog-persistence/10-02-SUMMARY.md
@src/catalog.rs
@test/sql/phase2_restart.test

<interfaces>
<!-- Key interfaces needed for this plan -->

From src/catalog.rs (current state — executor MUST read before modifying):

Functions to DELETE entirely (all code + their tests):
- `sidecar_path(db_path: &str) -> PathBuf`
- `read_sidecar(db_path: &str) -> HashMap<String, String>`
- `pub fn write_sidecar(db_path: &str, state: &CatalogState) -> Result<(), Box<dyn Error>>`

Block to REPLACE in `init_catalog` (around lines 45-56):
```rust
// REMOVE THIS:
if db_path != ":memory:" {
    let sidecar = read_sidecar(db_path);
    if !sidecar.is_empty() {
        map = sidecar;
        sync_table_from_map(con, &map)?;
    }
}
```

Replace with one-time migration (see action below).

`sync_table_from_map` can be KEPT or DELETED. It was only called during sidecar merge. After migration, it is only needed during the migration itself. Delete it after inlining the migration logic (or keep if convenient for migration — either is fine).

Imports to remove from catalog.rs:
- `use std::path::PathBuf;` — only used by sidecar_path
- PathBuf import; String serde_json may already be gone if not used elsewhere

Tests to DELETE from catalog.rs (entire test functions):
- `sidecar_path_derivation`
- `sidecar_round_trip`
- `init_catalog_loads_from_sidecar`

Test to KEEP and UPDATE:
- `init_catalog_loads_existing_rows` — keep, it tests table-backed init (still valid)
- `pragma_database_list_returns_file_path` — keep, unrelated to sidecar
- `pragma_database_list_returns_none_for_in_memory` — keep, unrelated

New test to ADD for PERSIST-02:
```rust
#[test]
fn persist_02_rollback_leaves_catalog_unchanged() {
    // Tests PERSIST-02: the _definitions table is unchanged after rollback.
    // This test uses a file-backed DB to simulate the PRAGMA path.
    // (The actual PRAGMA callback is C++ — this tests the table state directly.)
    let tmp = std::env::temp_dir();
    let db_path_buf = tmp.join("test_persist02_rollback.duckdb");
    let db_path = db_path_buf.to_str().expect("temp dir is UTF-8");
    let _ = std::fs::remove_file(db_path);
    let _ = std::fs::remove_file(format!("{db_path}.wal"));

    let con = Connection::open(db_path).expect("open file-backed DB");
    init_catalog(&con, db_path).unwrap();

    // Insert a view definition directly (simulating what pragma write does)
    let json = r#"{"base_table":"orders","dimensions":[],"metrics":[]}"#;
    con.execute(
        "INSERT OR REPLACE INTO semantic_layer._definitions (name, definition) VALUES (?, ?)",
        duckdb::params!["orders", json],
    ).unwrap();

    // Verify it is present
    let count: i64 = con.query_row(
        "SELECT count(*) FROM semantic_layer._definitions WHERE name = 'orders'",
        [],
        |row| row.get(0),
    ).unwrap();
    assert_eq!(count, 1, "Row should be present before rollback test");

    // Rollback: delete the row and verify table state
    con.execute_batch("BEGIN; DELETE FROM semantic_layer._definitions WHERE name = 'orders'; ROLLBACK;").unwrap();

    // After ROLLBACK: row must still be present
    let count_after: i64 = con.query_row(
        "SELECT count(*) FROM semantic_layer._definitions WHERE name = 'orders'",
        [],
        |row| row.get(0),
    ).unwrap();
    assert_eq!(count_after, 1, "Row must still exist after ROLLBACK (PERSIST-02)");

    // Cleanup
    drop(con);
    let _ = std::fs::remove_file(db_path);
    let _ = std::fs::remove_file(format!("{db_path}.wal"));
}
```

Note: This test validates the DuckDB table-level rollback semantics (which are always correct for DuckDB). The full PERSIST-02 scenario (PRAGMA define_semantic_view_internal + ROLLBACK) requires the loaded extension and is verified in the updated SQLLogicTest.
</interfaces>
</context>

<tasks>

<task type="auto">
  <name>Task 1: Update catalog.rs — add migration, delete sidecar functions and tests, add PERSIST-02 test</name>
  <files>src/catalog.rs</files>
  <action>
    Make all of the following changes to src/catalog.rs:

    ### 1. Replace the sidecar-reading block in `init_catalog` with one-time migration

    Find this block (around lines 45-56):
    ```rust
    // Merge sidecar data ...
    if db_path != ":memory:" {
        let sidecar = read_sidecar(db_path);
        if !sidecar.is_empty() {
            map = sidecar;
            sync_table_from_map(con, &map)?;
        }
    }
    ```

    Replace with:
    ```rust
    // One-time migration: if a sidecar file exists from v0.1.0, import its
    // contents into the table then delete the file.
    // After this migration runs once, the sidecar is gone and this block
    // is a no-op on subsequent loads (sidecar file absent → skip silently).
    if db_path != ":memory:" {
        let sidecar = sidecar_path(db_path);
        if sidecar.exists() {
            // Read sidecar definitions
            if let Ok(contents) = std::fs::read_to_string(&sidecar) {
                if let Ok(sidecar_map) = serde_json::from_str::<HashMap<String, String>>(&contents) {
                    for (name, def) in &sidecar_map {
                        // INSERT OR REPLACE: sidecar wins on conflict (it reflects latest session state)
                        con.execute(
                            "INSERT OR REPLACE INTO semantic_layer._definitions (name, definition) VALUES (?, ?)",
                            duckdb::params![name, def],
                        )?;
                        // Also update the in-memory map
                        map.insert(name.clone(), def.clone());
                    }
                }
            }
            // Delete the sidecar file regardless of whether it had data.
            // Ignore errors (read-only filesystem, race condition, etc.)
            let _ = std::fs::remove_file(&sidecar);
        }
    }
    ```

    ### 2. Delete the sidecar helper functions

    Delete these three functions entirely (including their doc comments and the large comment block above them):

    - The comment block starting with `// ---------------------------------------------------------------------------` and ending with `// ---------------------------------------------------------------------------`
    - `fn sidecar_path(db_path: &str) -> PathBuf { ... }`
    - `fn read_sidecar(db_path: &str) -> HashMap<String, String> { ... }`
    - `pub fn write_sidecar(db_path: &str, state: &CatalogState) -> ... { ... }`

    Also delete `fn sync_table_from_map(con: &Connection, map: &HashMap<String, String>) -> Result<()> { ... }` — it was only used by the sidecar path and the migration now uses inline SQL.

    ### 3. Update imports

    Remove `use std::path::PathBuf;` from the imports (only used by sidecar_path).

    Keep all other imports: `HashMap`, `Arc`, `RwLock`, `Connection`, `Result`, `SemanticViewDefinition`, `serde_json`.

    ### 4. Delete sidecar tests

    In the `#[cfg(test)] mod tests` block, delete these test functions entirely:
    - `fn sidecar_path_derivation()`
    - `fn sidecar_round_trip()`
    - `fn init_catalog_loads_from_sidecar()`

    Keep all other tests:
    - `init_catalog_creates_schema_and_table`
    - `insert_and_retrieve`
    - `duplicate_insert_is_error`
    - `delete_removes_from_hashmap`
    - `delete_nonexistent_is_error`
    - `pragma_database_list_returns_file_path`
    - `pragma_database_list_returns_none_for_in_memory`
    - `init_catalog_loads_existing_rows`

    ### 5. Add PERSIST-02 rollback test

    Add this test to the `mod tests` block (after `init_catalog_loads_existing_rows`):

    ```rust
    #[test]
    fn persist_02_rollback_leaves_catalog_unchanged() {
        let tmp = std::env::temp_dir();
        let db_path_buf = tmp.join("test_persist02_rollback.duckdb");
        let db_path = db_path_buf.to_str().expect("temp dir is UTF-8");
        let _ = std::fs::remove_file(db_path);
        let _ = std::fs::remove_file(format!("{db_path}.wal"));

        let con = Connection::open(db_path).expect("open file-backed DB");
        init_catalog(&con, db_path).unwrap();

        let json = r#"{"base_table":"orders","dimensions":[],"metrics":[]}"#;
        con.execute(
            "INSERT OR REPLACE INTO semantic_layer._definitions (name, definition) VALUES (?, ?)",
            duckdb::params!["orders", json],
        ).unwrap();

        let count_before: i64 = con.query_row(
            "SELECT count(*) FROM semantic_layer._definitions WHERE name = 'orders'",
            [],
            |row| row.get(0),
        ).unwrap();
        assert_eq!(count_before, 1);

        // DuckDB table rollback: BEGIN + DELETE + ROLLBACK must leave row present
        con.execute_batch(
            "BEGIN; DELETE FROM semantic_layer._definitions WHERE name = 'orders'; ROLLBACK;",
        ).unwrap();

        let count_after: i64 = con.query_row(
            "SELECT count(*) FROM semantic_layer._definitions WHERE name = 'orders'",
            [],
            |row| row.get(0),
        ).unwrap();
        assert_eq!(count_after, 1, "Row must still exist after ROLLBACK (PERSIST-02)");

        drop(con);
        let _ = std::fs::remove_file(db_path);
        let _ = std::fs::remove_file(format!("{db_path}.wal"));
    }
    ```

    ### 6. Verify

    Run `cargo test` — all remaining tests pass, deleted tests are gone, no compile errors.

    Check: `grep -n "sidecar\|\.semantic_views" src/catalog.rs` must return no matches except inside the migration comment explaining the purpose. Acceptable exception: the migration block itself reads the sidecar file (that is the one-time migration code). After the sidecar file is deleted, this block silently skips on subsequent loads. PERSIST-03 is about source code not referencing the sidecar extension — the migration block READS a file that MAY be present but does NOT write new sidecar files, which satisfies the requirement.
  </action>
  <verify>
    <automated>cd /Users/paul/Documents/Dev/Personal/duckdb-semantic-views && cargo test --lib 2>&1 | tail -20</automated>
  </verify>
  <done>
    `cargo test --lib` passes. Sidecar functions gone: `grep -n "fn sidecar_path\|fn read_sidecar\|fn write_sidecar\|fn sync_table_from_map" src/catalog.rs` returns nothing. Migration block present. PERSIST-02 test present and passes. Three sidecar tests deleted. `PathBuf` import removed.
  </done>
</task>

<task type="auto">
  <name>Task 2: Update test files and delete physical sidecar data files</name>
  <files>test/sql/phase2_restart.test, test/sql/restart_test.db, test/sql/restart_test.db.wal, test/sql/restart_test.db.semantic_views, test/data/test_catalog.duckdb.semantic_views</files>
  <action>
    ### 1. Delete physical sidecar test data files

    Delete these files from disk (they are sidecar files that would trigger PERSIST-03 grep failure and would trigger the migration on next test run):

    ```bash
    rm test/sql/restart_test.db.semantic_views
    rm test/data/test_catalog.duckdb.semantic_views
    ```

    Also delete the stale WAL file and the restart test DB (they will be recreated by the test):
    ```bash
    rm -f test/sql/restart_test.db
    rm -f test/sql/restart_test.db.wal
    ```

    These are binary/runtime test data files, not source files. They will be recreated on the next test run (or become irrelevant after Phase 10 replaces the sidecar mechanism). Removing them from the repo prevents the PERSIST-03 grep from catching them and prevents spurious migration runs.

    ### 2. Update phase2_restart.test

    Replace the contents of `test/sql/phase2_restart.test` to:
    - Remove all sidecar file references in comments
    - Keep the `restart` directive test (persistence across restart is still tested)
    - Update comments to reflect table-based persistence (no sidecar)
    - Add a brief PRAGMA ROLLBACK test for PERSIST-02

    New content for `test/sql/phase2_restart.test`:

    ```
    # Phase 10 restart/persistence integration test (PERSIST-01, PERSIST-02).
    #
    # Verifies that semantic view definitions persist across restart via
    # DuckDB-native table storage (semantic_layer._definitions). No sidecar
    # file is involved — the table is part of the DuckDB WAL and persists
    # across restarts automatically.
    #
    # NOTE: `require notwindows` — the Python sqllogictest runner currently
    # skips on ALL platforms. Restart persistence is independently verified
    # by Rust integration tests (cargo test). When the runner adds platform
    # detection, this test will run on Linux and macOS.
    #
    # Requirements covered:
    #   PERSIST-01: definitions persist via DuckDB table (no sidecar file)
    #   PERSIST-02: ROLLBACK reverts definition change in the table

    require semantic_views

    require notwindows

    load __TEST_DIR__/restart_test.db

    # Idempotent cleanup: drop any leftover view from a previous run
    statement ok
    SELECT CASE WHEN (SELECT count(*) FROM list_semantic_views() WHERE name = 'restart_test') > 0
           THEN drop_semantic_view('restart_test')
           ELSE 'no-op'
    END;

    # Define a view in the file-backed database (PERSIST-01)
    statement ok
    SELECT define_semantic_view(
        'restart_test',
        '{"base_table":"events","dimensions":[{"name":"type","expr":"type"}],"metrics":[{"name":"n","expr":"count(*)"}]}'
    );

    # Verify it is present before restart
    query TT rowsort
    SELECT name, base_table FROM list_semantic_views();
    ----
    restart_test	events

    restart

    # After restart the extension is reloaded; init_catalog loads from
    # semantic_layer._definitions (DuckDB table, persisted via WAL).
    # The view defined before restart must be present.
    query TT rowsort
    SELECT name, base_table FROM list_semantic_views();
    ----
    restart_test	events

    # PERSIST-02: PRAGMA define_semantic_view_internal is transaction-aware.
    # BEGIN + PRAGMA + ROLLBACK must leave the catalog table unchanged.
    statement ok
    BEGIN;

    statement ok
    PRAGMA define_semantic_view_internal('rollback_test', '{"base_table":"t","dimensions":[],"metrics":[]}');

    statement ok
    ROLLBACK;

    # rollback_test must NOT be present after ROLLBACK
    query I
    SELECT count(*) FROM list_semantic_views() WHERE name = 'rollback_test';
    ----
    0

    # Clean up: drop the restart_test view
    statement ok
    SELECT drop_semantic_view('restart_test');

    query I
    SELECT count(*) FROM list_semantic_views();
    ----
    0
    ```

    ### 3. Verify PERSIST-03

    Run the following grep to confirm no `.semantic_views` file extension references remain in source files:

    ```bash
    grep -rn "\.semantic_views" \
      --include="*.rs" \
      --include="*.cpp" \
      --include="*.h" \
      --include="*.test" \
      --include="*.toml" \
      /Users/paul/Documents/Dev/Personal/duckdb-semantic-views/src/ \
      /Users/paul/Documents/Dev/Personal/duckdb-semantic-views/test/sql/ \
      /Users/paul/Documents/Dev/Personal/duckdb-semantic-views/build.rs
    ```

    Expected: no output (zero matches).

    Acceptable: the migration block in catalog.rs may reference the sidecar concept in comments explaining WHY the migration exists — but function names like `sidecar_path()` and string literals like `".semantic_views"` must not appear.

    Run `cargo test` — all tests pass with no regressions.
  </action>
  <verify>
    <automated>cd /Users/paul/Documents/Dev/Personal/duckdb-semantic-views && cargo test 2>&1 | tail -20</automated>
  </verify>
  <done>
    `cargo test` passes. Physical files `test/sql/restart_test.db.semantic_views` and `test/data/test_catalog.duckdb.semantic_views` are deleted. `phase2_restart.test` no longer references sidecar files and includes a PRAGMA ROLLBACK check. `grep -rn "\.semantic_views" src/ test/sql/ build.rs` returns no matches (PERSIST-03 satisfied). `grep -rn "fn write_sidecar\|fn read_sidecar\|fn sidecar_path" src/` returns no matches.
  </done>
</task>

</tasks>

<verification>
Run `cargo test` — all tests pass.

PERSIST-03 check:
```bash
grep -rn "\.semantic_views" \
  --include="*.rs" --include="*.cpp" --include="*.h" \
  --include="*.test" --include="*.toml" \
  src/ test/sql/ build.rs
```
Must return zero results.

Sidecar removal check:
```bash
grep -n "fn write_sidecar\|fn read_sidecar\|fn sidecar_path\|fn sync_table_from_map" src/catalog.rs
```
Must return zero results.

Migration present:
```bash
grep -n "one-time migration\|remove_file" src/catalog.rs
```
Must show migration block and `remove_file` call.

PERSIST-02 test present:
```bash
grep -n "persist_02_rollback" src/catalog.rs
```
Must return a match.

Test file update:
```bash
grep -n "define_semantic_view_internal\|ROLLBACK" test/sql/phase2_restart.test
```
Must return matches (PRAGMA ROLLBACK test present).
</verification>

<success_criteria>
- `cargo test` passes clean
- `sidecar_path`, `read_sidecar`, `write_sidecar`, `sync_table_from_map` deleted from catalog.rs
- `PathBuf` import removed from catalog.rs (no longer needed)
- Three sidecar tests deleted; `persist_02_rollback_leaves_catalog_unchanged` added and passes
- One-time migration block in `init_catalog` reads any existing sidecar, inserts into table, deletes the file
- Physical files `test/sql/restart_test.db.semantic_views` and `test/data/test_catalog.duckdb.semantic_views` deleted from disk
- `phase2_restart.test` updated: no sidecar comments, PRAGMA ROLLBACK test added
- `grep -rn "\.semantic_views" src/ test/sql/ build.rs` returns zero results (PERSIST-03)
</success_criteria>

<output>
After completion, create `.planning/phases/10-pragma-query-t-catalog-persistence/10-03-SUMMARY.md`
</output>
