Skip to content

feat(titan): first full pipeline run — audit report + skill improvements#623

Merged
carlos-alm merged 10 commits intomainfrom
feat/titan-run-pipeline
Mar 26, 2026
Merged

feat(titan): first full pipeline run — audit report + skill improvements#623
carlos-alm merged 10 commits intomainfrom
feat/titan-run-pipeline

Conversation

@carlos-alm
Copy link
Contributor

Summary

  • First end-to-end Titan pipeline run against the full codegraph codebase
  • Audited 88 files across 15 domains, found 6 FAIL targets, fixed all 6
  • 5 forge commits: empty-catch debug logging, impact.ts split, cfg-visitor split, MAX_WALK_DEPTH extraction, SLOC warning fixes
  • Improved titan-run skill: auto-creates worktree, skips pre-analysis confirmation, dispatches titan-close as final phase
  • Improved titan-close skill: loads all titan-run artifacts (gauntlet.ndjson, arch-snapshot.json, drift-report.json), adds architecture before/after comparison
  • Removed generated/titan/ from .gitignore so audit reports are tracked in git
  • Added full Titan audit report with per-pillar breakdowns, cohesion deltas, and artifact inventory

Forge commits

  1. e1dde35 fix: add debug logging to empty catch blocks (config, native, parser, resolve)
  2. e8f41f4 refactor: split impact.ts into fn-impact + diff-impact modules
  3. 2113bd6 refactor: split cfg-visitor.ts by control-flow construct (4 sub-modules)
  4. 4ceed5d refactor: extract MAX_WALK_DEPTH constant to extractors/helpers.ts
  5. 23bf546 refactor: address SLOC warnings in features (complexity, structure splits)

Key metrics

  • Architecture cohesion: ast-analysis +19%, infrastructure +44%, no regressions
  • Gate: 5/5 PASS, 0 rollbacks
  • Tests: 2113 passed (1 pre-existing ast-parity failure)

Test plan

  • All tests pass (114/115 files, pre-existing ast-parity failure only)
  • Lint clean on changed files
  • Each forge commit validated by titan-gate (cycles, lint, tests, semantic assertions, arch snapshot)
  • No new circular dependencies introduced

…d domain layers

Impact: 8 functions changed, 33 affected
Impact: 16 functions changed, 0 affected
Impact: 2 functions changed, 5 affected
…dit report

- titan-run: auto-create worktree instead of stopping, skip pre-analysis
  confirmation, dispatch titan-close as final phase
- titan-close: load all titan-run artifacts (gauntlet.ndjson, arch-snapshot,
  drift-report), add architecture before/after comparison, stop gitignoring
  reports
- Add Titan audit report with per-pillar breakdowns, architecture cohesion
  deltas, and full artifact inventory
@claude
Copy link

claude bot commented Mar 26, 2026

Claude finished @carlos-alm's task —— View job


Root Cause Analysis

Problem Being Solved: This PR represents the first full end-to-end run of the Titan pipeline — a comprehensive code quality automation system that audits codebases, identifies issues, and applies targeted fixes.

Why This Approach: The PR addresses legitimate code quality issues found by automated analysis: empty catch blocks lacking debug logging, oversized files exceeding SLOC thresholds, and duplicated magic numbers.

Risk Assessment: ✅ LOW RISK - These are genuine technical debt fixes, not symptom hiding.

Backlog Compliance

  • Zero-dep: ✅ — No new runtime dependencies added
  • Foundation-aligned: ✅ — All changes align with Principle 1 (maintainability improvements that keep the graph fast)
  • Problem-fit: 3 — Useful infrastructure improvements that support the core mission, though not directly addressing the "lost AI agents" problem
  • Breaking: No — All changes are internal refactoring with backward compatibility maintained via re-export barrels
  • Tier: 1 (zero-dep ✓ + foundation-aligned ✓)

Code Quality Assessment

✅ Positive Aspects

  1. Legitimate fixes: Debug logging for empty catch blocks improves error visibility
  2. Clean decomposition: File splits maintain clean module boundaries:
    • impact.tsfn-impact.ts + diff-impact.ts (single responsibility)
    • cfg-visitor.ts → 4 focused modules by control flow type
    • complexity.ts/structure.ts → query logic extracted to separate modules
  3. Backward compatibility: Re-export barrels maintain existing API contracts
  4. Architecture improvements: Cohesion increased in modified directories (+19% ast-analysis, +44% infrastructure)

⚠️ Minor Concerns

  1. Large PR scope: 28 files changed with 2604 additions, mixing multiple refactoring concerns
  2. Generated report committed: While allowed per CLAUDE.md, adds 285 lines to git history
  3. biome-ignore statements: While justified, the PR moves several existing ignores around during splits

🔍 Configuration & Test Integrity

  • ✅ No linting rules relaxed
  • ✅ No test files modified or assertions weakened
  • ✅ No configuration changes to biome.json, tsconfig.json, or package.json
  • ✅ All biome-ignore usage appears justified for tree-sitter AST handling

Critical Concerns

None identified. This is genuine technical debt remediation with proper validation.

Final Recommendation

  • Rating: ⭐⭐⭐⭐☆ (4/5)
  • Action: APPROVE WITH CONDITIONS
  • Reasoning: Well-executed automated refactoring addressing legitimate technical debt. The large scope is justified as it represents a complete pipeline run. Code quality improvements are clean and maintain architectural boundaries. Minor concerns about PR size don't warrant rejection given the automated validation and test coverage.

Conditions for Approval

  1. Consider breaking future Titan runs into smaller, focused PRs by domain rather than one omnibus PR
  2. Ensure the generated report mechanism doesn't bloat git history over time (consider retention policies)

The Titan pipeline demonstrates impressive automation capability and the fixes address real technical debt while preserving system architecture and backward compatibility.


- Add titan-close/SKILL.md to docs/examples/claude-code-skills/
- Update README pipeline diagram, skills table, and artifacts table
- Sync titan-run SKILL.md with latest changes (auto-worktree, close dispatch)
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 26, 2026

Greptile Summary

This PR delivers the first full end-to-end Titan pipeline audit of the codegraph codebase, fixing all 6 FAIL targets across 88 audited files, while also improving the titan-run and titan-close skills. The code changes are five forge commits: empty-catch debug logging, splitting impact.ts into fn-impact + diff-impact modules, splitting cfg-visitor.ts by control-flow construct, extracting MAX_WALK_DEPTH to a shared constant, and addressing SLOC warnings in the features layer.

Key changes:

  • impact.ts is now a thin re-export barrel; logic lives in fn-impact.ts (BFS + file/fn impact) and diff-impact.ts (git diff analysis helpers). diff-impact-mermaid.ts was moved to the presentation layer — a clean separation of concerns.
  • complexity.ts and structure.ts split into compute-time and query-time modules (*-query.ts), improving cohesion scores measured by the audit.
  • MAX_WALK_DEPTH = 200 centralized in src/extractors/helpers.ts and imported by all six language extractors — eliminates a repeated magic number.
  • All empty catch {} blocks across config.ts, native.ts, parser.ts, resolve.ts now emit debug(...) messages, preserving the silent-fallback behavior while making errors observable.
  • One issue found: src/ast-analysis/visitors/cfg-conditionals.ts lines 81–84 replaced elseChildren[0]!.type and elseChildren[0]! with elseChildren[0]?.type and elseChildren[0]. With "noUncheckedIndexedAccess": true in tsconfig.json, this produces two TypeScript compile errors: isIfNode expects string but receives string | undefined, and processIf expects TreeSitterNode but receives TreeSitterNode | undefined. The ! assertions were correct and should be restored.

Confidence Score: 4/5

Safe to merge after restoring the two ! non-null assertions in cfg-conditionals.ts to fix the TypeScript type errors introduced by the refactor.

All changes are well-structured refactors with backward-compatible re-exports and test coverage. The single concrete issue — removed ! assertions that break tsc with noUncheckedIndexedAccess: true — is a one-line fix in one file. Everything else (module splits, debug logging, constant extraction) is clean and low-risk.

src/ast-analysis/visitors/cfg-conditionals.ts — two removed ! assertions produce TypeScript compile errors under noUncheckedIndexedAccess: true

Important Files Changed

Filename Overview
src/ast-analysis/visitors/cfg-conditionals.ts Swapped ! non-null assertions for ?. optional chaining — breaks TypeScript compilation under noUncheckedIndexedAccess: true
src/domain/analysis/diff-impact.ts New file: git diff analysis helpers extracted from impact.ts; clean decomposition into focused helper functions with proper error handling
src/domain/analysis/fn-impact.ts New file: BFS transitive caller logic and fn/file impact data functions extracted from impact.ts; well-structured with clear exports
src/domain/analysis/impact.ts Reduced to a re-export barrel for backward compatibility; correctly forwards all previous exports from the new split modules
src/presentation/diff-impact-mermaid.ts New file: Mermaid diagram generation for diff-impact extracted to presentation layer; clean separation with subgraphs for changed files and blast radius
src/extractors/helpers.ts Adds exported MAX_WALK_DEPTH = 200 constant to eliminate the magic number repeated across all six language extractors
src/features/complexity.ts Compute-time concerns kept; query-time DB functions moved to complexity-query.ts; backward-compat re-exports preserved
src/features/structure.ts Build-time structure helpers kept; query-time DB functions moved to structure-query.ts; clean split
src/infrastructure/config.ts Empty catch blocks in detectWorkspaces and resolveWorkspaceEntry now log debug messages instead of silently swallowing errors
src/domain/graph/resolve.ts Three empty catch blocks in native resolution fallback paths now log debug messages with error context

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    subgraph Before["Before (impact.ts monolith)"]
        A["impact.ts\n(bfsTransitiveCallers + fnImpact + diffImpact\n+ mermaid rendering)"]
    end

    subgraph After["After (split modules)"]
        B["fn-impact.ts\nbfsTransitiveCallers\nimpactAnalysisData\nfnImpactData"]
        C["diff-impact.ts\ndiffImpactData\ngit diff helpers"]
        D["presentation/diff-impact-mermaid.ts\ndiffImpactMermaid"]
        E["impact.ts\nre-export barrel only"]
    end

    E -->|re-exports| B
    E -->|re-exports| C
    E -->|re-exports| D

    C -->|imports bfsTransitiveCallers| B
    D -->|imports diffImpactData| C
Loading

Comments Outside Diff (1)

  1. src/ast-analysis/visitors/cfg-conditionals.ts, line 81-84 (link)

    Removing ! assertions breaks TypeScript compilation under noUncheckedIndexedAccess

    tsconfig.json has "noUncheckedIndexedAccess": true, which means elseChildren[0] has type TreeSitterNode | undefined. The refactor changed two things that both produce type errors:

    1. isIfNode(elseChildren[0]?.type, cfgRules)elseChildren[0]?.type is string | undefined, but isIfNode is declared as isIfNode(type: string, cfgRules: AnyRules) — only accepts string.
    2. processIf(elseChildren[0], ...)processIf is declared as processIf(ifStmt: TreeSitterNode, ...) — only accepts TreeSitterNode, not TreeSitterNode | undefined.

    TypeScript cannot narrow elseChildren[0] to a defined value based on a .length === 1 check alone. The original ! non-null assertions were the correct mechanism to handle this. Running npm run typecheck (tsc --noEmit) will fail on these lines.

Reviews (2): Last reviewed commit: "fix: use narrowing variable instead of n..." | Re-trigger Greptile

@carlos-alm
Copy link
Contributor Author

Addressed both Greptile review comments:

  1. ProcessStatementsFn belongs in cfg-shared.ts — Resolved as part of the merge conflict resolution with main. PR refactor: split cfg-visitor.ts by control-flow construct #619 already landed this exact fix on main, so the merge now has ProcessStatementsFn defined and exported from cfg-shared.ts. The cross-module dependency from cfg-loops.ts is eliminated.

  2. Stale JSDoc on diffImpactData — Replaced the leftover Fix #2: Shell injection vulnerability development note with an accurate description: Compute diff-impact analysis between two git refs (or staged changes). Uses execFileSync (via runGitDiff) to avoid shell injection.

Also fixed lint issues in cfg-conditionals.ts (non-null assertions replaced with optional chaining) and formatting issues (extra blank lines) in cfg-loops.ts and cfg-shared.ts during merge conflict resolution.

@carlos-alm
Copy link
Contributor Author

@greptileai

@carlos-alm carlos-alm merged commit 59d4c75 into main Mar 26, 2026
13 checks passed
@carlos-alm carlos-alm deleted the feat/titan-run-pipeline branch March 26, 2026 07:18
@github-actions github-actions bot locked and limited conversation to collaborators Mar 26, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant