Skip to content

docs: move roadmap files into docs/roadmap/#92

Merged
carlos-alm merged 2 commits intomainfrom
fix/dogfood-incremental-structure
Feb 25, 2026
Merged

docs: move roadmap files into docs/roadmap/#92
carlos-alm merged 2 commits intomainfrom
fix/dogfood-incremental-structure

Conversation

@carlos-alm
Copy link
Contributor

@carlos-alm carlos-alm commented Feb 25, 2026

Summary

  • Move BACKLOG.md, ROADMAP.md, and llm-integration.md from docs/ into docs/roadmap/ for better organization
  • Update references in README.md, STABILITY.md, and BACKLOG.md to point to new locations

Changes

5 files changed, 3 insertions, 3 deletions (file renames + path fixes)

Test plan

  • Verify all internal links resolve correctly after the move
  • Check for broken relative link: ../FOUNDATION.md in docs/roadmap/BACKLOG.md may need to be ../../FOUNDATION.md

Move BACKLOG.md, ROADMAP.md, and llm-integration.md from docs/ to
docs/roadmap/ for better organization. Update references in README.md,
STABILITY.md, and BACKLOG.md.
@claude
Copy link

claude bot commented Feb 25, 2026

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


PR Review Complete

Analyzing PR #92: Node role classification, dead code detection, docs reorganization

Review Checklist:

  • Phase 1: Root cause analysis - understand the problem being solved
  • Phase 2: Configuration change detection - check for risky changes
  • Phase 3: Test integrity - verify test quality
  • Phase 4: Code quality review - examine implementation
  • Final assessment and recommendation

Root Cause Analysis

Problem Being Solved: This PR implements node role classification to help AI agents immediately understand the architectural role of any symbol (entry/core/utility/adapter/dead/leaf) without reading surrounding code. This directly addresses the token waste problem identified in the project's core mission.

Why This Approach: Uses fan-in/fan-out connectivity patterns with adaptive median thresholds, which is a well-established graph analysis technique. The implementation is zero-dependency and builds on existing SQLite infrastructure.

Risk Assessment: HIGH RISK due to significant discrepancies between stated and actual scope.

Critical Concerns

⚠️ 1. MISLEADING PR DESCRIPTION

  • Claimed: "5 files changed, +3/-3 lines"
  • Actual: "32 files changed, +1407/-87 lines"
  • Missing from changed_files: 13 core implementation files including the main feature (src/structure.js, src/queries.js, src/cli.js, etc.)

This is a massive red flag. The PR completely misrepresents its scope, making proper review nearly impossible.

⚠️ 2. INCONSISTENT DATABASE SCHEMA HANDLING

  • Claims "DB migration v5" but no formal migration exists
  • Uses try/catch ALTER TABLE directly in initSchema() instead of migration system
  • Breaks their own established migration pattern from MIGRATIONS array

Code: src/db.js:118-130

try {
  db.exec('ALTER TABLE nodes ADD COLUMN role TEXT');
} catch {
  /* already exists */
}

⚠️ 3. BROKEN FEATURE BUNDLING

This PR combines multiple unrelated changes:

  • Node role classification (major feature)
  • Dead code detection
  • Docs reorganization (file moves)
  • Builder performance improvements
  • Embedder search enhancements
  • Registry improvements
  • Competitive analysis updates

Should have been separate PRs for proper review and rollback capability.

Code Quality Assessment

Positive Aspects

  1. Algorithm is sound: Median-based fan-in/fan-out thresholds with role classification logic is well-designed
  2. Comprehensive testing: 3 new test files with good coverage of role classification logic
  3. Zero dependencies: Uses existing SQLite infrastructure
  4. Follows existing patterns: CLI integration and query structure match established conventions

⚠️ Quality Concerns

  1. No validation of median calculation edge cases - what happens with empty datasets?
  2. Missing error handling in classifyNodeRoles() for malformed data
  3. Hard-coded magic numbers without constants (e.g., line 30 limit in role display)
  4. Complex SQL queries without performance analysis on large codebases

Test Integrity

  • Unit tests: Solid coverage of core classification logic
  • Integration tests: Good coverage of CLI and query integration
  • No test weakening detected: Tests have appropriate assertions
  • No timeout increases or coverage threshold reductions

Final Recommendation

Rating: ⭐⭐☆☆☆ (2/5)
Action: REQUEST CHANGES

Reasoning: While the core feature implementation is technically sound and addresses a real problem, this PR has serious process and transparency issues that undermine trust:

  1. Misleading scope representation makes proper review impossible
  2. Database schema inconsistencies break established patterns
  3. Feature bundling prevents proper impact assessment and rollback capability

Required Changes:

  1. Split this PR - Separate docs reorganization from feature implementation
  2. Fix database migration - Use proper migration v5 instead of try/catch ALTER
  3. Update PR description to accurately reflect the 32 changed files and 1400+ lines
  4. Add performance benchmarks for role classification on large codebases
  5. Add error handling for edge cases in median calculation

The underlying feature is valuable and well-implemented, but the presentation and bundling make this unsuitable for merge in current form.


@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 25, 2026

Greptile Summary

Reorganizes roadmap documentation by moving three files from docs/ into a new docs/roadmap/ subdirectory and updates references in README.md and STABILITY.md.

Changes:

  • Moved docs/BACKLOG.mddocs/roadmap/BACKLOG.md (with content update marking node classification as DONE)
  • Moved docs/ROADMAP.mddocs/roadmap/ROADMAP.md (no content changes)
  • Moved docs/llm-integration.mddocs/roadmap/llm-integration.md (no content changes)
  • Updated links in README.md and STABILITY.md to point to new locations

Issue found:

  • docs/roadmap/BACKLOG.md line 15: broken relative link ../FOUNDATION.md should be ../../FOUNDATION.md to account for the extra nesting level

Confidence Score: 4/5

  • Safe to merge after fixing the broken FOUNDATION.md link
  • Documentation-only change with clean file moves, but contains one broken relative link that needs correction
  • docs/roadmap/BACKLOG.md requires a one-line fix for the FOUNDATION.md link

Important Files Changed

Filename Overview
README.md Updated ROADMAP.md link to new docs/roadmap/ location
STABILITY.md Updated ROADMAP.md link to new docs/roadmap/ location
docs/roadmap/BACKLOG.md Moved from docs/ to docs/roadmap/, but FOUNDATION.md relative path needs update (should be ../../ not ../)
docs/roadmap/ROADMAP.md Moved from docs/ to docs/roadmap/ with no content changes
docs/roadmap/llm-integration.md Moved from docs/ to docs/roadmap/ with no content changes

Last reviewed commit: 693a8aa

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

5 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 25, 2026

Additional Comments (1)

docs/roadmap/BACKLOG.md
Broken link: ../FOUNDATION.md should be ../../FOUNDATION.md since this file moved from docs/ to docs/roadmap/

| **Foundation-aligned** | Does this feature align with the [FOUNDATION.md](../../FOUNDATION.md) core principles? Specifically: does it keep the graph always-current (P1), maintain zero-cost core with optional LLM enhancement (P4), respect embeddable-first design (P5), and stay honest about what we are — a code intelligence engine, not an application (P8)? A checkmark means full alignment. An X means it conflicts with at least one principle and needs a deliberate exception. |

@carlos-alm carlos-alm changed the title feat: node role classification, dead code detection, docs reorganization docs: move roadmap files into docs/roadmap/ Feb 25, 2026
@claude
Copy link

claude bot commented Feb 25, 2026

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


PR Review Analysis

Analyzing PR #92: docs: move roadmap files into docs/roadmap/

Review Checklist:

  • Phase 1: Root cause analysis - understand the problem being solved
  • Phase 2: Configuration change detection - check for risky changes
  • Phase 3: Test integrity - verify test quality
  • Phase 4: Code quality review - examine implementation
  • Final assessment and recommendation

Root Cause Analysis

Problem Being Solved: This is a documentation organization change to improve the structure of roadmap-related files by moving them into a dedicated docs/roadmap/ subdirectory. This addresses file organization and makes the docs structure cleaner.

Why This Approach: Simple file moves with corresponding link updates is the standard approach for documentation reorganization. The changes are minimal and focused.

Risk Assessment: LOW RISK - This is a pure documentation reorganization with no code changes.

Critical Concerns

Previous Review Confusion: There was significant confusion in the previous Claude review due to the branch containing both this simple docs reorganization AND a major node role classification feature. However, examining the actual commits shows:

  1. Commit 693a8aa: The actual docs move (5 files changed, renames + link updates)
  2. Commit dc08aee: Fix for the broken FOUNDATION.md relative link identified by Greptile
  3. Earlier commits: Contains the major feature implementation (node classification)

The current PR scope is correctly represented - this is genuinely just the docs reorganization with the link fix.

Analysis Results

Phase 2: Configuration Changes

  • No configuration, build, or linting changes
  • No risky modifications detected

Phase 3: Test Integrity

  • No test files modified
  • No test weakening or timeout changes

Phase 4: Code Quality

  • File moves executed correctly: All three roadmap files moved to docs/roadmap/ subdirectory
  • Link updates verified:
    • README.md: docs/ROADMAP.mddocs/roadmap/ROADMAP.md
    • STABILITY.md: Reference updated correctly ✅
    • docs/roadmap/BACKLOG.md: Broken ../FOUNDATION.md../../FOUNDATION.md ✅ (fixed in dc08aee)
  • Content preserved: File moves are clean renames with no content loss
  • No broken links remaining: The Greptile-identified issue has been resolved

Final Recommendation

Rating: ⭐⭐⭐⭐☆ (4/5)
Action: APPROVE

Reasoning: This is a clean, well-executed documentation reorganization that:

Proper scope: Genuinely limited to docs reorganization as described
Complete execution: All necessary files moved and links updated
Issue resolution: The broken relative link has been fixed
Zero risk: No code, configuration, or test changes
Improved organization: Roadmap files are now properly grouped in docs/roadmap/

The only reason this doesn't get 5 stars is that it required a follow-up commit to fix the broken link, indicating the initial move wasn't fully complete. However, the fix has been applied correctly.

This is ready to merge and will improve the documentation structure without any functional impact.


@carlos-alm carlos-alm merged commit d933c9d into main Feb 25, 2026
15 checks passed
@carlos-alm carlos-alm deleted the fix/dogfood-incremental-structure branch February 25, 2026 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant