-
Notifications
You must be signed in to change notification settings - Fork 335
Kiln CLI including project packager #877
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds a Typer-based CLI (entrypoint, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as kiln_ai CLI (Typer)
participant Loader as Project Loader
participant Validator as RunConfig & Tool Validator
participant PromptBuilder as Prompt Builder
participant FS as Filesystem / Export
User->>CLI: run `kiln_ai package_project [path] [--tasks|--all]`
CLI->>Loader: load project(s) from path
Loader-->>CLI: Project object(s) or error
CLI->>CLI: parse task ids / --all flag
CLI->>Validator: validate tasks & get default run configs
Validator-->>CLI: validation result (ok / tools found)
alt blocking tools found
CLI-->>User: print error and exit
else no blocking tools
CLI->>PromptBuilder: build prompts for tasks
PromptBuilder-->>CLI: Prompt objects (or request confirmations)
alt dynamic prompts detected
CLI->>User: prompt for confirmation
User-->>CLI: confirm/abort
end
CLI->>FS: prepare export structure (tasks, run_configs, prompts, servers)
FS-->>CLI: write files and create zip
CLI-->>User: print success and artifact path
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📓 Common learnings🔇 Additional comments (1)
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 |
📊 Coverage ReportOverall Coverage: 92% Diff: origin/main...HEAD
Summary
Line-by-lineView line-by-line diff coveragelibs/core/kiln_ai/cli/commands/package_project.pyLines 57-70 57 raise typer.Exit(1)
58
59 try:
60 return Project.load_from_file(project_path)
! 61 except Exception as e:
! 62 print_projects_table(title="Available Projects")
! 63 console.print(
64 f"\n[red]Error:[/red] Failed to load project: {e}\nYou must provide this argument (see list above for available projects).\n"
65 )
! 66 raise typer.Exit(1)
67
68
69 def print_tasks_table(tasks: list[Task], title: str = "Available Tasks") -> None:
70 """Print a table of tasks with their IDs and names."""Lines 176-188 176 }
177
178 for task_id in task_ids:
179 if task_id not in task_id_to_task:
! 180 continue
181
182 task = task_id_to_task[task_id]
183 if not task.default_run_config_id:
! 184 continue
185
186 run_configs = task.run_configs()
187 run_config = next(
188 (rc for rc in run_configs if rc.id == task.default_run_config_id), NoneLines 187-195 187 run_config = next(
188 (rc for rc in run_configs if rc.id == task.default_run_config_id), None
189 )
190 if not run_config:
! 191 continue
192
193 tools = get_tools_from_run_config(run_config)
194 for tool_id in tools:
195 if tool_id.startswith(KILN_TASK_TOOL_ID_PREFIX):Lines 248-256 248
249 for task in tasks:
250 run_config = run_configs.get(task.id) # type: ignore
251 if not run_config:
! 252 continue
253
254 tools = get_tools_from_run_config(run_config)
255 for tool_id in tools:
256 tool_type = classify_tool_id(tool_id)Lines 310-318 310 console.print(
311 "\nThese remote MCP tools may require configuring secrets like API keys or other credentials. This must be done on any machine you run this task on."
312 )
313 if not typer.confirm("Continue?"):
! 314 raise typer.Exit(0)
315
316
317 def collect_required_tool_servers(
318 tasks: list[Task], run_configs: dict[str, TaskRunConfig]Lines 330-338 330
331 for task in tasks:
332 run_config = run_configs.get(task.id) # type: ignore
333 if not run_config:
! 334 continue
335
336 tools = get_tools_from_run_config(run_config)
337 for tool_id in tools:
338 tool_type = classify_tool_id(tool_id)Lines 402-414 402 try:
403 builder = prompt_builder_from_id(prompt_id, task)
404 prompt = build_prompt_from_builder(builder)
405 task_prompts[task.id] = prompt # type: ignore
! 406 except Exception as e:
! 407 console.print(
408 f"[red]Error:[/red] Failed to build prompt for task '{task.name}': {e}"
409 )
! 410 raise typer.Exit(1)
411
412 return task_prompts
413 Lines 437-449 437 Returns:
438 Tuple of (exported_task, exported_run_config)
439 """
440 if task.path is None:
! 441 raise ValueError(f"Task '{task.name}' path is not set")
442 if run_config.path is None:
! 443 raise ValueError(f"Run config '{run_config.name}' path is not set")
444 if exported_project.path is None:
! 445 raise ValueError("Exported project path is not set")
446
447 task_folder_name = task.path.parent.name
448 run_config_folder_name = run_config.path.parent.nameLines 487-495 487 ):
488 saved = True
489 break
490 if not saved:
! 491 raise ValueError(
492 f"Prompt saved to incorrect location: {prompt.path} != {exported_task_prompt.path}"
493 )
494
495 exported_run_config.run_config_properties.prompt_id = f"id::{prompt.id}"Lines 509-517 509 exported_run_config = exported_run_configs[task_id]
510
511 exported_prompts_list = exported_task.prompts()
512 if not exported_prompts_list:
! 513 raise ValueError(
514 f"No prompts found for exported task '{exported_task.name}'"
515 )
516
517 rebuilt_prompts = validate_and_build_prompts(Lines 528-536 528 if (
529 rebuilt_prompt.chain_of_thought_instructions
530 != original_prompt.chain_of_thought_instructions
531 ):
! 532 raise ValueError(
533 f"Chain of thought mismatch for task '{exported_task.name}': "
534 f"exported prompt does not match original"
535 )Lines 550-564 550 if not server_ids:
551 return
552
553 if exported_project.path is None:
! 554 raise ValueError("Exported project path is not set")
555
556 for server in project.external_tool_servers():
557 if server.id not in server_ids:
! 558 continue
559 if server.path is None:
! 560 raise ValueError(f"Server '{server.name}' path is not set")
561
562 folder_name = server.path.parent.name
563 server_dir = (
564 exported_project.path.parent / "external_tool_servers" / folder_nameLines 623-632 623
624 # 2. Parse and validate task IDs
625 available_tasks = project.tasks()
626 if not available_tasks:
! 627 console.print("[red]Error:[/red] Project has no tasks.")
! 628 raise typer.Exit(1)
629
630 task_ids = parse_task_ids(tasks, all_tasks, available_tasks)
631
632 # 3. Collect additional task IDs from kiln_task tools (sub-tasks)libs/core/kiln_ai/cli/commands/projects.pyLines 23-46 23 "Use the Kiln web UI to create a project, or add one with 'kiln_ai projects add <path>'"
24 )
25 return 0
26
! 27 table = Table(title=title)
! 28 table.add_column("ID")
! 29 table.add_column("Name", style="cyan", no_wrap=True)
! 30 table.add_column("Path", style="dim")
31
! 32 loaded_count = 0
! 33 for project_path in project_paths:
! 34 try:
! 35 project = Project.load_from_file(project_path)
! 36 table.add_row(project.id, project.name, str(project_path))
! 37 loaded_count += 1
! 38 except Exception:
! 39 table.add_row("[red]<failed to load>[/red]", "", str(project_path))
40
! 41 console.print(table)
! 42 return loaded_count
43
44
45 @app.command("list")
46 def list_projects() -> None:Lines 44-52 44
45 @app.command("list")
46 def list_projects() -> None:
47 """List all configured projects."""
! 48 loaded_count = print_projects_table()
! 49 if loaded_count == 0:
! 50 raise typer.Exit(0)
! 51 console.print(f"\n[green]{loaded_count}[/green] project(s) loaded")libs/core/kiln_ai/cli/commands/tasks.pyLines 16-32 16
17 Returns:
18 Project if found and loaded successfully, None otherwise.
19 """
! 20 project_paths = Config.shared().projects
! 21 for project_path in project_paths:
! 22 try:
! 23 project = Project.load_from_file(project_path)
! 24 if project.id == project_id:
! 25 return project
! 26 except Exception:
! 27 continue
! 28 return None
29
30
31 def load_project(project_ref: str) -> Project:
32 """Load a project by path or ID.Lines 39-62 39
40 Raises:
41 typer.Exit: If project cannot be found or loaded.
42 """
! 43 path = Path(project_ref)
! 44 if path.exists():
! 45 try:
! 46 return Project.load_from_file(path)
! 47 except Exception as e:
! 48 console.print(f"[red]Error loading project: {e}[/red]")
! 49 raise typer.Exit(1)
50
! 51 project = load_project_by_id(project_ref)
! 52 if project is not None:
! 53 return project
54
! 55 console.print(f"[red]Error: Project not found: {project_ref}[/red]")
! 56 console.print("Provide a valid project file path or project ID.")
! 57 console.print("Use 'kiln project list' to see available projects.")
! 58 raise typer.Exit(1)
59
60
61 @app.command("list")
62 def list_tasks(Lines 62-85 62 def list_tasks(
63 project: str = typer.Argument(help="Project file path or project ID"),
64 ) -> None:
65 """List all tasks in a project."""
! 66 loaded_project = load_project(project)
67
! 68 tasks = loaded_project.tasks(readonly=True)
69
! 70 if not tasks:
! 71 console.print("[yellow]No tasks found in this project.[/yellow]")
! 72 raise typer.Exit(0)
73
! 74 table = Table(title=f"Tasks in {loaded_project.name}")
! 75 table.add_column("ID", no_wrap=True)
! 76 table.add_column("Name", style="cyan")
! 77 table.add_column("Description", style="white")
! 78 table.add_column("Path", style="dim")
79
! 80 for task in tasks:
! 81 table.add_row(
82 task.id,
83 task.name,
84 task.description or "",
85 str(task.path) if task.path else "",Lines 84-90 84 task.description or "",
85 str(task.path) if task.path else "",
86 )
87
! 88 console.print(table)
! 89 console.print(f"\n[green]{len(tasks)}[/green] task(s) found")
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
libs/core/kiln_ai/cli/commands/projects.py (1)
32-38: Consider logging project load failures for debugging.The bare exception catch is acceptable for CLI UX (showing
<failed to load>is user-friendly), but silently swallowing errors could make debugging difficult when projects fail to load unexpectedly.for project_path in project_paths: try: project = Project.load_from_file(project_path) table.add_row(project.name, str(project_path)) loaded_count += 1 - except Exception: + except Exception as e: table.add_row("[red]<failed to load>[/red]", str(project_path)) + # Log for debugging if verbose mode is enabled in the future
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
libs/core/README.md(1 hunks)libs/core/kiln_ai/cli/__init__.py(1 hunks)libs/core/kiln_ai/cli/cli.py(1 hunks)libs/core/kiln_ai/cli/commands/package_project.py(1 hunks)libs/core/kiln_ai/cli/commands/projects.py(1 hunks)libs/core/kiln_ai/cli/commands/test_package_project.py(1 hunks)libs/core/pyproject.toml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
libs/core/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/project.mdc)
Use Python 3.10+ for the core library (libs/core) and Python 3.13 for the desktop app
Use python 3.10+ for the core library (libs/core) and python 3.13 for the desktop app
Files:
libs/core/kiln_ai/cli/commands/projects.pylibs/core/kiln_ai/cli/commands/test_package_project.pylibs/core/kiln_ai/cli/__init__.pylibs/core/kiln_ai/cli/commands/package_project.pylibs/core/kiln_ai/cli/cli.py
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/project.mdc)
**/*.py: Use asyncio for asynchronous operations in Python
Use Pydantic v2 (not v1) for data validation
Files:
libs/core/kiln_ai/cli/commands/projects.pylibs/core/kiln_ai/cli/commands/test_package_project.pylibs/core/kiln_ai/cli/__init__.pylibs/core/kiln_ai/cli/commands/package_project.pylibs/core/kiln_ai/cli/cli.py
libs/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use Pydantic v2 (not v1) for data validation and serialization
Files:
libs/core/kiln_ai/cli/commands/projects.pylibs/core/kiln_ai/cli/commands/test_package_project.pylibs/core/kiln_ai/cli/__init__.pylibs/core/kiln_ai/cli/commands/package_project.pylibs/core/kiln_ai/cli/cli.py
**/*test*.py
📄 CodeRabbit inference engine (.cursor/rules/project.mdc)
Use pytest for testing Python code
Use pytest for testing in python projects
Files:
libs/core/kiln_ai/cli/commands/test_package_project.py
🧠 Learnings (1)
📓 Common learnings
Learnt from: scosman
Repo: Kiln-AI/Kiln PR: 733
File: app/web_ui/src/lib/stores/local_storage_store.ts:9-11
Timestamp: 2025-10-21T00:06:57.115Z
Learning: When scosman is refactoring code by moving it to a new location, he prefers to keep the moved code unchanged and not mix in functional improvements or bug fixes during the refactor.
🧬 Code graph analysis (3)
libs/core/kiln_ai/cli/commands/projects.py (2)
libs/core/kiln_ai/utils/config.py (1)
Config(31-292)libs/core/kiln_ai/datamodel/basemodel.py (1)
load_from_file(355-403)
libs/core/kiln_ai/cli/commands/package_project.py (6)
libs/core/kiln_ai/cli/commands/projects.py (1)
print_projects_table(12-41)libs/core/kiln_ai/datamodel/prompt.py (1)
Prompt(32-37)libs/core/kiln_ai/datamodel/prompt_id.py (1)
PromptGenerators(8-16)libs/core/kiln_ai/datamodel/project.py (1)
tasks(42-43)libs/core/kiln_ai/datamodel/task.py (1)
run_configs(172-173)libs/core/kiln_ai/adapters/model_adapters/base_adapter.py (1)
build_prompt(197-208)
libs/core/kiln_ai/cli/cli.py (1)
libs/core/kiln_ai/cli/commands/package_project.py (1)
package_project(209-276)
🪛 LanguageTool
libs/core/README.md
[style] ~112-~112: To elevate your writing, try using a synonym like ‘required’ here. Or, to avoid using the passive voice, try replacing the past participle ‘needed’ with an adjective.
Context: ... service. Only a few of these files are needed for running the task: you can export a ...
(IS_NEEDED_NECESSARY)
🪛 markdownlint-cli2 (0.18.1)
libs/core/README.md
110-110: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
118-118: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Generate Coverage Report
🔇 Additional comments (12)
libs/core/pyproject.toml (1)
45-49: LGTM!The Typer dependency and CLI entrypoint are correctly configured. The entrypoint
kiln_ai.cli:appproperly references the Typer app exported from the CLI module.libs/core/kiln_ai/cli/__init__.py (1)
1-3: LGTM!Clean public API exposure that correctly wires up with the
kiln_ai.cli:appentrypoint inpyproject.toml.libs/core/kiln_ai/cli/cli.py (1)
1-11: LGTM!Clean Typer-based CLI structure. The
no_args_is_help=Trueprovides good UX, and the hierarchical command structure (projects namespace + top-level package_project) is well organized.libs/core/README.md (1)
110-125: LGTM - Clear documentation for the new CLI.The export command documentation is well-structured and provides clear guidance. The
uvx kiln_aiexample is appropriate for users who want to run without installing.Note: The static analysis flagged using bold text instead of proper headings (Lines 110, 118), but this is a common pattern in README step lists and is acceptable here.
libs/core/kiln_ai/cli/commands/test_package_project.py (2)
1-26: LGTM - Comprehensive test suite!Well-structured tests following pytest conventions per the coding guidelines. Good coverage of:
- Happy paths and error conditions
- Fixture reuse across test classes
- Proper mocking for user interaction (typer.confirm)
- Exit code validation for CLI errors
319-336: Good use of parametrized tests.The parametrized test for
is_dynamic_promptefficiently covers all prompt generator types, including the edge case for saved prompts (id::some_saved_prompt).libs/core/kiln_ai/cli/commands/projects.py (1)
44-50: LGTM!The list command correctly handles empty project lists and provides clear user feedback.
libs/core/kiln_ai/cli/commands/package_project.py (5)
17-24: LGTM - Good separation of dynamic prompt generators.The set of dynamic generators that require user confirmation is clearly defined and matches the generators that depend on dataset content (few-shot, multi-shot, repairs).
27-53: LGTM - Robust project loading with helpful error messages.Good handling of both file and directory paths, with clear error messages that also display available projects to guide the user.
149-159: Verify: JSON instructions are excluded from exported prompts.
build_prompt_from_builderalways passesinclude_json_instructions=False, which differs from the adapter logic inbase_adapter.pythat conditionally includes JSON instructions based onstructured_output_mode. This appears intentional for export purposes (keeping prompts clean for user customization), but verify this is the desired behavior for all use cases.
223-226: Stage 2 implementation pending -outputparameter is declared but unused.The
outputpath is defined and displayed (Line 273) but the actual file writing (Stage 2) is not implemented yet. The function returns prompts for testing and future implementation, which is fine for incremental development.Consider adding a TODO or note in the docstring to clarify the staged implementation:
) -> dict[str, Prompt]: """Package a Kiln project for deployment. Creates a minimal version of a Kiln project containing only the files needed to run specified tasks on a server. + + Note: Currently implements Stage 1 (validation). Stage 2 (file export) is pending. """
162-206: LGTM - Well-designed two-pass prompt validation.The approach of detecting dynamic prompts first, then asking for confirmation once (rather than per-task), provides good UX. The warning message clearly explains the implications of dynamic prompts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
libs/core/kiln_ai/cli/commands/test_package_project.py (1)
480-484: Consider movingshutilimport to the top of the file.The
shutilimport is repeated in multiple test methods' cleanup blocks. Moving it to the file-level imports (alongside other standard library imports) would be cleaner.Add at the top of the file with other imports:
import shutil import zipfile from pathlib import Path ...Then remove the inline
import shutilstatements from the test methods.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
libs/core/kiln_ai/cli/commands/package_project.py(1 hunks)libs/core/kiln_ai/cli/commands/test_package_project.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
libs/core/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/project.mdc)
Use Python 3.10+ for the core library (libs/core) and Python 3.13 for the desktop app
Use python 3.10+ for the core library (libs/core) and python 3.13 for the desktop app
Files:
libs/core/kiln_ai/cli/commands/test_package_project.pylibs/core/kiln_ai/cli/commands/package_project.py
**/*test*.py
📄 CodeRabbit inference engine (.cursor/rules/project.mdc)
Use pytest for testing Python code
Use pytest for testing in python projects
Files:
libs/core/kiln_ai/cli/commands/test_package_project.py
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/project.mdc)
**/*.py: Use asyncio for asynchronous operations in Python
Use Pydantic v2 (not v1) for data validation
Files:
libs/core/kiln_ai/cli/commands/test_package_project.pylibs/core/kiln_ai/cli/commands/package_project.py
libs/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use Pydantic v2 (not v1) for data validation and serialization
Files:
libs/core/kiln_ai/cli/commands/test_package_project.pylibs/core/kiln_ai/cli/commands/package_project.py
🧠 Learnings (1)
📓 Common learnings
Learnt from: scosman
Repo: Kiln-AI/Kiln PR: 733
File: app/web_ui/src/lib/stores/local_storage_store.ts:9-11
Timestamp: 2025-10-21T00:06:57.115Z
Learning: When scosman is refactoring code by moving it to a new location, he prefers to keep the moved code unchanged and not mix in functional improvements or bug fixes during the refactor.
🧬 Code graph analysis (1)
libs/core/kiln_ai/cli/commands/test_package_project.py (8)
libs/core/kiln_ai/datamodel/prompt.py (1)
Prompt(32-37)libs/core/kiln_ai/datamodel/prompt_id.py (1)
PromptGenerators(8-16)libs/core/kiln_ai/datamodel/run_config.py (1)
ToolsRunConfig(14-21)libs/core/kiln_ai/cli/commands/package_project.py (14)
package_project(347-464)build_prompt_from_builder(152-162)check_for_tools(132-144)create_export_directory(212-226)create_zip(335-344)export_task(229-263)get_default_run_config(104-129)is_dynamic_prompt(147-149)load_project(30-56)parse_task_ids(71-85)print_tasks_table(59-68)save_prompt_to_task(266-295)validate_and_build_prompts(165-209)validate_exported_prompts(298-332)libs/core/kiln_ai/datamodel/project.py (1)
tasks(42-43)libs/core/kiln_ai/datamodel/task.py (2)
run_configs(172-173)prompts(166-167)libs/core/kiln_ai/datamodel/basemodel.py (1)
load_from_file(355-403)libs/core/kiln_ai/utils/test_filesystem_cache.py (1)
temp_dir(13-15)
🪛 GitHub Actions: Debug Detector
libs/core/kiln_ai/cli/commands/package_project.py
[error] 1-1: Developer content found. The TODO/FIXME / Python print statement check detected developer content in this file during the 'Checking for TODO or FIXME' step, causing the pipeline to fail with exit code 1.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: Generate Coverage Report
- GitHub Check: Build Desktop Apps (macos-latest)
- GitHub Check: Build Desktop Apps (ubuntu-22.04-arm)
- GitHub Check: Build, Typecheck, and Test Python (3.12)
- GitHub Check: Build, Typecheck, and Test Python (3.10)
- GitHub Check: Build, Typecheck, and Test Python (3.11)
- GitHub Check: Build Desktop Apps (windows-latest)
- GitHub Check: Build Desktop Apps (ubuntu-22.04)
- GitHub Check: Build, Typecheck, and Test Python (3.13)
- GitHub Check: Build Desktop Apps (macos-15-intel)
🔇 Additional comments (33)
libs/core/kiln_ai/cli/commands/package_project.py (15)
1-17: Imports and structure look good.The imports are appropriate for a CLI module using Typer and Rich. The organization follows Python conventions.
20-27: LGTM!Using a set for the dynamic prompt generators enables efficient O(1) membership checks.
30-56: LGTM!Good error handling with user-friendly messages showing available projects when the path is invalid.
59-68: LGTM!Clean table rendering with appropriate fallback for missing task IDs.
71-85: LGTM!Properly handles the various input formats and filters out tasks without IDs.
88-101: LGTM!Good validation with clear error messaging and order preservation.
104-129: LGTM!Good validation with helpful hints guiding users to the UI for resolution.
132-144: LGTM!Proper null-safe checking of nested properties.
147-162: LGTM!Clean utility functions with clear responsibilities.
165-209: LGTM!Good two-pass approach with user confirmation for dynamic prompts. The
# type: ignorecomments are acceptable since tasks are validated to have IDs before reaching this point.
212-226: LGTM!Clean implementation with proper path validation.
229-263: LGTM!Good structure preservation and proper parent relationship setup for the exported entities.
298-332: LGTM!Thorough validation by rebuilding prompts and comparing both content and chain-of-thought instructions.
335-344: LGTM!Clean zip creation with proper parent directory handling and compression.
347-462: Well-structured packaging workflow.The two-stage approach (validation then export) with proper cleanup in the
finallyblock is good practice. The progress messaging provides clear feedback to users.Regarding the pipeline failure about "Developer content found" - this appears to be a false positive. This CLI module legitimately uses
console.print()(Rich library) for user-facing output, which is distinct from debugprint()statements. You may need to configure the pipeline check to allow this module or adjust the detection pattern.Please verify if the pipeline check can distinguish between
console.print()(Rich) and the built-inprint()function, or if this module needs to be allowlisted.libs/core/kiln_ai/cli/commands/test_package_project.py (18)
1-31: LGTM!Clean imports and well-organized module structure. Using pytest as per coding guidelines.
34-198: LGTM!Well-designed fixtures covering various project configurations. Good use of
tmp_pathfor automatic cleanup.
201-225: LGTM!Good coverage of both success and error paths for project loading.
228-261: LGTM!Comprehensive coverage of task ID parsing scenarios.
264-279: LGTM!Good coverage of task validation scenarios.
282-305: LGTM!Thorough testing of run config retrieval including edge cases.
308-322: LGTM!Clear tests for tool detection behavior.
325-342: LGTM!Excellent use of
pytest.mark.parametrizefor comprehensive coverage of all prompt types.
345-384: LGTM!Good testing of both static and dynamic prompt flows with proper mocking of user confirmation.
387-444: LGTM!Good coverage of the main packaging command including error scenarios.
447-451: LGTM!Smoke test for the table printing function.
454-466: LGTM!Good verification of prompt builder integration.
469-491: LGTM!Good testing of export directory creation. The manual cleanup in
finallyblocks is appropriate sincecreate_export_directorycreates its own temp directory outside of pytest'stmp_path.
494-544: LGTM!Thorough testing of task export with proper validation of structure preservation.
547-590: LGTM!Comprehensive test verifying prompt persistence and run config updates.
593-668: LGTM!Good testing of prompt validation for both matching and mismatched scenarios.
671-704: LGTM!Good verification of zip file creation and structure.
707-795: LGTM!Excellent end-to-end tests validating the complete packaging workflow, zip structure, and loadability of exported projects.
| # check it saved to the correct location | ||
| saved = False | ||
| for exported_task_prompt in exported_task.prompts(readonly=True): | ||
| if ( | ||
| exported_task_prompt.path == prompt.path | ||
| and exported_task_prompt.id == prompt.id | ||
| ): | ||
| saved = True | ||
| break | ||
| if not saved: | ||
| raise ValueError( | ||
| f"Prompt saved to incorrect location: {prompt.path} != {exported_task_prompt.path}" | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential NameError if no prompts exist.
If exported_task.prompts(readonly=True) returns an empty list, the loop never executes and exported_task_prompt is undefined when constructing the error message on line 289.
Apply this diff to fix the error message:
# check it saved to the correct location
saved = False
for exported_task_prompt in exported_task.prompts(readonly=True):
if (
exported_task_prompt.path == prompt.path
and exported_task_prompt.id == prompt.id
):
saved = True
break
if not saved:
raise ValueError(
- f"Prompt saved to incorrect location: {prompt.path} != {exported_task_prompt.path}"
+ f"Prompt was not found in task prompts after saving. Expected path: {prompt.path}"
)🤖 Prompt for AI Agents
In libs/core/kiln_ai/cli/commands/package_project.py around lines 278 to 290,
the code raises a ValueError referencing exported_task_prompt which may be
undefined if exported_task.prompts(readonly=True) is empty; change the error
construction to avoid using exported_task_prompt (e.g., report the expected
prompt.path and the set of actual exported paths or that no prompts were
exported) — compute a safe list of exported paths before the loop (or after when
saved is still False) and use that in the ValueError message so it never refers
to an undefined variable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
libs/core/kiln_ai/cli/commands/package_project.py (1)
469-498: Fix potentialNameErrorinsave_prompt_to_taskerror pathIf
exported_task.prompts(readonly=True)returns an empty list, the loop never assignsexported_task_prompt, and the error message references it, causing an exception unrelated to the actual problem.You can avoid this by not using
exported_task_promptin the error message:# check it saved to the correct location saved = False for exported_task_prompt in exported_task.prompts(readonly=True): if ( exported_task_prompt.path == prompt.path and exported_task_prompt.id == prompt.id ): saved = True break if not saved: raise ValueError( - f"Prompt saved to incorrect location: {prompt.path} != {exported_task_prompt.path}" + f"Prompt was not found in task prompts after saving. Expected path: {prompt.path}" )This keeps the check while ensuring the failure path is itself safe.
🧹 Nitpick comments (4)
libs/core/kiln_ai/cli/commands/test_package_project.py (2)
63-72: Unifymodel_provider_nameusage across fixturesSome fixtures pass
model_provider_name="openai"while others useModelProviderName.openai. For consistency (and to avoid breakage if enum values ever change), consider using the enum everywhere in test fixtures.- model_provider_name="openai", + model_provider_name=ModelProviderName.openai,Apply similarly in the other fixtures that currently use the plain string.
Also applies to: 102-110, 157-167, 198-202
653-676: Avoid writing test artifacts to the repo root
test_full_validation_flowandtest_all_tasks_flowwrite./test_output.zipwithout cleanup. This can leave artifacts in the repo and cause interference between runs.Consider using
tmp_path(as you already do in other tests) for these outputs, e.g.:- def test_full_validation_flow(self, temp_project): + def test_full_validation_flow(self, temp_project, tmp_path: Path): @@ - result = package_project( + result = package_project( project_path=temp_project["path"], tasks=temp_project["task"].id, all_tasks=False, - output=Path("./test_output.zip"), + output=tmp_path / "test_output.zip", )And similarly for
test_all_tasks_flow.libs/core/kiln_ai/cli/commands/tasks.py (1)
20-28: Broadexcept Exceptioninload_project_by_idhides real failuresSwallowing all exceptions when loading each project path can mask problems like corrupted files or schema issues and silently skip them.
If you want to keep this resilient, consider at least narrowing to expected I/O/validation errors or logging/debug-printing the exception so misconfigured projects are discoverable during debugging.
libs/core/kiln_ai/cli/commands/package_project.py (1)
415-429: Makecreate_export_directoryexception‑safe to avoid temp dir leaksIf
project.pathisNoneor if copying/loading fails, the function currently leaves the freshly created temp directory behind. This is mostly visible when called directly (as in tests), but easy to harden.Consider wrapping the body in a try/except and cleaning up on failure:
def create_export_directory(project: Project) -> tuple[Path, Project]: @@ - temp_dir = Path(tempfile.mkdtemp(prefix="kiln_export_")) - - if project.path is None: - raise ValueError("Project path is not set") - - shutil.copy(project.path, temp_dir / "project.kiln") - exported_project = Project.load_from_file(temp_dir / "project.kiln") - - return temp_dir, exported_project + temp_dir = Path(tempfile.mkdtemp(prefix="kiln_export_")) + + try: + if project.path is None: + raise ValueError("Project path is not set") + + shutil.copy(project.path, temp_dir / "project.kiln") + exported_project = Project.load_from_file(temp_dir / "project.kiln") + return temp_dir, exported_project + except Exception: + shutil.rmtree(temp_dir, ignore_errors=True) + raise
package_projectalready cleans uptemp_dirafter a successful call, so this only affects the exceptional path.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
libs/core/kiln_ai/cli/cli.py(1 hunks)libs/core/kiln_ai/cli/commands/package_project.py(1 hunks)libs/core/kiln_ai/cli/commands/projects.py(1 hunks)libs/core/kiln_ai/cli/commands/tasks.py(1 hunks)libs/core/kiln_ai/cli/commands/test_package_project.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- libs/core/kiln_ai/cli/commands/projects.py
- libs/core/kiln_ai/cli/cli.py
🧰 Additional context used
📓 Path-based instructions (4)
libs/core/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/project.mdc)
Use Python 3.10+ for the core library (libs/core) and Python 3.13 for the desktop app
Use python 3.10+ for the core library (libs/core) and python 3.13 for the desktop app
Files:
libs/core/kiln_ai/cli/commands/tasks.pylibs/core/kiln_ai/cli/commands/test_package_project.pylibs/core/kiln_ai/cli/commands/package_project.py
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/project.mdc)
**/*.py: Use asyncio for asynchronous operations in Python
Use Pydantic v2 (not v1) for data validation
Files:
libs/core/kiln_ai/cli/commands/tasks.pylibs/core/kiln_ai/cli/commands/test_package_project.pylibs/core/kiln_ai/cli/commands/package_project.py
libs/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use Pydantic v2 (not v1) for data validation and serialization
Files:
libs/core/kiln_ai/cli/commands/tasks.pylibs/core/kiln_ai/cli/commands/test_package_project.pylibs/core/kiln_ai/cli/commands/package_project.py
**/*test*.py
📄 CodeRabbit inference engine (.cursor/rules/project.mdc)
Use pytest for testing Python code
Use pytest for testing in python projects
Files:
libs/core/kiln_ai/cli/commands/test_package_project.py
🧠 Learnings (1)
📓 Common learnings
Learnt from: scosman
Repo: Kiln-AI/Kiln PR: 733
File: app/web_ui/src/lib/stores/local_storage_store.ts:9-11
Timestamp: 2025-10-21T00:06:57.115Z
Learning: When scosman is refactoring code by moving it to a new location, he prefers to keep the moved code unchanged and not mix in functional improvements or bug fixes during the refactor.
🧬 Code graph analysis (3)
libs/core/kiln_ai/cli/commands/tasks.py (3)
libs/core/kiln_ai/utils/config.py (1)
Config(31-292)libs/core/kiln_ai/datamodel/basemodel.py (1)
load_from_file(355-403)libs/core/kiln_ai/datamodel/project.py (1)
tasks(42-43)
libs/core/kiln_ai/cli/commands/test_package_project.py (8)
libs/core/kiln_ai/datamodel/prompt.py (1)
Prompt(32-37)libs/core/kiln_ai/datamodel/prompt_id.py (1)
PromptGenerators(8-16)libs/core/kiln_ai/datamodel/run_config.py (1)
ToolsRunConfig(14-21)libs/core/kiln_ai/datamodel/tool_id.py (1)
KilnBuiltInToolId(24-28)libs/core/kiln_ai/cli/commands/package_project.py (13)
package_project(587-729)build_prompt_from_builder(355-365)classify_tool_id(213-231)collect_required_tool_servers(317-347)collect_subtask_ids_from_tools(152-210)create_export_directory(415-429)create_zip(575-584)export_task(432-466)print_tasks_table(69-78)save_prompt_to_task(469-498)validate_exported_prompts(501-535)validate_tasks(98-111)validate_tools(234-314)libs/core/kiln_ai/datamodel/project.py (2)
tasks(42-43)external_tool_servers(63-64)libs/core/kiln_ai/datamodel/task.py (2)
run_configs(172-173)prompts(166-167)libs/core/kiln_ai/utils/test_filesystem_cache.py (1)
temp_dir(13-15)
libs/core/kiln_ai/cli/commands/package_project.py (8)
libs/core/kiln_ai/datamodel/prompt.py (1)
Prompt(32-37)libs/core/kiln_ai/datamodel/prompt_id.py (1)
PromptGenerators(8-16)libs/core/kiln_ai/datamodel/tool_id.py (3)
KilnBuiltInToolId(24-28)kiln_task_server_id_from_tool_id(132-154)mcp_server_and_tool_name_from_id(87-106)libs/core/kiln_ai/datamodel/basemodel.py (1)
load_from_file(355-403)libs/core/kiln_ai/datamodel/project.py (2)
tasks(42-43)external_tool_servers(63-64)libs/core/kiln_ai/tools/rag_tools.py (1)
project(110-114)libs/core/kiln_ai/datamodel/task.py (2)
run_configs(172-173)prompts(166-167)libs/core/kiln_ai/adapters/model_adapters/base_adapter.py (1)
build_prompt(197-208)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Generate Coverage Report
- GitHub Check: Build Desktop Apps (ubuntu-22.04-arm)
- GitHub Check: Build, Typecheck, and Test Python (3.11)
- GitHub Check: Build, Typecheck, and Test Python (3.10)
- GitHub Check: Build, Typecheck, and Test Python (3.12)
- GitHub Check: Build Desktop Apps (macos-latest)
- GitHub Check: Build Desktop Apps (ubuntu-22.04)
- GitHub Check: Build Desktop Apps (macos-15-intel)
- GitHub Check: Build Desktop Apps (windows-latest)
🔇 Additional comments (3)
libs/core/kiln_ai/cli/commands/test_package_project.py (1)
483-1479: Strong, end‑to‑end coverage of packaging workflowThe tests exercise happy paths, error paths, and tool/prompt edge cases for the packaging pipeline (including MCP, RAG, kiln_task subtasks, and zip structure). This is a solid base for safely evolving
package_project.libs/core/kiln_ai/cli/commands/tasks.py (1)
61-89: Task listing flow looks goodThe
list_taskscommand cleanly delegates project resolution toload_project, uses readonly task loading, and provides a concise Rich table plus helpful messaging for the empty-project case.libs/core/kiln_ai/cli/commands/package_project.py (1)
587-729: Overall packaging orchestration looks solidThe
package_projectflow has clear staging (project + task validation, run config + tool checks, prompt building, export, then zip) with consistent console feedback and a guarded temp‑dir lifecycle. Combined with the accompanying tests, this gives a robust baseline for the new CLI command.
| console.print(f"[red]Error: Project not found: {project_ref}[/red]") | ||
| console.print("Provide a valid project file path or project ID.") | ||
| console.print("Use 'kiln project list' to see available projects.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix suggested command name for listing projects
The guidance says:
console.print("Use 'kiln project list' to see available projects.")But the CLI surface in this PR uses the projects subcommand (and entrypoint kiln_ai in the examples), e.g. uv run kiln_ai projects list.
Updating this string to match the actual command (e.g. "Use 'kiln_ai projects list' to see available projects.") will avoid confusing users.
🤖 Prompt for AI Agents
In libs/core/kiln_ai/cli/commands/tasks.py around lines 55 to 57, the error
guidance references the wrong CLI command; change the message to match the
actual CLI entrypoint and subcommand (e.g. "kiln_ai projects list" or whatever
the repository uses) so users aren't misled. Update the third console.print call
to instruct users to run the correct command (replace "kiln project list" with
the real invocation like "kiln_ai projects list"), keeping phrasing and
formatting consistent with surrounding messages.
A new CLI tool to package up project for running via the library.
List Projects
uv run kiln_ai projects listList Projects
uv run kiln_ai tasks list /path/to/project.kilnPackage Project & Tasks
uv run kiln_ai package_project "/Users/scosman/Kiln Projects/test project 2 og/project.kiln" -t 920784333097Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.