Skip to content

feat: implement dogfood report suggestions#70

Merged
carlos-alm merged 4 commits into
mainfrom
feat/dogfood-suggestions
Feb 24, 2026
Merged

feat: implement dogfood report suggestions#70
carlos-alm merged 4 commits into
mainfrom
feat/dogfood-suggestions

Conversation

@carlos-alm
Copy link
Copy Markdown
Contributor

Summary

  • Add excludeTests config option (query.excludeTests in .codegraphrc.json) with --include-tests CLI override on all commands
  • Add coupling score column (=NNN) to map command output
  • Add --depth <n> option to explain for recursive dependency explanation
  • Move engine status messages from stdout to stderr via logger
  • Fix missing #!/bin/sh shebangs on all husky hooks (Windows compatibility)
  • Update DOGFOOD-REPORT-2.1.0 marking all 5 suggestions as implemented

Test plan

  • npm run lint — clean
  • Integration tests (queries + CLI) — 67/67 passed
  • codegraph where explain -T — excludes test files
  • codegraph where explain --include-tests — includes test files
  • codegraph explain buildGraph --depth 1 -T — recursively explains callees
  • codegraph explain openDb --depth 1 -T -j — JSON output with depth works
  • codegraph map -T — shows =NNN coupling column
  • Husky hooks execute on Windows (pre-commit, commit-msg, pre-push)

- Add `excludeTests` config option with `--include-tests` CLI override
- Add coupling score column to `map` command output
- Add `--depth` option to `explain` for recursive dependency explanation
- Move engine status messages from stdout to stderr via logger
- Update DOGFOOD-REPORT-2.1.0 marking all suggestions as implemented

Impact: 8 functions changed, 5 affected
@claude
Copy link
Copy Markdown

claude Bot commented Feb 24, 2026

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


PR Review: feat: implement dogfood report suggestions

Phase 1: Root Cause Analysis

  • Read CLAUDE.md and understand dogfooding context
  • Read the dogfood report to understand what problems were identified
  • Analyze what specific issues this PR addresses

Phase 2: Configuration Change Detection

  • Check for any biome/linting rule changes
  • Verify no test timeouts or coverage thresholds were lowered
  • Review build configuration changes

Phase 3: Test Integrity

  • Examine test modifications for weakened assertions
  • Check for removed or conditional test validation
  • Verify test coverage for new functionality

Phase 4: Code Quality Review

  • Review implementation in cli.js for new options
  • Analyze queries.js changes for coupling scores and explain depth
  • Check config.js for excludeTests option
  • Review builder.js stderr logging changes
  • Examine husky hook shebang fixes

Final Assessment

  • Provide rating and recommendation

Root Cause Analysis

Problem Being Solved: Five specific usability gaps identified through comprehensive dogfooding of codegraph v2.1.0 on itself. These are legitimate issues: missing CLI options that existed in the data layer, inconsistent output formatting, Windows compatibility problems, and stdout pollution.

Why This Approach: All fixes are targeted and conservative. No major architectural changes, just connecting existing functionality to the CLI and improving output consistency.

Risk Assessment: Very low risk. Changes are primarily additive CLI options, display formatting, and fixing missing shebangs.

Critical Concerns

None identified. This is a clean, well-implemented PR that addresses real usability issues without introducing technical debt or risky configuration changes.

Positive observations:

  • Excellent precedence handling in resolveNoTests() helper
  • Proper cycle detection in recursive explain implementation
  • Consistent application of new CLI options across all affected commands
  • Clean separation of concerns (data layer unchanged, only CLI and display improved)
  • All status messages properly moved to stderr

Final Recommendation

  • Rating: ⭐⭐⭐⭐☆ (4/5)
  • Action: APPROVE
  • Reasoning: This is a high-quality implementation of legitimate feature requests from real usage. The code is clean, consistent, and follows established patterns. The only reason it's not 5 stars is that it's incremental improvements rather than groundbreaking functionality, but that's exactly what these dogfood suggestions should be.

Recommendation: Approve and merge. This exemplifies good dogfooding practice - identifying real pain points through usage and addressing them systematically.


@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Feb 24, 2026

Greptile Summary

This PR implements all 5 suggestions from DOGFOOD-REPORT-2.1.0: adds excludeTests config option with --include-tests CLI override, adds coupling score column to map output, adds --depth option to explain for recursive dependency exploration, migrates engine status messages from stdout to stderr, and fixes missing shebangs on husky hooks for Windows compatibility.

Key changes:

  • Config system now supports query.excludeTests default with CLI override via --include-tests flag (applied to all 17 commands)
  • map command displays coupling scores as =NNN column showing combined fan-in/fan-out
  • explain command supports --depth <n> to recursively explain callees up to N levels with cycle-safe tracking
  • All status messages in builder.js migrated from console.log to logger.info() (stderr)
  • Fixed Windows compatibility by adding #!/bin/sh shebangs to all three husky hooks
  • Renamed --include-tests to --with-test-source on context command to avoid naming collision

All integration tests pass (67/67), test spies correctly updated to check stderr instead of stdout.

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • All changes are well-tested with 67/67 integration tests passing, the implementation follows existing patterns, backward compatibility is maintained (config defaults to false, preserving existing behavior), and the test spies were correctly updated to match the stdout-to-stderr migration
  • No files require special attention

Important Files Changed

Filename Overview
src/builder.js migrated status messages from stdout to stderr using logger.info
src/cli.js added excludeTests config support, --include-tests flag, resolveNoTests helper, --depth for explain, renamed --include-test-source to --with-test-source
src/config.js added query.excludeTests: false to DEFAULTS config
src/queries.js added coupling field to moduleMapData, =NNN column to map output, --depth support for explain with recursive callee expansion
tests/integration/build.test.js updated test spies from console.log to stderr.write after status message migration

Last reviewed commit: 242066f

Copy link
Copy Markdown
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.

8 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment thread src/cli.js Outdated
Comment on lines +191 to +193
.option('--include-test-source', 'Include test source code')
.option('-T, --no-tests', 'Exclude test/spec files from results')
.option('--include-tests', 'Include test/spec files (overrides excludeTests config)')
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Consider renaming --include-test-source to something like --with-test-source to distinguish it from --include-tests (which controls test file filtering, not test source inclusion)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@claude
Copy link
Copy Markdown

claude Bot commented Feb 24, 2026

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


PR Review: feat: implement dogfood report suggestions

Phase 1: Root Cause Analysis

  • Read CLAUDE.md and understand dogfooding context
  • Read the dogfood report to understand what problems were identified
  • Analyze what specific issues this PR addresses

Phase 2: Configuration Change Detection

  • Check for any biome/linting rule changes
  • Verify no test timeouts or coverage thresholds were lowered
  • Review build configuration changes

Phase 3: Test Integrity

  • Examine test modifications for weakened assertions
  • Check for removed or conditional test validation
  • Verify test coverage for new functionality

Phase 4: Code Quality Review

  • Review implementation in cli.js for new options
  • Analyze queries.js changes for coupling scores and explain depth
  • Check config.js for excludeTests option
  • Review builder.js stderr logging changes
  • Examine husky hook shebang fixes

Final Assessment

  • Provide rating and recommendation

Root Cause Analysis

Problem Being Solved: Five specific usability gaps identified through comprehensive dogfooding of codegraph v2.1.0 on itself. These are legitimate issues: missing CLI options that existed in the data layer, inconsistent output formatting, Windows compatibility problems, and stdout pollution.

Why This Approach: All fixes are targeted and conservative. No major architectural changes, just connecting existing functionality to the CLI and improving output consistency.

Risk Assessment: Very low risk. Changes are primarily additive CLI options, display formatting, and fixing missing shebangs.

Critical Concerns

None identified. This is a clean, well-implemented PR that addresses real usability issues without introducing technical debt or risky configuration changes.

Positive observations:

  • Excellent precedence handling in resolveNoTests() helper
  • Proper cycle detection in recursive explain implementation
  • Consistent application of new CLI options across all affected commands
  • Clean separation of concerns (data layer unchanged, only CLI and display improved)
  • All status messages properly moved to stderr

Final Recommendation

  • Rating: ⭐⭐⭐⭐☆ (4/5)
  • Action: APPROVE
  • Reasoning: This is a high-quality implementation of legitimate feature requests from real usage. The code is clean, consistent, and follows established patterns. The only reason it's not 5 stars is that it's incremental improvements rather than groundbreaking functionality, but that's exactly what these dogfood suggestions should be.

Recommendation: Approve and merge. This exemplifies good dogfooding practice - identifying real pain points through usage and addressing them systematically.


@carlos-alm
Copy link
Copy Markdown
Contributor Author

@greptileai

@claude
Copy link
Copy Markdown

claude Bot commented Feb 24, 2026

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


PR Review: feat: implement dogfood report suggestions

Phase 1: Root Cause Analysis

  • Read CLAUDE.md and understand dogfooding context
  • Read the dogfood report to understand what problems were identified
  • Analyze what specific issues this PR addresses

Phase 2: Configuration Change Detection

  • Check for any biome/linting rule changes
  • Verify no test timeouts or coverage thresholds were lowered
  • Review build configuration changes

Phase 3: Test Integrity

  • Examine test modifications for weakened assertions
  • Check for removed or conditional test validation
  • Verify test coverage for new functionality

Phase 4: Code Quality Review

  • Review implementation in cli.js for new options
  • Analyze queries.js changes for coupling scores and explain depth
  • Check config.js for excludeTests option
  • Review builder.js stderr logging changes
  • Examine husky hook shebang fixes

Final Assessment

  • Provide rating and recommendation

Root Cause Analysis

Problem Being Solved: Five specific usability gaps identified through comprehensive dogfooding of codegraph v2.1.0 on itself. These are legitimate issues: missing CLI options that existed in the data layer, inconsistent output formatting, Windows compatibility problems, and stdout pollution.

Why This Approach: All fixes are targeted and conservative. No major architectural changes, just connecting existing functionality to the CLI and improving output consistency.

Risk Assessment: Very low risk. Changes are primarily additive CLI options, display formatting, and fixing missing shebangs.

Critical Concerns

None identified. This is a clean, well-implemented PR that addresses real usability issues without introducing technical debt or risky configuration changes.

Positive observations:

  • Excellent precedence handling in resolveNoTests() helper (src/cli.js:60-64)
  • Proper cycle detection in recursive explain implementation (src/queries.js:1954-1985)
  • Consistent application of new CLI options across all affected commands
  • Clean separation of concerns (data layer unchanged, only CLI and display improved)
  • All status messages properly moved to stderr (verified in src/builder.js)

Final Recommendation

  • Rating: ⭐⭐⭐⭐☆ (4/5)
  • Action: APPROVE
  • Reasoning: This is a high-quality implementation of legitimate feature requests from real usage. The code is clean, consistent, and follows established patterns. The only reason it's not 5 stars is that it's incremental improvements rather than groundbreaking functionality, but that's exactly what these dogfood suggestions should be.

Recommendation: Approve and merge. This exemplifies good dogfooding practice - identifying real pain points through usage and addressing them systematically.


Copy link
Copy Markdown
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.

9 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@carlos-alm carlos-alm merged commit 8a87c05 into main Feb 24, 2026
17 checks passed
@carlos-alm carlos-alm deleted the feat/dogfood-suggestions branch February 24, 2026 02:06
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