diff --git a/REVIEW.md b/REVIEW.md
new file mode 100644
index 00000000..e465269b
--- /dev/null
+++ b/REVIEW.md
@@ -0,0 +1,705 @@
+# Deep Code Review: PR #235 - TypeCast Refactor to AST-Driven Logic
+
+**Reviewer**: Devin AI  
+**Date**: November 23, 2025  
+**PR**: https://github.com/launchql/pgsql-parser/pull/235  
+**Branch**: `devin/1763876326-typecast-ast-driven-clean`
+
+---
+
+## Executive Summary
+
+**Is this better?** **Yes, conceptually and architecturally** - but with important caveats and recommendations.
+
+**Is this more robust?** **Mostly yes** - eliminates string-based heuristics that could misfire on edge cases, but introduces new complexity around negative number detection that needs verification.
+
+**Is this more readable?** **Mixed** - the intent is clearer (AST-driven vs string inspection), but the implementation has dead code and could be simplified.
+
+**Overall Recommendation**: **Approve with modifications** - The core refactoring is sound and aligns with the project's AST-first philosophy. However, the PR should address unused helper methods and add targeted test coverage before merging.
+
+---
+
+## 1. Detailed Analysis: Old vs New Implementation
+
+### 1.1 Control Flow Comparison
+
+#### OLD IMPLEMENTATION (main branch)
+```typescript
+TypeCast(node: t.TypeCast, context: DeparserContext): string {
+  const arg = this.visit(node.arg, context);
+  const typeName = this.TypeName(node.typeName, context);
+
+  // Special case: pg_catalog.bpchar
+  if (typeName === 'bpchar' || typeName === 'pg_catalog.bpchar') {
+    const names = node.typeName?.names;
+    const isQualifiedBpchar = names && names.length === 2 &&
+                             (names[0] as any)?.String?.sval === 'pg_catalog' &&
+                             (names[1] as any)?.String?.sval === 'bpchar';
+    if (isQualifiedBpchar) {
+      return `CAST(${arg} AS ${typeName})`;
+    }
+  }
+
+  if (this.isPgCatalogType(typeName)) {
+    const argType = this.getNodeType(node.arg);
+    const isSimpleArgument = argType === 'A_Const' || argType === 'ColumnRef';
+    const isFunctionCall = argType === 'FuncCall';
+
+    if (isSimpleArgument || isFunctionCall) {
+      // STRING INSPECTION HERE ⚠️
+      const shouldUseCastSyntax = isSimpleArgument && (arg.includes('(') || arg.startsWith('-'));
+      
+      if (!shouldUseCastSyntax) {
+        const cleanTypeName = typeName.replace('pg_catalog.', '');
+        if (isFunctionCall) {
+          return `${context.parens(arg)}::${cleanTypeName}`;
+        }
+        return `${arg}::${cleanTypeName}`;
+      }
+    }
+  }
+
+  return `CAST(${arg} AS ${typeName})`;
+}
+```
+
+**Key Decision Points (OLD)**:
+1. Check rendered `typeName` string for bpchar, then verify AST
+2. Check if `typeName` is pg_catalog type (string-based)
+3. Check if `argType` is simple (A_Const/ColumnRef) or FuncCall
+4. **STRING INSPECTION**: Check if rendered `arg` contains `(` or starts with `-`
+5. Decide between `::` syntax vs `CAST()` syntax
+
+#### NEW IMPLEMENTATION (PR branch)
+```typescript
+TypeCast(node: t.TypeCast, context: DeparserContext): string {
+  const arg = this.visit(node.arg, context);
+  const typeName = this.TypeName(node.typeName, context);
+
+  // Special case: pg_catalog.bpchar (AST-based check)
+  if (this.isQualifiedName(node.typeName?.names, ['pg_catalog', 'bpchar'])) {
+    return `CAST(${arg} AS ${typeName})`;
+  }
+
+  if (this.isPgCatalogType(typeName)) {
+    const argType = this.getNodeType(node.arg);
+
+    // AST INSPECTION HERE ✓
+    const needsCastSyntax = this.argumentNeedsCastSyntax(node.arg);
+    
+    if (!needsCastSyntax) {
+      const cleanTypeName = typeName.replace(/^pg_catalog\./, '');
+      if (argType === 'FuncCall') {
+        return `${context.parens(arg)}::${cleanTypeName}`;
+      }
+      return `${arg}::${cleanTypeName}`;
+    }
+  }
+
+  return `CAST(${arg} AS ${typeName})`;
+}
+```
+
+**Key Decision Points (NEW)**:
+1. Check AST directly for `['pg_catalog', 'bpchar']` qualified name
+2. Check if `typeName` is pg_catalog type (string-based, unchanged)
+3. **AST INSPECTION**: Call `argumentNeedsCastSyntax()` to check node structure
+4. Decide between `::` syntax vs `CAST()` syntax
+
+### 1.2 Critical Difference: String Heuristics → AST Predicates
+
+The core improvement is replacing these string-based checks:
+- `arg.includes('(')` - detected if rendered string contains parentheses
+- `arg.startsWith('-')` - detected if rendered string starts with minus
+
+With AST-based logic in `argumentNeedsCastSyntax()`:
+- Inspects node type directly (`getNodeType(argNode)`)
+- Checks AST properties (`ival < 0`, `fval` starts with `-`)
+- Returns `true` for complex node types (A_Expr, SubLink, etc.)
+
+---
+
+## 2. Behavioral Equivalence Analysis
+
+### 2.1 What Stayed the Same ✓
+
+**For non-simple argument types**: The old code only considered `A_Const`, `ColumnRef`, and `FuncCall` for the `::` syntax path. Everything else (A_Expr, SubLink, TypeCast, RowExpr, ArrayExpr, etc.) fell through to `CAST()`. The new code preserves this: `argumentNeedsCastSyntax()` returns `true` for all node types except `FuncCall`, `A_Const` (non-negative), and `ColumnRef`.
+
+**For FuncCall**: Both old and new wrap in parentheses and use `::` syntax. No change.
+
+**For ColumnRef**: Both old and new use `::` syntax directly. No change.
+
+**For pg_catalog.bpchar**: Both force `CAST()` syntax. The new code uses AST check instead of string check, but the behavior is identical.
+
+### 2.2 What Changed 🔍
+
+**For A_Const with parentheses in rendered form**: 
+- **OLD**: `arg.includes('(')` would catch cases like `'('::text` (a string literal containing a parenthesis)
+- **NEW**: `argumentNeedsCastSyntax()` checks node type and properties, not rendered string
+- **Impact**: This is actually an **improvement** - the old code could misfire on string literals containing `(`. The new code correctly identifies that `'('::text` is a simple string constant and can use `::` syntax.
+
+**For A_Const with negative numbers**:
+- **OLD**: `arg.startsWith('-')` caught any rendered argument starting with `-`
+- **NEW**: `argumentNeedsCastSyntax()` checks AST properties: `ival < 0`, `fval` starts with `-`, `val.Integer.ival < 0`, `val.Float.fval` starts with `-`
+- **Impact**: More semantically correct, but depends on how PostgreSQL AST represents negative literals (see concerns below)
+
+---
+
+## 3. Deep Dive: `argumentNeedsCastSyntax()` Logic
+
+```typescript
+private argumentNeedsCastSyntax(argNode: any): boolean {
+  const argType = this.getNodeType(argNode);
+  
+  // FuncCall nodes can use :: syntax (TypeCast will add parentheses)
+  if (argType === 'FuncCall') {
+    return false;
+  }
+  
+  // Simple constants and column references can use :: syntax
+  if (argType === 'A_Const' || argType === 'ColumnRef') {
+    if (argType === 'A_Const') {
+      const nodeAny = (argNode.A_Const || argNode) as any;
+      
+      // Check for negative numbers (needs CAST() to avoid precedence issues)
+      if (nodeAny.ival !== undefined) {
+        const ivalValue = typeof nodeAny.ival === 'object' ? nodeAny.ival.ival : nodeAny.ival;
+        if (typeof ivalValue === 'number' && ivalValue < 0) {
+          return true;
+        }
+      }
+      
+      if (nodeAny.fval !== undefined) {
+        const fvalValue = typeof nodeAny.fval === 'object' ? nodeAny.fval.fval : nodeAny.fval;
+        const fvalStr = String(fvalValue);
+        if (fvalStr.startsWith('-')) {
+          return true;
+        }
+      }
+      
+      // Check for Integer/Float in val field
+      if (nodeAny.val) {
+        if (nodeAny.val.Integer?.ival !== undefined && nodeAny.val.Integer.ival < 0) {
+          return true;
+        }
+        if (nodeAny.val.Float?.fval !== undefined) {
+          const fvalStr = String(nodeAny.val.Float.fval);
+          if (fvalStr.startsWith('-')) {
+            return true;
+          }
+        }
+      }
+      
+      // All other A_Const types are simple
+      return false;
+    }
+    
+    // ColumnRef can always use :: syntax
+    return false;
+  }
+  
+  // All other node types are considered complex
+  return true;
+}
+```
+
+### 3.1 Strengths ✓
+
+1. **Explicit node type handling**: Clear logic for FuncCall, A_Const, ColumnRef
+2. **Multiple AST representations**: Handles different ways negative numbers might be stored
+3. **Safe fallback**: Unknown node types default to `true` (use CAST), which is conservative
+4. **Eliminates string inspection**: No longer depends on rendered output format
+
+### 3.2 Concerns ⚠️
+
+#### Concern #1: Negative Number Representation
+
+**Question**: How does PostgreSQL's AST actually represent `-1`?
+
+**Possibilities**:
+1. **As A_Const with negative ival**: `{ A_Const: { ival: -1 } }` ✓ Handled
+2. **As A_Const with val.Integer**: `{ A_Const: { val: { Integer: { ival: -1 } } } }` ✓ Handled
+3. **As unary minus operator**: `{ A_Expr: { kind: AEXPR_OP_MINUS, rexpr: { A_Const: { ival: 1 } } } }` ⚠️ Would fall through to `return true` (CAST syntax)
+
+**Evidence from fixtures**: Found examples like `(-1::int8<<63)` and `(-9223372036854775808)::int8` in `__fixtures__/kitchen-sink/original/upstream/int8.sql`, suggesting parenthesized negative numbers are common.
+
+**Analysis**: If PostgreSQL represents `-1` as a unary operator wrapping a positive constant, then `argumentNeedsCastSyntax()` would return `true` (because A_Expr is not in the simple list), which would force `CAST()` syntax. This is actually **safe and correct** - it preserves the old behavior of using `CAST()` for complex expressions. The only risk is if there's a case where the old string check `arg.startsWith('-')` caught something that the new AST checks miss, but given the conservative fallback, this seems unlikely.
+
+**Recommendation**: Add explicit test cases for:
+- `SELECT -1::integer` (unparenthesized negative)
+- `SELECT (-1)::integer` (parenthesized negative)
+- `SELECT -1.5::numeric` (negative float)
+- `SELECT (-1.5)::numeric` (parenthesized negative float)
+
+And verify the AST structure and deparsed output matches expectations.
+
+#### Concern #2: String Literal Edge Case
+
+**Old behavior**: `'('::text` would trigger `arg.includes('(')` → use CAST()
+**New behavior**: `'('::text` is A_Const → check for negative → return false → use `::`
+
+**Analysis**: This is actually an **improvement**. The old code was overly conservative. A string literal containing `(` is still a simple constant and can safely use `::` syntax.
+
+**Verification needed**: Confirm that `SELECT '('::text` round-trips correctly with the new logic.
+
+---
+
+## 4. Helper Methods Analysis
+
+### 4.1 Used Helper: `isQualifiedName()`
+
+```typescript
+private isQualifiedName(names: any[] | undefined, expectedPath: string[]): boolean {
+  if (!names || names.length !== expectedPath.length) {
+    return false;
+  }
+  
+  for (let i = 0; i < expectedPath.length; i++) {
+    const nameValue = (names[i] as any)?.String?.sval;
+    if (nameValue !== expectedPath[i]) {
+      return false;
+    }
+  }
+  
+  return true;
+}
+```
+
+**Assessment**: ✓ **Good**
+- Clear, single-purpose function
+- Used in TypeCast for bpchar check
+- More readable than inline AST navigation
+- Could potentially be reused elsewhere in the codebase
+
+### 4.2 Unused Helper: `isBuiltinPgCatalogType()`
+
+```typescript
+private isBuiltinPgCatalogType(typeNameNode: t.TypeName): boolean {
+  // ... 37 lines of code ...
+  // Checks if TypeName is built-in pg_catalog type
+  // Handles qualified and unqualified types
+  // Checks aliases
+}
+```
+
+**Assessment**: ⚠️ **Dead Code**
+- **0 call sites** in the entire codebase (verified via grep)
+- Duplicates logic from existing `isPgCatalogType(typeName: string)` method (line 2229)
+- Adds 37 lines of unused code to the PR
+
+**Comparison with existing `isPgCatalogType()`**:
+- **Existing** (line 2229): Takes rendered string, checks against `pgCatalogTypes` array and aliases
+- **New unused**: Takes TypeName AST node, extracts names, checks against same arrays
+
+**Recommendation**: **Remove** this helper or **integrate** it to replace the string-based `isPgCatalogType()` check in TypeCast. If the goal is to move toward AST-driven logic, then TypeCast should call `isBuiltinPgCatalogType(node.typeName)` instead of `isPgCatalogType(typeName)`. However, this would be a larger refactoring and should be done in a separate PR if desired.
+
+### 4.3 Unused Helper: `normalizeTypeName()`
+
+```typescript
+private normalizeTypeName(typeNameNode: t.TypeName): string {
+  // ... 24 lines of code ...
+  // Extracts normalized type name, strips pg_catalog prefix
+}
+```
+
+**Assessment**: ⚠️ **Dead Code**
+- **0 call sites** in the entire codebase (verified via grep)
+- Duplicates the inline logic: `typeName.replace(/^pg_catalog\./, '')`
+- Adds 24 lines of unused code to the PR
+
+**Recommendation**: **Remove** this helper. The inline regex replacement is clear enough, and there's no evidence this helper would be reused elsewhere. If AST-based type name normalization is needed in the future, it can be added then.
+
+### 4.4 Comparison with Existing Helpers
+
+The codebase already has these helpers (lines 2195-2243):
+- `getPgCatalogTypeName(typeName: string, size: string | null): string` - Maps internal type names to display names
+- `isPgCatalogType(typeName: string): boolean` - Checks if a string is a pg_catalog type
+
+The new unused helpers (`isBuiltinPgCatalogType`, `normalizeTypeName`) operate on AST nodes instead of strings, but they're not integrated into the TypeCast logic. This creates conceptual inconsistency: TypeCast uses `isPgCatalogType(typeName)` (string-based) but the PR adds AST-based helpers that aren't used.
+
+**Recommendation**: Either:
+1. **Remove** the unused helpers to keep this PR focused on the `argumentNeedsCastSyntax()` refactoring
+2. **Integrate** them by refactoring TypeCast to use AST-based type checking throughout (larger change, separate PR)
+
+---
+
+## 5. Test Coverage Analysis
+
+### 5.1 Current Test Coverage
+
+**Found test files**:
+- `packages/deparser/__tests__/misc/castings.test.ts` - 1 simple test: `SELECT '123'::INTEGER`
+- `packages/deparser/__tests__/misc/pg-catalog.test.ts` - 1 test with pg_catalog.json, but no TypeCast-specific assertions
+
+**Fixture coverage**:
+- Found negative number examples in `__fixtures__/kitchen-sink/original/upstream/int8.sql`:
+  - `SELECT (-1::int8<<63)::text`
+  - `SELECT (-9223372036854775808)::int8 * (-1)::int8`
+- Found bpchar examples in `__fixtures__/kitchen-sink/original/upstream/brin.sql`:
+  - `substr(stringu1, 1, 1)::bpchar`
+
+**Assessment**: ⚠️ **Insufficient**
+
+The PR description claims "All 657 deparser tests passing", which is good for regression testing, but there are **no specific tests** for the new `argumentNeedsCastSyntax()` logic or the edge cases it's supposed to handle.
+
+### 5.2 Recommended New Tests
+
+Add these test cases to `packages/deparser/__tests__/misc/castings.test.ts`:
+
+```typescript
+describe('TypeCast with negative numbers', () => {
+  it('should handle negative integer with CAST syntax', async () => {
+    const sql = `SELECT -1::integer`;
+    const result = await expectParseDeparse(sql);
+    expect(result).toMatchSnapshot();
+  });
+
+  it('should handle parenthesized negative integer', async () => {
+    const sql = `SELECT (-1)::integer`;
+    const result = await expectParseDeparse(sql);
+    expect(result).toMatchSnapshot();
+  });
+
+  it('should handle negative float with CAST syntax', async () => {
+    const sql = `SELECT -1.5::numeric`;
+    const result = await expectParseDeparse(sql);
+    expect(result).toMatchSnapshot();
+  });
+
+  it('should handle parenthesized negative float', async () => {
+    const sql = `SELECT (-1.5)::numeric`;
+    const result = await expectParseDeparse(sql);
+    expect(result).toMatchSnapshot();
+  });
+});
+
+describe('TypeCast with complex expressions', () => {
+  it('should handle expression with CAST syntax', async () => {
+    const sql = `SELECT (1 + 2)::integer`;
+    const result = await expectParseDeparse(sql);
+    expect(result).toMatchSnapshot();
+  });
+
+  it('should handle CASE expression with CAST syntax', async () => {
+    const sql = `SELECT (CASE WHEN a > 0 THEN 1 ELSE 2 END)::integer`;
+    const result = await expectParseDeparse(sql);
+    expect(result).toMatchSnapshot();
+  });
+});
+
+describe('TypeCast with function calls', () => {
+  it('should handle function call with :: syntax and parentheses', async () => {
+    const sql = `SELECT substring('test', 1, 2)::text`;
+    const result = await expectParseDeparse(sql);
+    expect(result).toMatchSnapshot();
+  });
+
+  it('should handle qualified function call', async () => {
+    const sql = `SELECT pg_catalog.substring('test', 1, 2)::text`;
+    const result = await expectParseDeparse(sql);
+    expect(result).toMatchSnapshot();
+  });
+});
+
+describe('TypeCast with pg_catalog.bpchar', () => {
+  it('should preserve CAST syntax for qualified bpchar', async () => {
+    const sql = `SELECT 'x'::pg_catalog.bpchar`;
+    const result = await expectParseDeparse(sql);
+    expect(result).toBe(`SELECT CAST('x' AS pg_catalog.bpchar)`);
+  });
+
+  it('should use :: syntax for unqualified bpchar', async () => {
+    const sql = `SELECT 'x'::bpchar`;
+    const result = await expectParseDeparse(sql);
+    expect(result).toBe(`SELECT 'x'::bpchar`);
+  });
+});
+
+describe('TypeCast with string literals containing special characters', () => {
+  it('should handle string literal with parenthesis', async () => {
+    const sql = `SELECT '('::text`;
+    const result = await expectParseDeparse(sql);
+    expect(result).toBe(`SELECT '('::text`);
+  });
+
+  it('should handle string literal starting with minus', async () => {
+    const sql = `SELECT '-hello'::text`;
+    const result = await expectParseDeparse(sql);
+    expect(result).toBe(`SELECT '-hello'::text`);
+  });
+});
+```
+
+These tests would:
+1. Verify the negative number detection logic works correctly
+2. Confirm complex expressions use CAST() syntax
+3. Verify function calls use :: syntax with parentheses
+4. Confirm pg_catalog.bpchar special handling
+5. Verify the improvement over old string-based heuristics (string literals with `(` or `-`)
+
+---
+
+## 6. Readability Assessment
+
+### 6.1 Improvements ✓
+
+1. **Clearer intent**: `argumentNeedsCastSyntax()` explicitly states what it's checking
+2. **AST-first approach**: Aligns with project philosophy of working with AST structures
+3. **Separation of concerns**: Helper methods isolate specific checks
+4. **Better comments**: New code has clear comments explaining the logic
+
+### 6.2 Concerns ⚠️
+
+1. **Dead code**: 61 lines of unused helper methods reduce readability
+2. **Complexity**: `argumentNeedsCastSyntax()` has 56 lines with multiple nested checks
+3. **Inconsistency**: Uses both string-based (`isPgCatalogType(typeName)`) and AST-based (`isQualifiedName()`) checks
+4. **Documentation**: No JSDoc comments explaining when/why CAST() vs :: syntax is chosen
+
+### 6.3 Recommendations
+
+1. **Remove unused helpers** to reduce noise
+2. **Add JSDoc to TypeCast method** explaining the decision logic:
+   ```typescript
+   /**
+    * Deparse a TypeCast node to SQL.
+    * 
+    * Decision logic:
+    * - pg_catalog.bpchar: Always use CAST() for round-trip fidelity
+    * - pg_catalog types with simple args: Use :: syntax (cleaner)
+    * - pg_catalog types with complex args: Use CAST() (precedence safety)
+    * - All other types: Use CAST() (default)
+    * 
+    * Simple args: positive constants, column refs, function calls
+    * Complex args: negative numbers, expressions, operators, etc.
+    */
+   ```
+3. **Consider extracting negative number detection** to its own helper:
+   ```typescript
+   private isNegativeConstant(node: any): boolean {
+     // ... all the ival/fval checking logic ...
+   }
+   ```
+   This would make `argumentNeedsCastSyntax()` more readable.
+
+---
+
+## 7. Robustness Assessment
+
+### 7.1 Improvements ✓
+
+1. **Eliminates string inspection bugs**: No longer depends on rendered format
+2. **More semantically correct**: Checks actual AST structure, not string representation
+3. **Conservative fallback**: Unknown node types default to CAST() (safe)
+4. **Handles multiple AST representations**: Checks ival, fval, val.Integer, val.Float
+
+### 7.2 Potential Issues ⚠️
+
+1. **Negative number representation uncertainty**: Depends on how PostgreSQL AST represents `-1`
+2. **Version compatibility**: Does AST structure change between PG13-17? (Not verified)
+3. **Incomplete negative detection**: What if there are other ways negatives are represented?
+
+### 7.3 Risk Mitigation
+
+The conservative fallback (`return true` for unknown node types) means that even if the negative number detection is incomplete, the worst case is using `CAST()` syntax instead of `::` syntax. This is **safe** - it may be slightly less clean, but it's functionally correct.
+
+The real risk would be if the new logic uses `::` syntax where the old logic used `CAST()`, potentially breaking precedence. However, the logic is structured to be **at least as conservative** as the old code:
+- Old: Only A_Const, ColumnRef, FuncCall could use ::, and A_Const had string checks
+- New: Only A_Const (non-negative), ColumnRef, FuncCall can use ::
+
+So the new code is **equally or more conservative**, making it safe.
+
+---
+
+## 8. Performance Considerations
+
+**Impact**: Negligible
+
+- **Old**: String operations (`includes()`, `startsWith()`) on already-rendered strings - O(n) where n is string length
+- **New**: AST property access and type checks - O(1) operations
+
+The new approach is likely **slightly faster** since it avoids string scanning, but the difference is negligible in practice. The main benefit is correctness, not performance.
+
+---
+
+## 9. Alignment with Project Goals
+
+### 9.1 Project Philosophy
+
+From the codebase analysis, this project is explicitly about:
+1. **AST-first approach**: Working with PostgreSQL ASTs, not string manipulation
+2. **Type safety**: TypeScript definitions for all AST nodes
+3. **Version compatibility**: Supporting PostgreSQL 13-17
+4. **Round-trip fidelity**: Parse → Deparse → Parse should be idempotent
+
+### 9.2 How This PR Aligns ✓
+
+1. **AST-first**: ✓ Moves from string inspection to AST structure inspection
+2. **Type safety**: ✓ Uses typed AST nodes, though with `any` casts in some places
+3. **Version compatibility**: ⚠️ Not explicitly verified, but conservative fallback should handle differences
+4. **Round-trip fidelity**: ✓ All 657 tests pass, including round-trip tests
+
+**Assessment**: This PR is **well-aligned** with project goals. The move from string heuristics to AST predicates is exactly the kind of improvement this project should be making.
+
+---
+
+## 10. Summary of Findings
+
+### 10.1 What's Better ✓
+
+1. **Conceptually superior**: AST-driven logic is more semantically correct than string inspection
+2. **Eliminates string inspection bugs**: No longer depends on rendered format
+3. **More maintainable**: Clear separation of concerns with helper methods
+4. **Safer for edge cases**: String literals with `(` or `-` no longer misfire
+5. **Aligns with project philosophy**: AST-first approach
+
+### 10.2 What's Concerning ⚠️
+
+1. **Dead code**: 61 lines of unused helper methods (`isBuiltinPgCatalogType`, `normalizeTypeName`)
+2. **Insufficient test coverage**: No specific tests for new logic or edge cases
+3. **Negative number representation**: Uncertainty about how AST represents `-1`
+4. **Inconsistent approach**: Mixes string-based and AST-based type checking
+5. **Complexity**: `argumentNeedsCastSyntax()` is 56 lines with multiple nested checks
+
+### 10.3 What's Unchanged (Good) ✓
+
+1. **Behavioral equivalence**: For most cases, behavior is preserved
+2. **Conservative fallback**: Unknown node types still use CAST()
+3. **FuncCall handling**: Unchanged (parentheses + :: syntax)
+4. **ColumnRef handling**: Unchanged (:: syntax)
+5. **pg_catalog.bpchar**: Unchanged behavior (CAST syntax)
+
+---
+
+## 11. Recommendations
+
+### 11.1 Required Before Merge 🔴
+
+1. **Remove unused helper methods** (`isBuiltinPgCatalogType`, `normalizeTypeName`)
+   - Reduces PR size by 61 lines
+   - Eliminates dead code
+   - Keeps PR focused on the core refactoring
+
+2. **Add targeted test cases** (see section 5.2)
+   - Negative numbers: `-1::integer`, `(-1)::integer`, `-1.5::numeric`
+   - Complex expressions: `(1 + 2)::integer`, `(CASE ...)::integer`
+   - Function calls: `substring(...)::text`
+   - pg_catalog.bpchar: `'x'::pg_catalog.bpchar` vs `'x'::bpchar`
+   - String literals: `'('::text`, `'-hello'::text`
+
+3. **Verify AST structure** for negative numbers
+   - Parse `-1::integer` and inspect the AST
+   - Confirm that `argumentNeedsCastSyntax()` handles it correctly
+   - Document the expected AST structure in a comment
+
+### 11.2 Recommended Improvements 🟡
+
+1. **Add JSDoc comments** to TypeCast and argumentNeedsCastSyntax methods
+   - Explain when CAST() vs :: syntax is used
+   - Document the decision logic
+   - Clarify the precedence safety concerns
+
+2. **Extract negative number detection** to its own helper
+   ```typescript
+   private isNegativeConstant(constNode: any): boolean
+   ```
+   - Makes `argumentNeedsCastSyntax()` more readable
+   - Isolates the complex ival/fval checking logic
+   - Easier to test and verify
+
+3. **Consider consistent AST-based approach**
+   - Currently mixes `isPgCatalogType(typeName)` (string) with `isQualifiedName()` (AST)
+   - Could refactor to use AST-based type checking throughout
+   - But this should be a **separate PR** if pursued
+
+### 11.3 Future Enhancements 🔵
+
+1. **Version compatibility testing**
+   - Verify behavior across PostgreSQL 13-17
+   - Check if AST structure for negative numbers differs between versions
+   - Add version-specific tests if needed
+
+2. **Performance benchmarking**
+   - Measure deparser performance before/after
+   - Confirm no regression (though unlikely)
+
+3. **Expand AST-driven approach**
+   - Apply similar refactoring to other deparser methods
+   - Eliminate remaining string inspection patterns
+   - Build a library of AST predicate helpers
+
+---
+
+## 12. Final Verdict
+
+### Is this really a much better, or more robust way to do it?
+
+**Yes, with caveats.**
+
+The move from string inspection to AST structure inspection is **conceptually superior** and **more robust** for the cases it handles correctly. It eliminates a class of bugs where string formatting could affect behavior (e.g., string literals containing `(` or `-`). The conservative fallback ensures that even if the negative number detection is incomplete, the code remains safe.
+
+However, the PR has **dead code** (unused helpers) and **insufficient test coverage** that need to be addressed before merge.
+
+### Is this going to be an improvement?
+
+**Yes, definitely.**
+
+This is the right direction for the project. Moving toward AST-driven logic aligns with the project's philosophy and makes the codebase more maintainable. The refactoring is well-structured and the core logic is sound.
+
+### Is this more readable?
+
+**Mixed.**
+
+The **intent** is clearer (AST-driven vs string inspection), but the **implementation** has issues:
+- 61 lines of dead code reduce readability
+- `argumentNeedsCastSyntax()` is complex (56 lines)
+- Lacks documentation explaining the decision logic
+
+With the recommended improvements (remove dead code, add JSDoc, extract negative detection), readability would be **significantly better** than the current state.
+
+### Is this... better?
+
+**Yes, this is better** - but it needs polish before merge.
+
+The core refactoring is **sound and valuable**. The issues identified (dead code, test coverage, documentation) are **fixable** and don't undermine the fundamental improvement. With the recommended changes, this PR would be a **clear win** for the codebase.
+
+---
+
+## 13. Action Items for PR Author
+
+### Before Merge (Required)
+- [ ] Remove `isBuiltinPgCatalogType()` method (lines 2372-2409)
+- [ ] Remove `normalizeTypeName()` method (lines 2415-2438)
+- [ ] Add test cases for negative numbers (see section 5.2)
+- [ ] Add test cases for complex expressions (see section 5.2)
+- [ ] Add test cases for pg_catalog.bpchar (see section 5.2)
+- [ ] Add test cases for string literals with special chars (see section 5.2)
+- [ ] Verify AST structure for `-1::integer` and document findings
+
+### Recommended Improvements
+- [ ] Add JSDoc comments to TypeCast method
+- [ ] Add JSDoc comments to argumentNeedsCastSyntax method
+- [ ] Consider extracting `isNegativeConstant()` helper
+- [ ] Update PR description with test coverage details
+
+### Future Considerations
+- [ ] Version compatibility testing across PG13-17
+- [ ] Consider refactoring to use AST-based type checking throughout (separate PR)
+- [ ] Performance benchmarking (optional)
+
+---
+
+## Appendix: Code Metrics
+
+- **Lines added**: 169
+- **Lines removed**: 23
+- **Net change**: +146 lines
+- **Unused code**: 61 lines (42% of additions)
+- **Test coverage**: 2 existing tests, 0 new tests
+- **Helper methods added**: 4 (1 used, 3 unused)
+- **Complexity**: `argumentNeedsCastSyntax()` has cyclomatic complexity ~8
+
+---
+
+**Review completed by**: Devin AI  
+**Session**: https://app.devin.ai/sessions/21764651d79044d7b81e3c4d19222521  
+**Date**: November 23, 2025