---
phase: 65.1-phase-65-code-review-remediation
plan: 09
subsystem: cleanup
tags: [rust, dead-code-purge, sqllogictest, null-handling, in-02, in-03, in-05, d-23, d-25]

# Dependency graph
requires:
  - phase: 65.1-phase-65-code-review-remediation
    provides: "65.1-01 — phase651_null_name_inputs.test stub created and wired into TEST_LIST"
  - phase: 65.1-phase-65-code-review-remediation
    provides: "65.1-03a — query/table_function.rs migrated to BorrowedConnection (wrap-on-entry)"
  - phase: 65.1-phase-65-code-review-remediation
    provides: "65.1-03b — sv_semantic_view_bind_rust + try_infer_schema migrated to BorrowedConnection; same file as type_id_to_display_name deletion target"
provides:
  - "IN-02 closed (D-23): src/type_cache.rs module deleted; lib.rs declaration removed"
  - "IN-03 closed (D-23): type_id_to_display_name function deleted from src/query/table_function.rs"
  - "IN-05 closed (D-25): phase651_null_name_inputs.test populated with three null-name short-circuit assertions"
  - "317 LOC purged (deletion-only: 224 in type_cache.rs + 82 in type_id_to_display_name + 11 across two dangling-comment cleanups)"
affects: []

# Tech tracking
tech-stack:
  added: []
  patterns:
    - "Deletion-only refactor: grep-driven consumer search BEFORE deletion confirms zero in-tree consumers; cargo build + cargo test --lib serves as the final compile-time invariant check that no consumer was missed"
    - "Comment cleanup co-located with module deletion: file-level comments that reference the deleted module are cleaned up in the same commit so a future grep for the symbol name returns 0 hits everywhere, not just in source code"

key-files:
  created: []
  modified:
    - "src/lib.rs — `pub mod type_cache;` declaration removed"
    - "src/query/table_function.rs — `type_id_to_display_name` function + leading doc-comment deleted (82 LOC); dangling file-level comment block referencing src/type_cache.rs cleaned up"
    - "src/ddl/show_dims.rs — file-level comment referencing src/type_cache.rs cleaned up (kept the data_type-is-empty-on-fresh-CREATEs explanation; dropped the orphan sentence about the now-removed module)"
    - "test/sql/phase651_null_name_inputs.test — stub body replaced with three D-25 `query I` blocks (read_yaml_from_semantic_view(NULL), get_ddl(NULL,'x'), get_ddl('SEMANTIC_VIEW',NULL))"
  deleted:
    - "src/type_cache.rs — 224 LOC unconsumed module (per BATCH2-SUMMARY a deferred optimisation; per Plan 09 D-23 a contributor trap to be removed)"

key-decisions:
  - "Treated the dangling file-level-comment references (src/ddl/show_dims.rs and src/query/table_function.rs) as part of the IN-02/IN-03 deletion rather than scope creep — leaving them would have created a stale-doc trap pointing readers at a non-existent module, defeating the D-23 'contributor trap' rationale. Pure deletion (kept the surrounding explanation about the type-inference architecture); reported in deviations as a Rule 2 auto-clean."
  - "Did not split Task 1 across the two IN findings (IN-02 module deletion + IN-03 function deletion). They are textbook deletion-only changes targeting symbols that are entirely unconsumed; rolling them into one commit minimises bisect noise and keeps the LOC purge attributable to the single D-23 decision."

patterns-established:
  - "Consumer search BEFORE deletion: `grep -rn '<symbol>' src/ tests/ benches/` enumerates ALL non-deletion references first; if grep finds anything outside the deletion sites themselves, halt and surface. This time grep returned only comment references — handled as in-commit cleanup, not as scope-blocking external consumers."

requirements-completed:
  - REM-IN-02
  - REM-IN-03
  - REM-IN-05

# Metrics
duration: ~15min
completed: 2026-05-25
---

# Phase 65.1 Plan 09: Internal Cleanup — type_cache + type_id_to_display_name purge + IN-05 null-name sqllogictest

**Three internal cleanups land in Wave 3: deletion of an unconsumed type-inference cache module (IN-02), deletion of an `#[allow(dead_code)]` helper function (IN-03), and population of a Plan-01 sqllogictest stub with three D-25 null-name short-circuit assertions (IN-05). Net 317 LOC purged with zero behaviour change to the extension surface.**

## Performance

- **Duration:** ~15 min
- **Started:** 2026-05-25T22:15:00Z (approx)
- **Completed:** 2026-05-25T22:30:00Z (approx)
- **Tasks:** 2
- **Files modified:** 4 (src/lib.rs + src/query/table_function.rs + src/ddl/show_dims.rs + test/sql/phase651_null_name_inputs.test)
- **Files deleted:** 1 (src/type_cache.rs)

## Accomplishments

- **IN-02 (D-23) — type_cache module deletion:** `src/type_cache.rs` (224 LOC) deleted in full; `pub mod type_cache;` line removed from `src/lib.rs`. Pre-deletion grep confirmed zero in-tree consumers; the module landed during Phase 65 Plan 05 as a deferred-optimisation cache that was never wired up. Per D-23 the contributor-trap risk outweighs the speculative-future-use value, so it goes.
- **IN-03 (D-23) — type_id_to_display_name deletion:** `pub(crate) fn type_id_to_display_name` (82 LOC including leading doc-comment) deleted from `src/query/table_function.rs`. The function had the `#[allow(dead_code)]` warning suppression in place — same rationale as IN-02 (kept against future use that never materialised).
- **Dangling comment cleanup:** Two file-level comment blocks (one in `src/ddl/show_dims.rs`, one in `src/query/table_function.rs`) referenced `src/type_cache.rs` as a wired-up artefact. Both blocks were trimmed to remove the stale pointer while preserving the surrounding architecture-explanation paragraphs. Result: `grep -rn 'type_cache' src/ tests/` returns zero hits.
- **IN-05 (D-25) — sqllogictest populated:** `test/sql/phase651_null_name_inputs.test` (Plan 01 stub) replaced with the three required `query I` assertions: `read_yaml_from_semantic_view(NULL) IS NULL`, `get_ddl(NULL, 'x') IS NULL`, `get_ddl('SEMANTIC_VIEW', NULL) IS NULL`. All three return `true` — the underlying scalar functions already short-circuit null inputs (the test confirms current behaviour rather than driving a fix).
- Quality gates: `cargo build` → 0; `cargo test --lib` 840/840; `just build` → 0; `just test-sql` → 56/56 SUCCESS; `just test-all` → all green (939 Rust + 56 SQL + 6 ADBC + 6 query + 12/12 readonly_load + 3/3 multi_db_isolation + concurrent_reads + concurrent_writes + concurrent_ddl).

## Task Commits

Each task was committed atomically:

1. **Task 1: Delete type_cache module + type_id_to_display_name (IN-02 + IN-03, D-23)** — `3f1bd8e` (refactor)
2. **Task 2: Populate phase651_null_name_inputs.test (IN-05, D-25)** — `da32fec` (test)

## Files Created/Modified

See `key-files` above for the full list. Per-category summary:

- **Module deletion (1 file):** `src/type_cache.rs` (-224 LOC).
- **Module-decl removal (1 file):** `src/lib.rs` (-1 LOC).
- **Function deletion + comment cleanup (1 file):** `src/query/table_function.rs` (-94 LOC: 82 function + 12 comment).
- **Comment cleanup (1 file):** `src/ddl/show_dims.rs` (-3 LOC: trimmed file-level comment).
- **Test populated (1 file):** `test/sql/phase651_null_name_inputs.test` (-8 net LOC: stub body replaced with three `query I` blocks).

**Net LOC delta:** -317 (close to the plan's ~225 estimate; the extra ~92 LOC came from the function's full doc-comment + dangling-comment cleanups that the plan estimate did not include).

## Decisions Made

- **Dangling-comment cleanup co-located with deletion:** Rule 2 auto-clean (see Deviations). The comments at `src/ddl/show_dims.rs:13-16` and `src/query/table_function.rs:32-43` both pointed at `src/type_cache.rs` as if it were wired-up architecture. Leaving them would have created a future contributor's confusion vector — exactly the D-23 contributor-trap concern. The cleanup is in scope under "and any helper-only-used-by-this-function private items in the same range" plan wording, extended to "and any file-level doc-comment references that would become stale on deletion."
- **Single Task 1 commit covering both IN-02 + IN-03:** Both target the same decision (D-23), are pure deletions, and have no inter-dependence; splitting them would have inflated bisect noise.

## Deviations from Plan

### Auto-fixed Issues

**1. [Rule 2 — Auto-clean] File-level comments referenced `src/type_cache.rs`/`type_id_to_display_name` as if wired up**

- **Found during:** Task 1 first verification (`grep -rn 'type_cache' src/`)
- **Issue:** Plan said "if grep shows consumers, halt and surface". Grep returned 3 results:
  1. `src/lib.rs:13: pub mod type_cache;` — the declaration to delete (expected)
  2. `src/ddl/show_dims.rs:15: // the read-side type cache in src/type_cache.rs is wired up in the single-`
  3. `src/query/table_function.rs:37: //     The type_cache module (`src/type_cache.rs`) is intentionally NOT`
  Strictly the comment lines are not "consumers" — they don't cause the compiler to fail if `type_cache` goes away. But they are file-level documentation pointing at a now-absent module, which is the exact "contributor trap" footprint that D-23 is supposed to eliminate.
- **Fix:** Trimmed both comment blocks to remove the stale pointer while preserving the surrounding architecture-explanation paragraph (data_type-is-empty-on-fresh-CREATEs in show_dims.rs; CREATE-time vs read-side type-inference architecture in table_function.rs).
- **Files modified:** src/ddl/show_dims.rs, src/query/table_function.rs.
- **Verification:** `grep -rn 'type_cache' src/ tests/` returns zero hits after cleanup; `cargo build` exit 0.
- **Committed in:** `3f1bd8e` (Task 1 commit — same commit as the deletions).

**Total deviations:** 1 auto-cleaned (Rule 2 — leaving the stale doc-comments would have defeated the D-23 contributor-trap rationale).

**Impact on plan:** Net positive — the cleanup is small, fully attributable to the same D-23 decision, and avoids a future grep-and-puzzle moment for a contributor.

## Issues Encountered

- **Pre-existing sqllogictest sandbox quirk (CLAUDE.md Rule 2):** First `just test-sql` invocation under the default sandbox failed at the runner-init stage with `mktemp: mkstemp failed on /var/folders/.../T/tmp.X: Operation not permitted` followed by `/bin/sh: : No such file or directory` and the runner's `_duckdb.InvalidInputException: Invalid Input Error: This setting is not adjustable by a user`. This is the documented macOS sandbox quirk: the project Makefile invokes `mktemp` against `/var/folders/.../T/` (macOS hardcoded), which the sandbox blocks unless `dangerouslyDisableSandbox: true` is set. Per CLAUDE.md Rule 2 the bypass is pre-approved for `just test-sql` / `just test-all` / `cargo build` / etc. — retried with bypass and got the canonical green-baseline `56 tests run, 0 failed`.
- Same quirk reappeared on `just test-all`; same bypass applied; full quality gate green.

## User Setup Required

None — all changes are internal cleanup + a new test assertion against existing behaviour.

## Next Phase Readiness

- **Wave 3 finishes Plan 09 cleanly** alongside Plans 07 and 08; the wave is now done. Plan 09 was the last Wave 3 item per the orchestrator dispatch.
- **No follow-up TECH-DEBT created** — both IN-02 and IN-03 close the original deferred-optimisation footnotes in Phase 65 Plan 05's BATCH2-SUMMARY. The previously-tracked "type_cache stays available for a future follow-up if benchmarks ever justify it" is now superseded by D-23: if a future need arises the cache can be reintroduced from git history, and the LIMIT-0 probe cost has been empirically sub-millisecond on the existing test surface for the entire Phase 65 series.
- **IN-05 closure does not gate any other plan.** The null-short-circuit behaviour for `read_yaml_from_semantic_view` and `get_ddl` is already in place (Phase 64 / Phase 51 era); the test simply pins the contract.

## Self-Check: PASSED

Verified:
- `[ ! -f src/type_cache.rs ]` → file no longer exists ✓
- `grep -rn 'type_cache' src/ tests/` → 0 hits ✓
- `grep -rn 'type_id_to_display_name' src/ tests/` → 0 hits ✓
- `grep -q 'mod type_cache' src/lib.rs` → no match ✓
- `cargo build` exit 0 ✓
- `cargo test --lib` 840/840 passed ✓
- `just build` exit 0 ✓
- `just test-sql` 56/56 SUCCESS (including phase651_null_name_inputs.test) ✓
- `just test-all` exit 0 (939 Rust + 56 SQL + 6 ADBC + 6 query + 12/12 readonly + 3/3 multi_db + concurrent_reads + concurrent_writes + concurrent_ddl all green) ✓
- Both task commits exist in `git log`: `3f1bd8e` (Task 1), `da32fec` (Task 2) ✓
- `test/sql/phase651_null_name_inputs.test` contains all three required `IS NULL` assertions ✓
- `test/sql/TEST_LIST` still lists `test/sql/phase651_null_name_inputs.test` ✓

## Threat Flags

None — Plan 09 is deletion-only + a sqllogictest assertion against existing behaviour. No new network endpoints, no new auth paths, no new file access, no schema changes at trust boundaries. The deletions narrow the attack surface (one fewer module + one fewer dead helper to confuse a future contributor); the test pins a defensive null-handling contract.

---
*Phase: 65.1-phase-65-code-review-remediation*
*Completed: 2026-05-25*
