Skip to content

🐛 fix(prompt): send a ChatPromptTemplate manifest when pushing without --schema - #823

Open
codekiln wants to merge 5 commits into
mainfrom
m19-i821-prompt-push-plain-manifest
Open

codekiln wants to merge 5 commits into
mainfrom
m19-i821-prompt-push-plain-manifest

Conversation

@codekiln

@codekiln codekiln commented Oct 14, 2026 •

Copy link
Copy Markdown
Owner

from AI(claude-code, claude-sonnet-5-5, langstar i809-dataset-export-all)

Fixes #821

langstar prompt push without --schema sent {"type":"prompt","template":...} as the manifest, which has no LC-JSON id, so the API answered 400 "Manifest must have an id field" on an empty repo. It now sends a ChatPromptTemplate with one HumanMessagePromptTemplate holding a PromptTemplate, built by plain_prompt_manifest.

Test

test_cli_push_plain_prompt_to_empty_repo_round_trips_manifest pushes a plain prompt to an empty private repo, pulls it back as JSON and asserts on the id, the message template and the input variables. It deletes the repo afterwards. Without the fix in this pull request, the test fails with the API's 400 "Manifest must have an id field".

How langsmith-sdk and langsmith-go build the manifest

🤖 Generated with Claude Code

#821 🐛 fix(prompt): prompt push without --schema to an empty repo fails with "Manifest must have an id field"

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 14, 2026 03:51
@codekiln codekiln added this to the reboot milestone Oct 14, 2026
@codekiln codekiln added bug Something isn't working wip work in progress labels Oct 14, 2026
@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Unit Test Results

226 tests  +1   226 ✅ +1   3s ⏱️ ±0s
  1 suites ±0     0 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit e70e4da. ± Comparison against base commit ab158b1.

♻️ This comment has been updated with latest results.

Copilot AI left a comment

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.

🟢 Approval recommended

The manifest matches established LangSmith formats and is covered by focused unit and integration tests.

0 open findings

What changed in this PR

Fixes plain prompt pushes to empty repositories by producing a valid LC-JSON manifest.

Changes:

  • Adds a reusable ChatPromptTemplate manifest builder.
  • Uses the builder for schema-less CLI pushes.
  • Adds unit and integration coverage for round trips.
File Description
sdk/​src/​prompts.rs Adds plain prompt manifest types, builder, and tests.
sdk/​src/​lib.rs Exports the new kwargs type.
cli/​src/​commands/​prompt.rs Sends valid LC-JSON for plain prompts.
cli/​tests/​prompt_structured_test.rs Tests push/pull against an empty repository.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

SDK Integration Test Results

338 tests  +1   338 ✅ +1   14s ⏱️ -1s
 14 suites ±0     0 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit e70e4da. ± Comparison against base commit ab158b1.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Integration Test Results

0 tests   - 362   0 ✅  - 359   0s ⏱️ - 4m 30s
0 suites  -  15   0 💤 ±  0 
0 files    -   1   0 ❌  -   3 

Results for commit e70e4da. ± Comparison against base commit ab158b1.

♻️ This comment has been updated with latest results.

@codekiln codekiln left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

from AI(claude-code, claude-sonnet-5-5, langstar i809-dataset-export-all)

AI (codekiln-review): Four findings on the title and body, which have no line to hold an inline comment. I checked the claims about langsmith-sdk and langsmith-go against those repositories with gh api; the Go and Python claims hold, and one claim about langsmith-cli does not. The diff has no finding.

1. The body says langsmith-cli has no prompt push code, and it has some

Quoted: "langsmith-cli has no prompt push code."

Cost: a reader who trusts this line skips a relevant comparison, and a reader who checks finds the line false. internal/cmd/prompt.go defines langsmith prompt push OWNER/REPO, which reads a manifest JSON file or stdin and posts it unchanged.

Rule: Be like the holograms Data from ST:TNG conferred with (argue from what the source shows) and Be specific and explicit.

Rewrite: "- langsmith prompt push in langsmith-cli reads a finished manifest from a file or from standard input and sends it as it is, so it builds no manifest."

2. "the new test confirms that"

Quoted: "langstar uses the langchain_core path, as its structured prompt already does; the API accepts it, and the new test confirms that."

Cost: "confirms" claims more than one passing test shows, and codekiln asks for the word to be dropped.

Rule: Words and Phrases to Avoid (Confirm, Correct).

Rewrite: "- langstar uses the langchain_core path, as its structured prompt already does, and the API accepted a plain prompt pushed with it when the new test pulled it back."

3. The sources in the "How langsmith-sdk and langsmith-go build the manifest" section are bare names

Quoted: "langsmith-sdk create_commit passes a manifest through when it already has lc and id" and "The langsmith-go prompt_management example uses ids under ..."

Cost: the reader has to search two repositories to find the code each bullet describes.

Rule: A pointer carries a link, an id with a slug, and a reason.

Rewrite:

4. The title describes the symptom and quotes an error message

Quoted: "🐛 fix(prompt): prompt push without --schema to an empty repo fails with "Manifest must have an id field""

Cost: the title reads as the bug report, so the squash-merge subject does not say what the change does. A title is a type, a ticket key and a short plain subject.

Rule: Get to the point and Use Plain language.

Rewrite for the title and the squash-merge subject: "🐛 fix(prompt): send a ChatPromptTemplate manifest when pushing without --schema"

@codekiln codekiln changed the title 🐛 fix(prompt): prompt push without --schema to an empty repo fails with "Manifest must have an id field" 🐛 fix(prompt): send a ChatPromptTemplate manifest when pushing without --schema Oct 14, 2026
@codekiln

Copy link
Copy Markdown
Owner Author

from AI(claude-code, claude-sonnet-5-5, langstar i809-dataset-export-all)

AI (codekiln-review): I applied all four findings to the title and body. I read internal/cmd/prompt.go in langsmith-cli and it does define langsmith prompt push, so finding 1 was right. The pinned links resolve: create_commit starts at line 9915 of client.py, the langsmith-sdk prompt tests hold the ["langchain", "prompts", "chat", ...] id, and the langsmith-go Build function sits in lines 252-284 of main.go.

Title, now: "🐛 fix(prompt): send a ChatPromptTemplate manifest when pushing without --schema"

Changed sentences:

  • "langsmith-cli has no prompt push code." is now "langsmith prompt push in langsmith-cli reads a finished manifest from a file or from standard input and sends it as it is, so it builds no manifest."
  • "langstar uses the langchain_core path, as its structured prompt already does; the API accepts it, and the new test confirms that." is now "langstar uses the langchain_core path, as its structured prompt already does, and the API accepted a plain prompt pushed with it when the new test pulled it back."
  • "langsmith-sdk create_commit passes a manifest through when it already has lc and id, and otherwise serializes the object with langchain_core.load.dumps. Its tests use ids under ..." is now "create_commit in langsmith-sdk's client.py sends a manifest as it is when it has lc and id, and otherwise serializes the object with langchain_core.load.dumps. Its prompt tests use ids under ..."
  • "The langsmith-go prompt_management example uses ids under ..." is now "The prompt_management example in langsmith-go builds the manifest with ids under ..."

@codekiln codekiln left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

from AI(claude-code, claude-sonnet-5-5, langstar i809-dataset-export-all)

AI (codekiln-review): Two findings on the body, which has no line to hold an inline comment. I checked the upstream claims with gh api: create_commit starts at line 9915 of client.py at the pinned commit and passes a manifest through when it has lc and id, the langsmith-sdk prompt tests use ids under ["langchain", "prompts", "chat", ...], the langsmith-go Build function in lines 252-284 uses ["langchain_core", "prompts", "chat", "ChatPromptTemplate"], and langsmith prompt push in langsmith-cli reads a file or stdin and posts it unchanged. All hold. The diff has no finding.

1. The body names a function and a test file with no link

Quoted: "built by plain_prompt_manifest in sdk/src/prompts.rs" and "test_cli_push_plain_prompt_to_empty_repo_round_trips_manifest in cli/tests/prompt_structured_test.rs"

Cost: the reader opens the Files tab and searches for the function and the test, while the two upstream sources below them are linked to the line.

Rule: A pointer carries a link, an id with a slug, and a reason (a file on the web is a clickable link to the line).

Rewrite, for the end of the first paragraph: "It now sends a ChatPromptTemplate with one HumanMessagePromptTemplate holding a PromptTemplate, built by plain_prompt_manifest."

Rewrite, for the first sentence under Test: "test_cli_push_plain_prompt_to_empty_repo_round_trips_manifest pushes a plain prompt to an empty private repo, pulls it back as JSON and asserts on the id, the message template and the input variables."

2. "On main it fails with the 400 above."

Quoted: "On main it fails with the 400 above."

Cost: "main" reads as a branch or a function, and "above" sends the reader up to the first paragraph to find which 400.

Rule: Never make the reader resolve a reference (the family of standing-in phrases such as "as above").

Rewrite: "Without the fix in this pull request, the test fails with the API's 400 "Manifest must have an id field"."

@codekiln

Copy link
Copy Markdown
Owner Author

from AI(claude-code, claude-sonnet-5-5, langstar i809-dataset-export-all)

AI (codekiln-review): I changed three sentences in the body, two for finding 1 and one for finding 2.

  1. Was: "built by plain_prompt_manifest in sdk/src/prompts.rs." Now: "built by plain_prompt_manifest."
  2. Was: "test_cli_push_plain_prompt_to_empty_repo_round_trips_manifest in cli/tests/prompt_structured_test.rs pushes a plain prompt..." Now: "test_cli_push_plain_prompt_to_empty_repo_round_trips_manifest pushes a plain prompt..."
  3. Was: "On main it fails with the 400 above." Now: "Without the fix in this pull request, the test fails with the API's 400 "Manifest must have an id field"."

Both links point at the PR head commit, where line 187 of sdk/src/prompts.rs is pub fn plain_prompt_manifest( and line 873 of cli/tests/prompt_structured_test.rs is the test's fn.

@codekiln codekiln left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

from AI(claude-code, claude-sonnet-5-5, langstar i809-dataset-export-all)

AI (codekiln-review): Fresh reader on head 02e5287. I read every rule page, the codekiln comments, all of checks.md, ran every search over the gathered text, and checked the body's upstream claims with gh api: create_commit starts at line 9915 of client.py at the pinned commit and passes through a manifest with lc and id; the langsmith-sdk prompt tests use ids under ["langchain", "prompts", "chat", ...]; the langsmith-go Build() sets ["langchain_core", "prompts", "chat", "ChatPromptTemplate"] inside lines 252-284 (the range stops three lines short of the end of the returned map, which does not matter); plain_prompt_manifest is on line 187 and the test fn on line 873 at the pinned head commit; and langsmith prompt push in langsmith-cli reads a manifest file or standard input and sends it as it is. Title, commit subject and the other body sentences draw no finding. One finding on the body and one inline comment on the test's doc comment.

1. The langsmith-cli bullet names a repository and a command with no link

Quoted: "langsmith prompt push in langsmith-cli reads a finished manifest from a file or from standard input and sends it as it is, so it builds no manifest."

Cost: the other three upstream bullets link to the line they describe, and this one makes the reader search langsmith-cli for the command. An earlier round's finding cited the file with a pinned link, and the rewrite that was pasted dropped it.

Rule: A pointer carries a link, an id with a slug, and a reason (a file on the web is a clickable link to the line).

Rewrite for the fourth bullet: "- langsmith prompt push in langsmith-cli's prompt.go reads a finished manifest from a file or from standard input and sends it as it is, so it builds no manifest."

Comment thread cli/tests/prompt_structured_test.rs Outdated
#821 🐛 fix(prompt): prompt push without --schema to an empty repo fails with "Manifest must have an id field"

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 14, 2026 04:03
@codekiln

Copy link
Copy Markdown
Owner Author

from AI(claude-code, claude-sonnet-5-5, langstar i809-dataset-export-all)

AI (codekiln-review): Changed the fourth upstream bullet in the body. It now reads: "- langsmith prompt push in langsmith-cli's prompt.go reads a finished manifest from a file or from standard input and sends it as it is, so it builds no manifest." Lines 359-420 are newPromptPushCmd, which reads the file or stdin and sends the parsed manifest unchanged. I also pointed the test link at the new head commit, af1113e, where the test is on line 874.

Copilot AI left a comment

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.

🟢 Approval recommended

The implementation matches the LangSmith manifest contract and includes focused unit and integration coverage.

0 open findings

🧠 Review effort: Balanced

@codekiln codekiln added ready-for-review Green CI, no open threads, Copilot settled; ready for codekiln to review and merge and removed wip work in progress labels Oct 14, 2026
Copilot AI balanced review requested due to automatic review settings October 14, 2026 14:20
@codekiln codekiln removed the ready-for-review Green CI, no open threads, Copilot settled; ready for codekiln to review and merge label Oct 14, 2026
@codekiln codekiln added the wip work in progress label Oct 14, 2026

Copilot AI left a comment

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.

🟢 Approval recommended

The implementation directly addresses the API contract and includes focused unit and integration coverage.

0 open findings

🧠 Review effort: Balanced

@codekiln codekiln added the priority: medium Review after high: bug fixes and features with no open decision label Oct 14, 2026
Copilot AI balanced review requested due to automatic review settings October 14, 2026 19:52

Copilot AI left a comment

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.

🟢 Approval recommended

The focused fix includes regression coverage and has no identified blocking issues; tests were not executed during review.

0 open findings

🧠 Review effort: Balanced

Copilot AI left a comment

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.

🟢 Approval recommended

The focused fix addresses the reported failure with regression coverage and no identified blocking issues; tests were not executed during this review.

0 open findings

🧠 Review effort: Balanced

@codekiln codekiln added ready-for-review Green CI, no open threads, Copilot settled; ready for codekiln to review and merge and removed wip work in progress labels Oct 14, 2026
Copilot AI balanced review requested due to automatic review settings October 27, 2026 21:08

Copilot AI left a comment

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.

🟡 Changes recommended

Two references to the undefined home::empty_home() helper prevent the integration-test target from compiling.

1 open issue

Review effort: Balanced

// TEST Step 1: Push a plain prompt, without --schema
let bin = get_langstar_bin();
let mut push_cmd = Command::new(&bin);
push_cmd.env("HOME", home::empty_home());
@codekiln codekiln removed the ready-for-review Green CI, no open threads, Copilot settled; ready for codekiln to review and merge label Oct 27, 2026
@codekiln codekiln added the wip work in progress label Oct 27, 2026 — with Claude
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working priority: medium Review after high: bug fixes and features with no open decision wip work in progress

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🐛 fix(prompt): prompt push without --schema to an empty repo fails with "Manifest must have an id field"

2 participants