Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 35 additions & 5 deletions cli/src/commands/prompt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -619,9 +619,10 @@ impl PromptCommands {
let repo_handle = format!("{}/{}", owner, repo);
formatter.info(&format!("Checking if repository {} exists...", repo_handle));

match client.prompts().get(&repo_handle).await {
let repo_exists = match client.prompts().get(&repo_handle).await {
Ok(_) => {
println!("✓ Repository exists");
true
}
Err(_) => {
formatter.info(&format!(
Expand All @@ -641,14 +642,38 @@ impl PromptCommands {
{
Ok(_) => {
println!("✓ Repository created successfully");
false // New repo, no commits yet
}
Err(e) => {
eprintln!("⚠ Warning: Could not create repository: {}", e);
eprintln!(" Will attempt to push anyway...");
eprintln!(
" Will attempt to push anyway, but auto-parent will be disabled..."
);
false // Repository creation failed; treat as non-existent for auto-parent
}
}
}
}
};

// Determine parent commit for the push
// Automatically fetch latest commit as parent if repo exists
let final_parent_commit = if repo_exists {
formatter.info("Fetching latest commit as parent...");
match client.prompts().get_commit(owner, repo, "latest").await {
Ok(commit_info) => {
println!("✓ Latest commit: {}", commit_info.commit_hash);
Some(commit_info.commit_hash)
}
Err(e) => {
eprintln!("⚠ Warning: Could not fetch latest commit: {}", e);
eprintln!(" Proceeding without parent commit (assuming first commit)");
None
}
}
} else {
formatter.info("New repository, no parent commit needed");
None
};

// Parse input variables
let vars: Vec<String> = if let Some(vars_str) = input_variables {
Expand Down Expand Up @@ -727,7 +752,12 @@ impl PromptCommands {
// Push structured prompt
client
.prompts()
.push_structured_prompt(owner, repo, structured_prompt, None)
.push_structured_prompt(
owner,
repo,
structured_prompt,
final_parent_commit.clone(),
)
.await
.map_err(crate::error::CliError::Sdk)?
} else {
Expand All @@ -741,7 +771,7 @@ impl PromptCommands {
"input_variables": vars,
"template_format": template_format
}),
parent_commit: None,
parent_commit: final_parent_commit,
example_run_ids: None,
};

Expand Down
262 changes: 262 additions & 0 deletions docs/implementation/719-prompt-push-auto-parent-default.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,262 @@
# Plan: Fix Issue #719 - Auto-Parent Commit as Default Behavior

**Issue**: #719 - "prompt push fails with 409 conflict when updating existing prompt"
**Milestone**: #16 (ls-prompt-ux)
**Date**: 2026-01-23

## Context

**Root Cause**: LangSmith API requires parent commit when updating existing prompts
**Current State**: PR #723 added three flags (`--parent-commit`, `--auto-parent`, `--force`) but this is over-engineered

**Key Insight from User**:
> "It's my expectation that 'The push should automatically fetch the latest commit hash and use it as the parent commit' as per the description of #719. That's what happens in git and it's what should happen here."

**Design Document Context**:
The `docs/implementation/ls-prompt-ux-command-redesign.md` outlines a future CRUD redesign (Phase 2, Issue #668.2) where:
- `push` will be split into `create` (new prompts) and `update` (existing prompts)
- This is part of a broader AI-first UX redesign
- Phase 2 is planned but not yet implemented

## Current State Analysis

### What Exists Now
- **PR #723 implementation**: Three flags for parent commit handling
- **Uncommitted changes**: Removed all three flags, made auto-parent the default
- **Code location**: `cli/src/commands/prompt.rs` lines 100-800
- **SDK support**: `get_commit()` method exists and works (added in PR #723)

### Current Behavior After Changes
```rust
// If repo exists → auto-fetch latest commit as parent
// If new repo → no parent needed
let final_parent_commit = if repo_exists {
// Fetch latest commit automatically
match client.prompts().get_commit(owner, repo, "latest").await {
Ok(commit_info) => Some(commit_info.commit_hash),
Err(e) => None // Graceful fallback
}
} else {
None // New repo, no parent needed
};
```

## Decision Point: Minimal Fix vs CRUD Redesign Now

### Option A: Minimal Fix (RECOMMENDED)
Keep the current `push` command, make auto-parent the default.

**Pros:**
- Solves #719 immediately and completely
- Simple, Git-like UX (just works™)
- No breaking changes
- Aligns with future CRUD redesign (can be implemented later in Phase 2)
- Minimal code changes (flags removed, auto-fetch as default)

**Cons:**
- `push` still does both create AND update (slightly confusing)
- Doesn't implement the CRUD redesign yet

### Option B: Implement CRUD Commands Now
Split `push` into `create` and `update` commands as outlined in redesign doc.

**Pros:**
- Implements Phase 2 of redesign immediately
- Clear separation: `create` for new prompts, `update` for existing
- More explicit intent

**Cons:**
- Much larger scope than issue #719
- Requires additional planning and testing
- Breaking change (deprecating `push`)
- Out of scope for current milestone

## Recommended Approach: Option A (Minimal Fix)

**Rationale:**
1. Issue #719 is about fixing 409 conflicts, not redesigning commands
2. The CRUD redesign is planned for Phase 2 (Issue #668.2) - separate work
3. Auto-parent as default solves the problem completely and elegantly
4. Keeps changes focused and testable
5. Doesn't block future CRUD implementation

## Implementation Plan

### Files to Modify

1. **`cli/src/commands/prompt.rs`** (primary changes)
- Lines 100-148: Remove three flag definitions
- Lines 588-593: Remove flag variables from pattern match
- Lines 656-678: Simplify parent commit logic (already done in uncommitted changes)

2. **Tests to Update**
- Any CLI tests that use `--parent-commit`, `--auto-parent`, or `--force` flags
- Verify auto-parent behavior with integration tests

3. **Documentation to Update**
- PR #723 description needs updating
- CHANGELOG.md entry should reflect "auto-parent as default" not "added flags"

### Detailed Changes

#### 1. Remove Flag Definitions (DONE in uncommitted changes)
```rust
// REMOVE these lines from Push struct:
/// Parent commit hash for updates (resolves 409 conflicts)
#[arg(long, value_name = "HASH", conflicts_with = "auto_parent")]
parent_commit: Option<String>,

/// Automatically fetch and use latest commit as parent
#[arg(long, conflicts_with = "parent_commit")]
auto_parent: bool,

/// Force push without parent commit validation (use with caution)
#[arg(long, conflicts_with_all = ["parent_commit", "auto_parent"])]
force: bool,
```

#### 2. Simplify Parent Commit Logic (DONE in uncommitted changes)
```rust
// Replace complex if/else with simple auto-fetch logic
let final_parent_commit = if repo_exists {
formatter.info("Fetching latest commit as parent...");
match client.prompts().get_commit(owner, repo, "latest").await {
Ok(commit_info) => {
println!("✓ Latest commit: {}", commit_info.commit_hash);
Some(commit_info.commit_hash)
}
Err(e) => {
eprintln!("⚠ Warning: Could not fetch latest commit: {}", e);
eprintln!(" Proceeding without parent commit (assuming first commit)");
None
}
}
} else {
formatter.info("New repository, no parent commit needed");
None
};
```

#### 3. Update Tests
Search for any tests using the removed flags and update them:
```bash
# Find tests that might use the flags
grep -r "parent-commit\|auto-parent\|force" sdk/tests/ cli/tests/
```

### Behavior Specification

**For new prompts:**
```bash
$ langstar prompt push -o owner -r new-prompt -t "template"
ℹ Checking if repository owner/new-prompt exists...
ℹ Repository not found, creating owner/new-prompt...
✓ Repository created successfully
ℹ New repository, no parent commit needed
ℹ Pushing prompt to owner/new-prompt...
✓ Pushed successfully
```

**For existing prompts:**
```bash
$ langstar prompt push -o owner -r existing-prompt -t "updated template"
ℹ Checking if repository owner/existing-prompt exists...
✓ Repository exists
ℹ Fetching latest commit as parent...
✓ Latest commit: abc123def456
ℹ Pushing prompt to owner/existing-prompt...
✓ Pushed successfully (commit: def789ghi012)
```

**Error handling (graceful fallback):**
```bash
$ langstar prompt push -o owner -r existing-prompt -t "template"
ℹ Checking if repository owner/existing-prompt exists...
✓ Repository exists
ℹ Fetching latest commit as parent...
⚠ Warning: Could not fetch latest commit: API error: 404
Proceeding without parent commit (assuming first commit)
ℹ Pushing prompt to owner/existing-prompt...
✓ Pushed successfully
```

## Verification Plan

### 1. Manual Testing
```bash
# Test 1: Create new prompt (should work without parent)
langstar prompt push -o "-" -r test-new-prompt -t "Hello {name}"

# Test 2: Update existing prompt (should auto-fetch parent)
langstar prompt push -o "-" -r test-new-prompt -t "Hi {name}, how are you?"

# Test 3: Verify no 409 errors on update
langstar prompt push -o "-" -r test-new-prompt -t "Greetings {name}"
```

### 2. Automated Tests
```bash
# Run full test suite
cargo nextest run --profile ci --all-features --workspace

# Focus on prompt tests
cargo nextest run --profile ci test_push test_get_commit
```

### 3. Validation Criteria
- ✅ No 409 errors when updating existing prompts
- ✅ New prompts created without parent commit
- ✅ Existing prompts updated with auto-fetched parent
- ✅ Graceful fallback if parent fetch fails
- ✅ All existing tests pass
- ✅ Help text updated (no mention of removed flags)

## Commit Strategy

### Commit Message
```
✨ feat(prompt): make auto-parent default behavior for push updates

Resolves 409 conflicts by automatically fetching latest commit as parent
when updating existing prompts, following Git-like UX expectations.

Changes:
- Remove --parent-commit, --auto-parent, and --force flags
- Make parent commit fetching automatic and transparent
- New repos: no parent needed (first commit)
- Existing repos: auto-fetch latest commit as parent
- Graceful error handling with fallback to no parent

Fixes #719

Co-Authored-By: Claude <noreply@anthropic.com>
```

### PR Updates
1. Update PR #723 description to reflect simplified approach
2. Remove validation analysis about flags (no longer applicable)
3. Update test plan to focus on auto-parent behavior
4. Emphasize Git-like UX and simplicity

## Future Work (Out of Scope)

The following are planned but NOT part of this fix:

1. **Phase 2 CRUD Redesign** (Issue #668.2):
- Split `push` into `create` and `update` commands
- Implement progressive disclosure help system
- Add `get` command with modifiers

2. **Advanced Features** (if needed):
- `--force` flag for emergency overrides (only if use case emerges)
- `--parent-commit` for advanced scenarios (only if use case emerges)

## Summary

**Approach**: Minimal fix - make auto-parent the default behavior
**Scope**: Issue #719 only (409 conflict resolution)
**Changes**: Remove flags, simplify logic (already done in uncommitted changes)
**Testing**: Manual + automated verification
**Outcome**: Git-like UX where "push just works" for both create and update

This plan solves #719 completely while staying focused and not over-engineering the solution.
5 changes: 3 additions & 2 deletions sdk/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,9 @@ pub use projects::{
ListProjectsParams, Project, ProjectCreate, ProjectSortColumn, ProjectUpdate, TraceTier,
};
pub use prompts::{
CommitRequest, CommitResponse, LcJson, MessagePromptTemplateKwargs, Prompt, PromptClient,
PromptTemplateKwargs, StructuredOutputKwargs, StructuredPrompt, Visibility,
CommitManifestResponse, CommitRequest, CommitResponse, LcJson, MessagePromptTemplateKwargs,
Prompt, PromptClient, PromptTemplateKwargs, StructuredOutputKwargs, StructuredPrompt,
Visibility,
};
pub use runs::{Cursors, QueryRunsRequest, QueryRunsResponse, Run, RunDateOrder, RunType};
pub use secrets::{SecretKey, SecretUpsert};
Loading