docs: revise architecture audit and roadmap for v2.6.0

[carlos-alm]

Summary

• Revise generated/architecture.md against the actual codebase as it grew from v1.4.0 (5K lines, 12 modules) to v2.6.0 (17,830 lines, 35 modules)
• Add Phase 2.5 (Analysis Expansion) to docs/roadmap/ROADMAP.md documenting 18 modules shipped across v2.0.0-v2.6.0
• Reprioritize all architectural recommendations based on actual growth patterns

Key priority shifts:

• Dual-function anti-pattern across 15 modules is now Bump actions/setup-node from 4 to 6 #1 (was Bump actions/github-script from 7 to 8 #3)
• Parser plugin system dropped from Bump actions/setup-node from 4 to 6 #1 to feat: registry hardening, single-repo MCP, and strategic repositioning #20 (parser.js shrank from 2,215 to 404 lines)
• MCP composability, CLI command objects, and curated API jumped from Medium to High
• 3 new recommendations added: complexity.js decomposition, unified graph model, pagination standardization

Roadmap changes:

• Phase 2.5 documents all features shipped since v1.4.0 that weren't in the original roadmap
• Phase 5.3 (Hybrid Search) marked as completed early during Phase 2.5
• Phase 3 re-prioritized with command/query separation and repository pattern as critical foundation
• Preserves Phase 4 (TypeScript Migration) from docs: add TypeScript migration phase to roadmap #255
• Version updated from 1.4.0 to 2.6.0

Test plan

• No code changes -- docs only
• Verify all internal links resolve
• Verify phase numbering is consistent throughout

[claude]

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

---

PR Review Complete

Root Cause Analysis

Problem Being Solved: This PR updates documentation to reflect actual codebase evolution from v1.4.0 (5K lines, 12 modules) to v2.6.0 (17,830 lines, 35 modules). The architecture audit reprioritizes recommendations based on real growth patterns, and the roadmap documents features shipped across v2.0.0-v2.6.0.

Why This Approach: The original architecture audit and roadmap were written for a much smaller codebase. As the project grew 3.5x, the priority of architectural issues shifted (e.g., dual-function pattern became #1 concern, parser plugin system dropped in priority).

Risk Assessment: HIGH RISK - Misleading PR Scope

Critical Concerns

1. Misleading PR Scope ⚠️

• Claims: "No code changes -- docs only"
• Reality: 31 files changed across 12 commits, including:
  • 6 src/ modules modified
  • New normalizeSymbol utility added
  • Dataflow analysis features
  • New test files
  • GitHub workflows and infrastructure

2. Mixed Concerns in Single PR ⚠️

This bundles multiple unrelated changes:

• Documentation updates (architecture.md, roadmap.md) ✅
• New feature development (dataflow analysis, normalizeSymbol) ❌
• Infrastructure changes (CLA, workflows, CONTRIBUTING.md) ❌
• Test additions ❌

3. Code Quality Issues

The added normalizeSymbol function in queries.js:

• Adds more exports to an already bloated index.js (120+ exports mentioned in architecture.md)
• Continues the dual-function pattern that the architecture audit identifies as the Bump actions/setup-node from 4 to 6 #1 problem
• No clear justification for why this utility needed to be added alongside documentation

4. Inconsistent with Architecture Analysis

The PR adds code that exemplifies the problems described in the architecture audit:

• Adds more exports to index.js (which audit says should be reduced 120+ → 30)
• Continues patterns the audit identifies as technical debt

Positive Aspects

• Documentation quality: The architecture audit revision is thorough and well-researched
• Accurate analysis: Correctly identifies that dual-function pattern became the dominant issue
• Good prioritization: Roadmap Phase 2.5 properly documents organic feature growth

Recommendations

Option 1: Split the PR ⭐ RECOMMENDED

1. Accept docs-only subset: Just architecture.md + roadmap.md changes
2. Separate feature PR: Move normalizeSymbol + dataflow + tests to dedicated feature PR
3. Separate infrastructure PR: Move CLA + workflows + contributing changes

Option 2: Accurate Relabeling

If keeping together, update PR description to accurately reflect scope:

• "docs: major architecture audit revision + normalizeSymbol utility + dataflow features + infrastructure updates"
• Remove "docs only" claim from description and test plan

Final Recommendation

• Rating: ⭐⭐☆☆☆ (2/5)
• Action: REQUEST CHANGES - Split mixed concerns or accurately describe scope
• Reasoning: While the documentation updates are valuable, the misleading scope description and mixed concerns make this difficult to review properly. The code additions contradict the architectural problems being documented.

The documentation work is excellent, but it shouldn't be bundled with unrelated feature development and infrastructure changes.

---

[greptile-apps]

Greptile Summary

This PR combines feature implementation with documentation updates, despite the title indicating "docs only." The main technical contribution is the introduction of normalizeSymbol, a utility function that standardizes symbol output across all query commands to a consistent 7-field schema (name, kind, file, line, endLine, role, fileHash). This implementation includes hash caching for performance, comprehensive unit and integration tests, and updates to embedder search functions.

The documentation changes update the roadmap from v1.4.0 to v2.6.0, add Phase 2.5 documenting 18 new analysis modules shipped since the original roadmap, and revise architectural recommendations based on actual codebase growth (3.5x expansion). A new docs/json-schema.md file documents the stable symbol format.

Key concerns:

• PR description mismatch: States "No code changes -- docs only" but includes significant code changes (new function, API changes, tests)
• Bundled commits: Contains multiple feature commits, fixes, and doc updates that could have been separate PRs
• Schema change impact: All consumers relying on query output will now receive additional fields (endLine, role, fileHash)

The code quality is excellent with proper caching, comprehensive tests, and consistent application. However, the PR structure makes it harder to review and understand the scope of changes.

Confidence Score: 4/5

• Safe to merge with attention to PR description accuracy and potential downstream impacts
• Code implementation is well-tested and follows good practices (caching, consistent API, comprehensive tests). Deducted one point due to misleading PR description and bundled changes that complicate review. The schema changes are backwards-compatible additions but may affect downstream consumers.
• Verify that src/queries.js changes maintain backwards compatibility for existing API consumers, and ensure the PR description accurately reflects the scope of changes

Important Files Changed

Filename	Overview
docs/json-schema.md	New documentation file describing the stable 7-field symbol schema used across all commands
src/queries.js	Added normalizeSymbol utility function and updated all query functions to use it for consistent output
src/embedder.js	Updated search functions to use normalizeSymbol for consistent 7-field output with hash caching
tests/unit/normalize-symbol.test.js	Comprehensive unit tests for normalizeSymbol function covering all edge cases and caching behavior
docs/roadmap/ROADMAP.md	Updated roadmap to reflect v2.6.0 status, added Phase 2.5 documenting analysis expansion features
generated/architecture.md	Revised architecture recommendations based on actual codebase growth from v1.4.0 to v2.6.0

_{Last reviewed commit: a4f34d4}

[greptile-apps]

_{8 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

[carlos-alm]

Superseded by #266 (docs only) and #267 (normalizeSymbol feature) — split to address mixed-concern feedback from review.

[carlos-alm]

Superseded by #266 (rebased, clean commit history, all CI passing).