Skip to content

Conversation

@sigmachirality
Copy link
Member

@sigmachirality sigmachirality commented Nov 27, 2025

Finishes and expands on #225

@semanticdiff-com
Copy link

semanticdiff-com bot commented Nov 27, 2025

Review changes with  SemanticDiff

Changed Files
File Status
  src/lib/nodes/utils.ts  27% smaller
  src/lib/nodes/list.tsx  8% smaller

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Nov 27, 2025

Greptile Overview

Greptile Summary

Added filtering and pagination capabilities to the sf nodes list command. Users can now filter nodes by status using --status and limit the number of displayed nodes using --limit (default: 12).

Key changes:

  • Extracted DEFAULT_NODE_LS_LIMIT constant for consistent default behavior
  • Implemented --status filter accepting multiple status values (pending, awaitingcapacity, running, released, failed, terminated)
  • Added --limit option with pagination support showing overflow message when nodes exceed limit
  • Updated help text with practical examples demonstrating new filtering capabilities
  • Filter applies before display in all output modes (table, verbose, JSON)

Confidence Score: 4/5

  • Safe to merge with minor style improvement opportunity
  • The implementation is solid with proper validation and consistent behavior across output modes. One minor style improvement identified (redundant check in filter callback) that doesn't affect functionality. The feature additions align well with CLI best practices and include helpful documentation.
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
src/lib/nodes/list.tsx 4/5 Added --status and --limit filters to nodes list command with proper help text and validation
src/lib/nodes/utils.ts 5/5 Extracted DEFAULT_NODE_LS_LIMIT constant and updated createNodesTable to support limit parameter with overflow message

Sequence Diagram

sequenceDiagram
    participant User
    participant CLI as sf nodes list
    participant API as Nodes API
    participant Filter as Filter Logic
    participant Display as Display Logic

    User->>CLI: sf nodes list --status pending running --limit 100
    CLI->>API: client.nodes.list()
    API-->>CLI: Return all nodes
    CLI->>Filter: Apply status filter
    Filter->>Filter: Check if options.status?.length
    Filter->>Filter: Filter nodes by status (pending, running)
    Filter-->>CLI: Return filteredNodes
    CLI->>Display: Check output format
    alt JSON output (--json)
        Display->>User: JSON.stringify(filteredNodes)
    else Verbose output (--verbose)
        Display->>Display: Slice filteredNodes to limit
        Display->>User: Render NodesVerboseDisplay
    else Table output (default)
        Display->>Display: createNodesTable(filteredNodes, limit)
        Display->>User: Print table with limit message
        Display->>User: Print "Found X nodes total" message
    end
Loading

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.

6 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@sigmachirality sigmachirality changed the title feat: [PRODUCT-608] add filter on sf nodes list feat: [PRODUCT-608] [ENG-2210] add filter on sf nodes list Nov 27, 2025
@sigmachirality sigmachirality changed the title feat: [PRODUCT-608] [ENG-2210] add filter on sf nodes list feat: [PRODUCT-608], [ENG-2210] add filter on sf nodes list Nov 27, 2025
@sigmachirality
Copy link
Member Author

@greptileai review this again

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.

2 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@sigmachirality sigmachirality merged commit 3a92a08 into main Nov 27, 2025
1 check passed
@sigmachirality sigmachirality deleted the dt/filter-node-statuses-in-memory branch November 27, 2025 03:38
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.

3 participants