feat(phase4): terminal formatter — ranked review path for wild diff#19
Open
avikalpg wants to merge 1 commit into
Open
feat(phase4): terminal formatter — ranked review path for wild diff#19avikalpg wants to merge 1 commit into
avikalpg wants to merge 1 commit into
Conversation
TerminalFormatter renders a schema v2 DiffGraph dict as a priority-ranked terminal output. This is the main user-facing change in DiffGraph v2: 'wild diff' stops opening a browser and instead prints a ranked review path to the terminal. Key features: - Three-bucket ranking: REVIEW FIRST (imported by changed files), REVIEW NEXT (isolated changes), CONTEXT (unchanged symbols in touched files) - Score = importer_count * 3 + lines_changed; evidence-based, LLM-free - Symbol truncation cap (default 10) with --all override - ANSI color with NO_COLOR and TTY auto-detection - --compact flag omits CONTEXT section - File-level fallback when symbol extraction unavailable - Footer shows analysis tier, languages, duration, diff context - 14 unit tests; all pass; pure functions, no git/network Wires as 'wild diff --format terminal'. Default swap (browser → terminal) pending B1 answer from Avikalp. Can ship as --format terminal opt-in today. Spec: docs/DiffGraph-CLI/design/TERMINAL-FORMATTER.md
Contributor
WalkthroughAdds a schema v2 terminal formatter that ranks symbols by review priority, renders colored or plain terminal output, supports compact and truncation options, and includes tests for ranking and rendering behavior. ChangesTerminal formatter
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant DiffGraph
participant TerminalFormatter
participant TerminalOutput
DiffGraph->>TerminalFormatter: provide schema v2 dictionary
TerminalFormatter->>TerminalFormatter: rank symbols and build sections
TerminalFormatter->>TerminalOutput: write formatted review path
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
tests/test_terminal_formatter.py (2)
369-374: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer unpacking over list concatenation (Ruff RUF005).
🔧 Proposed fix
- files = importer_files + [target_file] + files = [*importer_files, target_file]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_terminal_formatter.py` around lines 369 - 374, Update the files construction near importer_files and target_file to use iterable unpacking rather than list concatenation, preserving the existing order and resulting list contents.Source: Linters/SAST tools
71-77: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winImplicit
Optionalon_make_diffgraphparams (Ruff RUF013).
files,symbols,relationships,metadata, anddiff_refare typed as barelist/dictwithNonedefaults, which PEP 484 prohibits implicitly.🔧 Proposed fix
+from typing import Optional + def _make_diffgraph( - files: list = None, - symbols: list = None, - relationships: list = None, - metadata: dict = None, - diff_ref: dict = None, + files: Optional[list] = None, + symbols: Optional[list] = None, + relationships: Optional[list] = None, + metadata: Optional[dict] = None, + diff_ref: Optional[dict] = None, ) -> dict:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_terminal_formatter.py` around lines 71 - 77, Update the `_make_diffgraph` parameters `files`, `symbols`, `relationships`, `metadata`, and `diff_ref` to explicitly use nullable `Optional` type annotations while retaining their existing `None` defaults and return type.Source: Linters/SAST tools
diffgraph/formatters/terminal.py (1)
307-337: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
section_styleparameter is accepted but never used.
_write_sectionhardcodes header styling viaif title == "REVIEW FIRST"/elif title == "CONTEXT"branches, ignoring thesection_styleargument passed by every caller (Line 155-158). This currently renders correctly only because the branches happen to match the caller-supplied styles — any future call with a differentsection_stylewould be silently ignored.♻️ Use the passed style instead of re-deriving it from title
+_STYLE_FNS = {"bold_yellow": bold_yellow, "bold": bold, "dim": dim} + def _write_section( self, title: str, items: list[_RankedSymbol], out, color: bool, section_style: str = "bold", ) -> None: ... prefix = "▶ " if not _is_dumb_terminal() else "> " header_text = f"{prefix}{title}" - if title == "REVIEW FIRST": - header = bold_yellow(header_text, color) - elif title == "CONTEXT": - header = dim(header_text, color) - else: - header = bold(header_text, color) + header = _STYLE_FNS.get(section_style, bold)(header_text, color)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@diffgraph/formatters/terminal.py` around lines 307 - 337, Update _write_section to derive the header styling from its section_style parameter instead of matching title values. Preserve the existing bold-yellow, dim, and bold behavior by mapping the caller-provided styles accordingly, and ensure all callers’ current rendering remains unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@diffgraph/formatters/terminal.py`:
- Around line 307-337: Update _write_section to derive the header styling from
its section_style parameter instead of matching title values. Preserve the
existing bold-yellow, dim, and bold behavior by mapping the caller-provided
styles accordingly, and ensure all callers’ current rendering remains unchanged.
In `@tests/test_terminal_formatter.py`:
- Around line 369-374: Update the files construction near importer_files and
target_file to use iterable unpacking rather than list concatenation, preserving
the existing order and resulting list contents.
- Around line 71-77: Update the `_make_diffgraph` parameters `files`, `symbols`,
`relationships`, `metadata`, and `diff_ref` to explicitly use nullable
`Optional` type annotations while retaining their existing `None` defaults and
return type.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f9c5dc8d-8d25-4b84-ab00-8df54579687c
📒 Files selected for processing (3)
diffgraph/formatters/__init__.pydiffgraph/formatters/terminal.pytests/test_terminal_formatter.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 4 — Terminal Formatter
Status: 14 tests passing · Standalone — no dependency on Phases 1–3
Spec:
docs/DiffGraph-CLI/design/TERMINAL-FORMATTER.mdRoadmap:
docs/DiffGraph-CLI/design/V2-IMPLEMENTATION-ROADMAP.mdPhase 4What this does
Adds
TerminalFormatterindiffgraph/formatters/terminal.py.When wired as
wild diff --format terminal, instead of openingdiffgraph.htmlin a browser, the tool prints a priority-ranked review path to the terminal:Files changed
diffgraph/formatters/__init__.pydiffgraph/formatters/terminal.pyTerminalFormatterclass (430 lines)tests/test_terminal_formatter.pyHow the ranking works
Three-bucket ranking, fully evidence-based (no LLM required):
Score formula:
importers × 3 + lines_changed— importers weighted heavier than lines.Acceptance criteria (all met)
diffgraph/formatters/terminal.pycreated withTerminalFormatter_rank_symbols()is a pure function (14 tests pass; no stdout capture needed)--compactomits CONTEXT section--alldisables 10-symbol truncation capNO_COLOR=1and piped stdout suppress ANSIB1 decision point
The formatter ships as
wild diff --format terminal(opt-in). Whether this becomes the default output depends on B1:cli.pydefault fromhtmltoterminal(1-line change)This PR is mergeable regardless of B1. The B1 change is a trivial 1-line follow-up.
Phase context
This is Phase 4 of the V2 implementation roadmap. It is standalone — it does not depend on Phases 1, 2, or 3 merging first. The formatter is a pure consumer of any schema v2 dict and can be reviewed and merged independently.
The critical unblocked path:
Summary by CodeRabbit
New Features
Bug Fixes