Skip to content

Conversation

@sigmachirality
Copy link
Member

Uploading Screen Recording 2025-11-26 at 4.29.29 PM.mov…

image

@semanticdiff-com
Copy link

semanticdiff-com bot commented Nov 27, 2025

Review changes with  SemanticDiff

Changed Files
File Status
  src/lib/orders/OrderDisplay.tsx  51% smaller
  src/lib/dev.ts  50% smaller
  src/lib/extend/index.tsx  22% smaller
  src/lib/orders/index.tsx  22% smaller
  src/lib/sell/index.tsx  22% smaller
  src/lib/buy/index.tsx  13% smaller
  src/lib/scale/create.tsx  11% smaller
  deno.lock Unsupported file format
  src/lib/sell.ts  0% smaller

@sigmachirality sigmachirality changed the title feat: nudge users from sf buy legacy VMs to sf nodes Node VMs feat: [PRODUCT-607] nudge users from sf buy legacy VMs to sf nodes Node VMs Nov 27, 2025
@sigmachirality sigmachirality self-assigned this Nov 27, 2025
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Nov 27, 2025

Greptile Overview

Greptile Summary

This PR adds deprecation warnings to nudge users from legacy VM commands (sf buy and sf scale) to the newer sf nodes commands. The implementation includes:

  • Fetching zone information to detect VM delivery types
  • State-managed warning prompts with confirmation flows
  • Equivalent command generation to help users migrate
  • Fallback to quote zone when user doesn't specify a zone (prevents price surprises)
  • Improved error message formatting with JSON stringification

Key changes:

  • Added VMWarning component in src/lib/buy/index.tsx that displays a boxen-styled deprecation warning
  • Added ScaleWarning component in src/lib/scale/create.tsx with similar functionality
  • Both files now fetch zone info via API to determine if the target is a VM
  • Warning state machine: "prompt" → "accepted"/"dismissed" → order placement
  • When --yes flag is used, warnings auto-accept and proceed with the order

Minor issues:

  • ANSI color codes embedded in boxen strings may create visual inconsistencies
  • TODO comment about --any-zone support indicates incomplete feature

Confidence Score: 4/5

  • This PR is safe to merge with minor style considerations
  • The changes follow existing patterns in the codebase and implement a straightforward deprecation warning flow. The logic for detecting VMs (type ends with 'v' OR zone.delivery_type === 'VM') is sound. State management for warnings is clear and well-structured. API error handling is properly implemented. The score is 4 rather than 5 due to minor style concerns around ANSI color code usage within boxen and the incomplete --any-zone feature noted in TODOs.
  • No files require special attention - both files follow consistent patterns

Important Files Changed

File Analysis

Filename Score Overview
src/lib/buy/index.tsx 4/5 Added VM deprecation warning with state management, zone fetching for context, and improved error messages with JSON formatting
src/lib/scale/create.tsx 4/5 Added procurement deprecation warning with similar state management pattern, zone fetching, and improved exit handling

Sequence Diagram

sequenceDiagram
    participant User
    participant CLI
    participant BuyOrder/CreateProcurement
    participant API
    participant VMWarning/ScaleWarning
    
    User->>CLI: sf buy -t h100v -n 8 -d 1h
    CLI->>API: GET /v0/zones/{id}
    API-->>CLI: ZoneInfo (delivery_type: VM)
    CLI->>BuyOrder/CreateProcurement: Render with zone info
    
    alt VM type detected (type ends with 'v' OR zone.delivery_type === 'VM')
        BuyOrder/CreateProcurement->>VMWarning/ScaleWarning: Display deprecation warning
        VMWarning/ScaleWarning->>User: Show boxen warning + equivalent command
        
        alt User with --yes flag
            Note over BuyOrder/CreateProcurement: Auto-accept warning (state: "accepted")
            BuyOrder/CreateProcurement->>User: Show order preview
            BuyOrder/CreateProcurement->>API: Place order
        else User without --yes flag
            VMWarning/ScaleWarning->>User: "Place order for legacy VM anyway? (y/n)"
            User->>BuyOrder/CreateProcurement: Input response
            
            alt User responds 'n'
                BuyOrder/CreateProcurement->>User: Exit with message
            else User responds 'y'
                Note over BuyOrder/CreateProcurement: Update state to "accepted"
                BuyOrder/CreateProcurement->>User: Show order preview
                User->>BuyOrder/CreateProcurement: Confirm order (y/n)
                BuyOrder/CreateProcurement->>API: Place order
            end
        end
    else Non-VM type
        BuyOrder/CreateProcurement->>User: Show order preview directly
        User->>BuyOrder/CreateProcurement: Confirm order
        BuyOrder/CreateProcurement->>API: Place order
    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.

2 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

@sigmachirality sigmachirality merged commit 2cacd23 into main Nov 27, 2025
1 check passed
@sigmachirality sigmachirality deleted the dt/warn-legacy-vms branch November 27, 2025 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.

2 participants