---
phase: 65.1-phase-65-code-review-remediation
plan: 11
type: execute
wave: 5
depends_on:
  - 65.1-02b
  - 65.1-03b
  - 65.1-10
  - 65.1-01
files_modified:
  - cpp/src/shim.cpp
  - src/query/table_function.rs
  - test/sql/phase651_bind_probe_hard_errors.test
autonomous: true
requirements:
  - REM-WR-07
  - REM-WR-08

must_haves:
  truths:
    - "`sv_resolve_output_logical_types` (cpp/src/shim.cpp:~2236-2242) NO LONGER falls back to type_id-only declaration on probe failure — it throws `BinderException(\"semantic_view: failed to infer column types via LIMIT 0 probe: <error>\")`"
    - "A second mismatched-cols throw covers the rare case where `probe_result->types.size() != cols.size()` — distinct error message naming both counts"
    - "`try_infer_schema` (src/query/table_function.rs:~704) returns `Result<(Vec<String>, Vec<ffi::duckdb_type>), String>` instead of `Option`; callers at ~294 and ~349 pattern-match and surface the error via `write_err` + return 1"
    - "Existing `BorrowedConnection` shape from Plan 03 is preserved (`try_infer_schema(borrowed: &BorrowedConnection, sql: &str)`)"
    - "sqllogictest `phase651_bind_probe_hard_errors.test` exercises (a) a deliberately broken FACTS expression that triggers probe failure → expects `Binder Error` block containing `failed to infer column types`; (b) a known-good `semantic_view(...)` query still succeeds"
    - "CHANGELOG `### Changed` entry from Plan 06 covers this behaviour change"
  artifacts:
    - path: cpp/src/shim.cpp
      provides: "WR-07 hard-error promotion in sv_resolve_output_logical_types"
      contains: "failed to infer column types"
    - path: src/query/table_function.rs
      provides: "try_infer_schema returns Result<_, String>; callers surface via error_buf"
      contains: "Result<(Vec<String>"
    - path: test/sql/phase651_bind_probe_hard_errors.test
      provides: "Populated WR-07/WR-08 hard-error sqllogictest"
      contains: "failed to infer column types"
  key_links:
    - from: src/query/table_function.rs::try_infer_schema
      to: cpp/src/shim.cpp::sv_semantic_view_bind (BinderException re-raise)
      via: "error_buf plumb: Rust write_err → rc=1 → C++ throw BinderException prefixed"
      pattern: "BinderException.*semantic_view"
---

<objective>
Promote two silent fallbacks to hard `BinderException` errors per CONTEXT D-14, D-15, D-16:

1. **WR-07 (D-14)** — `sv_resolve_output_logical_types` (`cpp/src/shim.cpp:~2236-2242`) currently falls back to `sv_logical_type_from_c_type_id(c.type_id)` when the LIMIT 0 probe fails or the column count mismatches. Replace with two distinct `throw BinderException(...)` paths:
   - On `probe_result->HasError()`: `throw BinderException("semantic_view: failed to infer column types via LIMIT 0 probe: " + probe_result->GetError());`
   - On count mismatch: `throw BinderException("semantic_view: LIMIT 0 probe returned <N> columns, expected <M>");`

2. **WR-08 (D-15)** — `try_infer_schema` (`src/query/table_function.rs:~704-729`) currently returns `Option<(Vec<String>, Vec<ffi::duckdb_type>)>` and swallows the underlying error via `.ok()?`. Change return type to `Result<(Vec<String>, Vec<ffi::duckdb_type>), String>` so the underlying `execute_sql_raw` error text plumbs upward. Callers at `src/query/table_function.rs:~294, ~349` pattern-match `Err(msg)` and surface via `write_err(error_buf, error_buf_len, &format!("semantic_view: type inference failed for query \`<sql>\`: {msg}")); return 1_u8;`. The downstream C++ `sv_semantic_view_bind` (`cpp/src/shim.cpp:~2257`) ALREADY re-raises rc=1 as `throw BinderException(std::string("semantic_view: ") + error_buf)` — no new C++ plumbing.

Per D-16: CHANGELOG entry under `### Changed` covers this user-visible behaviour change (Plan 06 already wrote the entry). Sqllogictest exercises both the new error surface AND confirms valid queries still pass.

Wave 3 because:
- Depends on Plan 02 (uses the WR-02 error_buf mechanism on the Rust side)
- Depends on Plan 03 (table_function.rs already migrated to `BorrowedConnection`; this plan stacks the Result change on top of the existing signature)
- Depends on Plan 10 (shim.cpp serialization — Plan 10 lands first)

Implements D-14, D-15, D-16 (WR-07/WR-08).
</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
@cpp/src/shim.cpp
@src/query/table_function.rs
@test/sql/phase651_bind_probe_hard_errors.test
</context>

<interfaces>
WR-07 target — `cpp/src/shim.cpp:~2236-2242` (PATTERNS.md lines 254-278):

Replace:
```
if (probe_result->HasError() || probe_result->types.size() != cols.size()) {
    for (const auto &c : cols) {
        out.emplace_back(sv_logical_type_from_c_type_id(c.type_id));
    }
    return out;
}
```

With:
```
if (probe_result->HasError()) {
    throw BinderException(
        "semantic_view: failed to infer column types via LIMIT 0 probe: " +
        probe_result->GetError());
}
if (probe_result->types.size() != cols.size()) {
    throw BinderException(
        "semantic_view: LIMIT 0 probe returned " +
        std::to_string(probe_result->types.size()) +
        " columns, expected " + std::to_string(cols.size()));
}
```

WR-08 target — `src/query/table_function.rs:~704-729` (PATTERNS.md lines 469-489):

Replace signature `try_infer_schema(borrowed: &BorrowedConnection, sql: &str) -> Option<(Vec<String>, Vec<ffi::duckdb_type>)>` with `Result<(Vec<String>, Vec<ffi::duckdb_type>), String>`.

Body change: `let mut result = execute_sql_raw(borrowed.as_raw(), sql).ok()?;` → `let mut result = execute_sql_raw(borrowed.as_raw(), sql).map_err(|e| e.to_string())?;`

Callers (lines ~294, ~349) — pattern-match the Result; on Err surface via write_err + return rc=1.
</interfaces>

<tasks>

<task type="auto">
  <name>Task 1: WR-07 — promote sv_resolve_output_logical_types to throw BinderException</name>
  <read_first>
    - cpp/src/shim.cpp lines 2200-2260 (sv_resolve_output_logical_types body)
    - cpp/src/shim.cpp lines 761-763, 802, 986 (existing `throw BinderException(...)` usages for tone/format consistency)
    - .planning/phases/65.1-phase-65-code-review-remediation/65.1-PATTERNS.md lines 250-278 (target shape)
    - .planning/phases/65.1-phase-65-code-review-remediation/65.1-RESEARCH.md lines 540-563
  </read_first>
  <files>cpp/src/shim.cpp</files>
  <action>
Locate `sv_resolve_output_logical_types` (lines ~2203-2255). Find the fallback block at ~2236-2242 (the `if (probe_result->HasError() || probe_result->types.size() != cols.size()) { ... fall back to type_id-only ... return out; }`).

Replace with the two distinct throw paths from `<interfaces>` above:
1. `probe_result->HasError()` → throw with prefix `"semantic_view: failed to infer column types via LIMIT 0 probe: "` + `probe_result->GetError()`
2. `probe_result->types.size() != cols.size()` → throw with both column counts: `"semantic_view: LIMIT 0 probe returned <N> columns, expected <M>"`

Use `std::to_string(...)` to stringify the size_t values. NO silent fallback path remains — the function either returns the resolved type vector OR throws.

Verify no other branch in the function still does `sv_logical_type_from_c_type_id` as a fallback for probe failure (the happy path may use it for declared types; that's unrelated and stays).

Build + test:
- `just build` + `just test-sql` should pass — the existing tests exercise the happy path (probe succeeds) so no behavioural regression. The new throws are only reached on broken queries which existing tests do not cover.
  </action>
  <verify>
    <automated>just build &gt; /tmp/claude/p11_t1_build.log 2>&amp;1; RC=$?; tail -40 /tmp/claude/p11_t1_build.log; test $RC -eq 0 &amp;&amp; grep -nE "failed to infer column types via LIMIT 0 probe" cpp/src/shim.cpp &amp;&amp; just test-sql &gt; /tmp/claude/p11_t1_sql.log 2>&amp;1; RC2=$?; tail -40 /tmp/claude/p11_t1_sql.log; test $RC2 -eq 0</automated>
  </verify>
  <acceptance_criteria>
    - `sv_resolve_output_logical_types` contains two distinct `throw BinderException(...)` paths
    - Probe-failure throw uses prefix `"semantic_view: failed to infer column types via LIMIT 0 probe: "`
    - Count-mismatch throw uses both counts (e.g. `"semantic_view: LIMIT 0 probe returned 3 columns, expected 4"`)
    - No `sv_logical_type_from_c_type_id` fallback remains inside the probe-failure branch
    - `just build` + `just test-sql` exit 0
  </acceptance_criteria>
  <done>WR-07 closed: probe failure throws BinderException with full diagnostic; no silent DECIMAL(18,3) placeholder.</done>
</task>

<task type="auto">
  <name>Task 2: WR-08 — promote try_infer_schema return type to Result + plumb errors at callers</name>
  <read_first>
    - src/query/table_function.rs (full file; specifically lines 101 sv_semantic_view_bind_rust, 280-360 first try_infer_schema caller, 348-368 second caller, 700-729 try_infer_schema body)
    - .planning/phases/65.1-phase-65-code-review-remediation/65.1-PATTERNS.md lines 465-504 (target signature + caller migration shape)
    - .planning/phases/65.1-phase-65-code-review-remediation/65.1-RESEARCH.md lines 565-597
    - src/ddl/read_ffi.rs (write_err signature — for the caller surfacing pattern)
  </read_first>
  <files>src/query/table_function.rs</files>
  <action>
1. **Change `try_infer_schema` signature**:
   ```rust
   pub(crate) unsafe fn try_infer_schema(
       borrowed: &BorrowedConnection,
       sql: &str,
   ) -> Result<(Vec<String>, Vec<ffi::duckdb_type>), String> {
       let mut result = execute_sql_raw(borrowed.as_raw(), sql)
           .map_err(|e| e.to_string())?;
       // ... existing body returns Ok((names, types)) instead of Some((names, types))
   }
   ```
   Replace every `Some(...)` return with `Ok(...)`. Replace any `None` early-exit with `Err("<diagnostic>".to_string())`.

2. **Update both callers** (line ~294 + line ~349):
   - Replace `let Some((names, types)) = try_infer_schema(&borrowed, &limit0_sql) else { /* fallback */ };` (or whatever the current pattern is) with:
     ```rust
     let (names, types) = match try_infer_schema(&borrowed, &limit0_sql) {
         Ok(pair) => pair,
         Err(msg) => {
             write_err(error_buf, error_buf_len,
                 &format!("semantic_view: type inference failed for query `{limit0_sql}`: {msg}"));
             return 1_u8;
         }
     };
     ```
   - Remove the silent VARCHAR / `DUCKDB_TYPE_INVALID` fallback path (the `vec![0u32; names.len()]` shape per RESEARCH.md line 569). Eliminate any orphan code that depended on the fallback.

3. The downstream C++ at `cpp/src/shim.cpp:~2257` (`sv_semantic_view_bind`) ALREADY re-raises rc=1 as `throw BinderException(std::string("semantic_view: ") + error_buf)` — confirm by inspecting; no C++ change needed in this task.

4. Run `cargo build` + `cargo test` — the unit tests should still pass (the signature change is API-internal). Any existing test that called `try_infer_schema` expecting `Option` must be updated to expect `Result` — fix those too.
  </action>
  <verify>
    <automated>cargo build &gt; /tmp/claude/p11_t2_build.log 2>&amp;1; RC=$?; tail -40 /tmp/claude/p11_t2_build.log; test $RC -eq 0 &amp;&amp; cargo test &gt; /tmp/claude/p11_t2_test.log 2>&amp;1; RC2=$?; tail -40 /tmp/claude/p11_t2_test.log; test $RC2 -eq 0 &amp;&amp; grep -nE "Result<\\(Vec<String>" src/query/table_function.rs</automated>
  </verify>
  <acceptance_criteria>
    - `try_infer_schema` returns `Result<(Vec<String>, Vec<ffi::duckdb_type>), String>` (verify via grep on signature)
    - Both callers pattern-match the Result; Err path calls `write_err` + `return 1_u8;`
    - No silent `vec![0u32; names.len()]` (DUCKDB_TYPE_INVALID fallback) remains for probe failure
    - The error message at the caller embeds the original SQL + the inner error: `"semantic_view: type inference failed for query \`<sql>\`: <msg>"`
    - `cargo build` and `cargo test` exit 0
  </acceptance_criteria>
  <done>WR-08 closed: try_infer_schema error text plumbs through to BinderException via existing FFI surfacing.</done>
</task>

<task type="auto">
  <name>Task 3: Populate phase651_bind_probe_hard_errors.test sqllogictest</name>
  <read_first>
    - test/sql/phase651_bind_probe_hard_errors.test (Plan 01 stub)
    - test/sql/65_pk_error.test (template for `statement error` block-form pattern + multi-statement setup style)
    - .planning/phases/65.1-phase-65-code-review-remediation/65.1-CONTEXT.md (D-16 — error surface AND existing valid path must both pass)
    - .planning/phases/65.1-phase-65-code-review-remediation/65.1-RESEARCH.md lines 540-597 (the exact error wording strings to substring-match)
  </read_first>
  <files>test/sql/phase651_bind_probe_hard_errors.test</files>
  <action>
Replace the Plan 01 stub body. The test exercises BOTH the new hard-error surface (WR-07/WR-08) AND a known-good path that must still succeed (regression guard).

Block 1 — happy path (must still pass):
```
statement ok
CREATE TABLE t1 (id INTEGER, name VARCHAR);
INSERT INTO t1 VALUES (1, 'alice'), (2, 'bob');

statement ok
CREATE SEMANTIC VIEW v_ok
  TABLES (t1 AS t1)
  DIMENSIONS (t1.name AS name)
  METRICS (t1.cnt AS count(*));

query II
SELECT name, cnt FROM semantic_view('v_ok', dimensions := ['name'], metrics := ['cnt']) ORDER BY name;
----
alice	1
bob	1
```

Block 2 — WR-07 hard-error surface. A deliberately broken FACTS / METRICS expression that triggers a bind-probe failure. Example: METRIC expression referencing a non-existent column:
```
statement ok
CREATE SEMANTIC VIEW v_broken
  TABLES (t1 AS t1)
  DIMENSIONS (t1.name AS name)
  METRICS (t1.broken AS sum(t1.nonexistent_column));

statement error
SELECT name, broken FROM semantic_view('v_broken', dimensions := ['name'], metrics := ['broken']) ORDER BY name
----
failed to infer column types
```

The substring `failed to infer column types` is the stable prefix shared by both WR-07 (C++ side) and WR-08 (Rust side surfacing). DO NOT pin to specific column or table names that may vary in the underlying DuckDB error wording.

Block 3 — WR-08 surface (Rust side). If a different failure mode triggers `try_infer_schema` to return `Err` (e.g. a CREATE that succeeds but whose expanded SQL has a missing-table error), exercise that too. If not easily distinguishable from Block 2 at the SQL surface, add `# NOTE: WR-08 plumb-up surface uses the same "type inference failed" wording family; covered by Block 2 via the FFI re-raise path.`

The test MUST end with both `DROP SEMANTIC VIEW v_ok;` and `DROP SEMANTIC VIEW v_broken;` to leave a clean state (if any subsequent test file depends on a clean catalog).
  </action>
  <verify>
    <automated>just test-sql &gt; /tmp/claude/p11_t3_sql.log 2>&amp;1; RC=$?; tail -60 /tmp/claude/p11_t3_sql.log; test $RC -eq 0 &amp;&amp; grep -q "failed to infer column types" test/sql/phase651_bind_probe_hard_errors.test</automated>
  </verify>
  <acceptance_criteria>
    - File contains at least one `statement error` block matching `failed to infer column types`
    - File contains at least one `statement ok` + `query` block exercising a valid semantic_view query that returns rows (regression guard)
    - File ends with cleanup DROPs
    - `just test-sql` exits 0
    - The error-block assertion would FAIL pre-Task-1 — verify by recording behaviour before Task 1/2 land if possible
  </acceptance_criteria>
  <done>WR-07/WR-08 surface covered by sqllogictest; regression guard for valid queries preserved.</done>
</task>

</tasks>

<threat_model>
## Trust Boundaries

| Boundary | Description |
|----------|-------------|
| LIMIT 0 probe ↔ binder column-type inference | Probe may fail for many reasons (broken expanded SQL, missing tables, permissions); silent fallback hides root cause |

## STRIDE Threat Register

| Threat ID | Category | Component | Disposition | Mitigation Plan |
|-----------|----------|-----------|-------------|-----------------|
| T-65.1-11-01 | Tampering / Repudiation | Silent wrong-type column data returned to user under probe failure | mitigate | Hard-error on probe failure (D-14, D-15); user sees the actual DuckDB error wording |
| T-65.1-11-02 | Information disclosure | Underlying SQL leaked in error message via the embedded `{limit0_sql}` | accept | The user's own DDL produced the SQL; surfacing it aids debugging. No additional secret material |
</threat_model>

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

<success_criteria>
- `sv_resolve_output_logical_types` throws on probe failure + count mismatch (no silent type_id fallback)
- `try_infer_schema` returns `Result<_, String>`; callers surface via write_err + rc=1
- C++ binder re-raise unchanged (existing surface)
- sqllogictest pins both the new error surface AND a valid-query regression guard
- `just test-all` green
</success_criteria>

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