Improve API docs#1182
Merged
Merged
Conversation
Phase 2 of OpenAPI documentation improvements. Adds Path(description=...), Query(description=...), and Body(description=...) annotations to all parameters across both libs/server/ and app/desktop/studio_server/. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add Path(description=...) and Query(description=...) annotations to all undescribed parameters across libs/server/ and app/desktop/studio_server/. Covers 19 files including tool_api, skill_api, copilot_api, prompt_optimization_job_api, run_config_api, and import_api which were missed in the initial pass.
Added docstrings and Field(description=...) across core datamodels, server API models, and desktop server models to improve OpenAPI spec documentation.
Backend: singular→plural path standardization (tasks, prompts, specs, evals), run config path unification, repair endpoint renames, eval endpoint renames, GET→POST for provider connect endpoints. Frontend: regenerated api_schema.d.ts, updated all path references in stores and components.
Added tags to endpoints in document_api, tool_api, skill_api, prompt_api, copilot_api, run_config_api, prompt_optimization_job_api, and import_api that were showing up in an untagged 'Other API' group.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds OpenAPI metadata (summaries, tags, parameter descriptions) and Pydantic Field descriptions across many backend APIs and datamodels; pluralizes several REST segments (e.g., /task→/tasks, /eval→/evals, /spec→/specs); switches two provider connect endpoints GET→POST; renames repair endpoints; updates matching frontend calls and tests. Changes
Sequence Diagram(s)(Skipped — changes are documentation/schema metadata and route renames rather than new multi-component control flow.) Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request significantly enhances the OpenAPI documentation for the Kiln AI API by adding comprehensive descriptions, summaries, and tags to FastAPI endpoints and Pydantic models. It standardizes API paths to plural forms, unifies run configuration endpoints, and corrects HTTP method usage for mutations. Corresponding updates were made to the Svelte frontend to maintain compatibility with the revised API schema. Feedback includes a correction for a query parameter nesting issue in the frontend, a warning regarding mutable default arguments in Python, and a type correction for a path parameter.
| run_config_ids: Annotated[ | ||
| list[str], | ||
| Query(description="The list of run configuration IDs to evaluate."), | ||
| ] = [], |
Contributor
There was a problem hiding this comment.
Using a mutable default value like [] for a function argument is a common pitfall in Python. While FastAPI's Query parameter handling often mitigates this, it's safer and more idiomatic to use None as the default and initialize it to an empty list within the function body if needed.
Collaborator
Author
There was a problem hiding this comment.
Not an issue here — FastAPI's Query() creates a new list per request, so the mutable default pitfall doesn't apply.
📊 Coverage ReportOverall Coverage: 91% Diff: origin/main...HEAD
Summary
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
app/desktop/studio_server/settings_api.py (1)
46-77:⚠️ Potential issue | 🟠 MajorPreserve HTTPException semantics and avoid returning raw exception text.
Line 76 currently converts all exceptions to
500and exposesdetail=str(e). That masks intended HTTP status codes (like404) and leaks internals.Based on learnings: "The `project_from_id` function in `kiln_server.project_api` already handles HTTP errors internally, raising HTTPException with status code 404 when a project is not found, so additional error handling around calls to this function is not needed."Proposed fix
def open_logs(): """Opens the log folder in the system file browser.""" try: open_logs_folder() return {"message": "opened"} - except Exception as e: - raise HTTPException(status_code=500, detail=str(e)) + except Exception: + raise HTTPException(status_code=500, detail="Failed to open logs folder.") def open_project_folder( @@ """Opens the project folder in the system file browser.""" try: project = project_from_id(project_id) @@ open_folder(path) return {"message": "opened"} - except Exception as e: - raise HTTPException(status_code=500, detail=str(e)) + except HTTPException: + raise + except Exception: + raise HTTPException(status_code=500, detail="Failed to open project folder.")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/desktop/studio_server/settings_api.py` around lines 46 - 77, The open_project_folder handler currently catches all exceptions and returns HTTP 500 with detail=str(e), which masks intended HTTPExceptions (e.g., from project_from_id) and leaks internals; change the error handling in open_project_folder so that it does not convert existing HTTPException instances—either remove the broad try/except or re-raise if isinstance(e, HTTPException)—and for unexpected errors raise a new HTTPException(status_code=500, detail="Internal server error") (do not include str(e)); keep the call to project_from_id and open_folder unchanged so project_from_id can raise its own 404 as designed.libs/server/kiln_server/project_api.py (2)
107-109:⚠️ Potential issue | 🟠 MajorGuard config projects before filtering.
Line 108 can raise
TypeErrorifConfig.shared().projectsisNoneor non-list (which is already handled inadd_project_to_config). Make delete resilient to the same state.🛠️ Suggested fix
- projects_before = Config.shared().projects + projects_before = Config.shared().projects + if not isinstance(projects_before, list): + projects_before = [] projects_after = [p for p in projects_before if p != str(project.path)] Config.shared().save_setting("projects", projects_after)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/server/kiln_server/project_api.py` around lines 107 - 109, Guard against Config.shared().projects being None or non-list before filtering: retrieve projects_before = Config.shared().projects, if not isinstance(projects_before, list) then normalize it to an empty list (or to list(projects_before) if you expect other iterables), compute projects_after by filtering that normalized list (e.g., [p for p in normalized if p != str(project.path)]), and then call Config.shared().save_setting("projects", projects_after); update the code around projects_before/projects_after to use the normalized variable so deletion is resilient to None or wrong types.
65-68:⚠️ Potential issue | 🟠 MajorUse the validated model instance before saving.
On line 67,
Project.model_validate(...)is called for validation but its result is ignored. Sincemodel_copy(update=...)does not validate the updated fields, any coercion or normalization (e.g., thenamefield'sFilenameStringvalidator) is skipped. Line 68 then persists the unvalidatedupdated_project.Suggested fix
- updated_project = original_project.model_copy(update=project_updates) - # Force validation using model_validate() - Project.model_validate(updated_project.model_dump()) - updated_project.save_to_file() - return updated_project + updated_project = original_project.model_copy(update=project_updates) + # Force validation and keep the validated/coerced model + validated_project = Project.model_validate(updated_project.model_dump()) + validated_project.save_to_file() + return validated_project🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/server/kiln_server/project_api.py` around lines 65 - 68, The code calls Project.model_validate(...) but ignores its return value, so coercions/validators from Project aren't applied before saving; replace the sequence that uses updated_project.model_copy(update=project_updates) followed by a discarded Project.model_validate(...) by validating and using the validated instance—call Project.model_validate(...) (or updated_project.model_validate() if available) and assign its result to updated_project, then call updated_project.save_to_file() so the validated/coerced model is what's persisted; reference updated_project, Project.model_validate, model_copy, and save_to_file when making the change.libs/server/kiln_server/test_task_api.py (1)
96-104:⚠️ Potential issue | 🔴 CriticalBug: Mock is not active when making the request.
The
response = client.post(...)call at line 101 is outside thewith patch(...)context manager (which ends at line 99). This meansmock_load.side_effectwon't be applied when the request is made, so the test won't actually test the error handling path.🐛 Proposed fix to move the request inside the context manager
with patch("kiln_server.task_api.project_from_id") as mock_load: mock_load.side_effect = HTTPException( status_code=404, detail="Project not found" ) - - response = client.post("/api/projects/FAKEPROJECTID/tasks", json=task_data) + response = client.post("/api/projects/FAKEPROJECTID/tasks", json=task_data) assert response.status_code == 404🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/server/kiln_server/test_task_api.py` around lines 96 - 104, The test's HTTPException side effect on project_from_id isn't applied because the client.post call is outside the with patch(...) context; move the request into the same with patch("kiln_server.task_api.project_from_id") as mock_load: block so mock_load.side_effect = HTTPException(...) is active when calling client.post("/api/projects/FAKEPROJECTID/tasks", json=task_data), then assert response.status_code and response.json()["message"] as written.app/desktop/studio_server/eval_api.py (1)
524-552:⚠️ Potential issue | 🟠 MajorValidate
train_set_filter_idbefore saving it.This patch path still stores any string into
eval.train_set_filter_id. Later flows in this file resolve that value throughdataset_filter_from_id(), so a typo here becomes persisted invalid state and breaks follow-on eval APIs instead of failing fast on the PATCH request.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/desktop/studio_server/eval_api.py` around lines 524 - 552, The update_eval handler currently assigns request.train_set_filter_id into eval.train_set_filter_id without validating it; before setting it (inside update_eval and after the existing "already set" guard), call the same resolver used elsewhere (dataset_filter_from_id) with the project_id and request.train_set_filter_id to ensure the filter exists/loads successfully, and if that resolver fails or returns None raise an HTTPException 400 with a clear message; only assign eval.train_set_filter_id after successful validation so typos or invalid IDs fail fast on PATCH.
🧹 Nitpick comments (1)
app/desktop/studio_server/settings_api.py (1)
23-67: Add explicit return types on the touched handlers.
update_settings,read_setting_item,open_logs, andopen_project_folderin this changed block should declare concrete return types for stronger typing contracts.As per coding guidelines: "Maintain very high code quality in Python files, with strong typing and comprehensive test coverage."Proposed type annotations
def connect_settings(app: FastAPI): @@ - def update_settings( + def update_settings( new_settings: dict[str, int | float | str | bool | list | None], - ): + ) -> dict[str, Any]: @@ - def read_setting_item( + def read_setting_item( item_id: Annotated[str, Path(description="The setting item key to retrieve.")], - ): + ) -> dict[str, Any]: @@ - def open_logs(): + def open_logs() -> dict[str, str]: @@ - def open_project_folder( + def open_project_folder( project_id: Annotated[ str, Path(description="The unique identifier of the project.") ], - ): + ) -> dict[str, str]:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/desktop/studio_server/settings_api.py` around lines 23 - 67, Add explicit return type annotations to the modified route handlers: annotate update_settings to return dict[str, Any] (it returns Config.shared().settings(...)), read_setting_item to return dict[str, Optional[Any]] (it returns {item_id: settings.get(item_id, None)}), and annotate open_logs and open_project_folder to return dict[str, str] (they return {"message": "opened"} or raise HTTPException). Ensure necessary typing imports (e.g., Any, Optional, Dict) are present and update the function signatures for update_settings, read_setting_item, open_logs, and open_project_folder accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/desktop/studio_server/eval_api.py`:
- Around line 842-845: The OpenAPI summary for the route registered by the
`@app.get` decorator for
"/api/projects/{project_id}/tasks/{task_id}/evals/{eval_id}/run_calibration" is
out-of-date ("Run Eval Config Comparison"); update the summary parameter on that
`@app.get` route to match the new path name (e.g., "Run Calibration" or "Run Eval
Calibration") so the generated docs and UI reflect the renamed endpoint (locate
the `@app.get`(...) decorator in eval_api.py and change its summary value).
- Around line 801-816: Update the type annotation for the eval_config_id path
parameter in the set_default_eval_config function from str | None to str so the
OpenAPI/TypeScript schema correctly emits a non-nullable string; keep the
existing string-literal handling (the check if eval_config_id == "None")
unchanged, and regenerate/openapi schema as needed so clients no longer see
"string | null" for eval_config_id.
In `@app/desktop/studio_server/provider_api.py`:
- Around line 523-536: The route handler save_openai_compatible_providers is
currently accepting secrets via Query (api_key) which can leak; change the
endpoint to accept a Pydantic request body model (e.g.,
OpenAICompatibleProviderCreate with fields name, base_url, api_key) and replace
the Annotated Query parameters with a single body parameter (e.g., payload:
OpenAICompatibleProviderCreate) in the function signature, update any usages
inside the function to read payload.name/payload.base_url/payload.api_key, and
ensure the route still uses app.post("/api/provider/openai_compatible") so the
secret is passed in the request body instead of URL query params.
In
`@app/web_ui/src/routes/`(fullscreen)/setup/(setup)/connect_providers/connect_providers.svelte:
- Around line 556-562: The POST call using client.POST to
"/api/provider/docker_model_runner/connect" is passing query parameters under a
top-level "query" key so docker_model_runner_custom_url is never sent; update
the call to place the query object under params.query (i.e., replace the
top-level "query" with "params: { query: { docker_model_runner_custom_url:
docker_model_runner_custom_url || undefined } }") to match the other provider
connect calls and ensure the parameter is transmitted.
In `@libs/server/kiln_server/document_api.py`:
- Around line 1342-1359: Add an OpenAPI-visible docstring to the GET SSE
mutation handler run_extractor_config explaining that the endpoint performs
mutations and why GET is required (EventSource/SSE doesn’t support POST), and do
the same for the other GET SSE mutation handler in this module (the later SSE
GET endpoint that streams mutation results). Place the explanatory docstrings
immediately under each async def (e.g., async def run_extractor_config(...):) so
the FastAPI/OpenAPI generator includes the mutation warning and rationale in the
docs.
In `@libs/server/kiln_server/run_api.py`:
- Around line 504-507: The form description for the parameter "splits" is
misleading: it says a mapping to tag lists but the code path (parse_splits)
expects a JSON mapping of split names to numeric weights between 0 and 1. Update
the Annotated description for the splits parameter to state that it should be a
JSON string mapping split names to numeric values in the range [0,1] (or
percentages), and mention any normalization behavior if applicable so it matches
parse_splits' validation and error messages; reference the splits parameter and
the parse_splits function to locate and align the docs with the validation
contract.
In `@specs/projects/document_api/functional_spec.md`:
- Around line 27-39: The fenced code blocks containing the API snippets (e.g.,
the blocks starting with "DELETE .../runs/{run_id}", "POST
.../tasks/{task_id}/run", and "POST .../tasks/{task_id}/runs") need explicit
language identifiers to satisfy markdownlint MD040; update each opening
triple-backtick to include a language label (use "text") so the blocks become
```text and ensure all similar unlabeled code fences in this section (including
the repeated blocks around lines 27 and 42–47) are updated consistently.
In `@specs/projects/document_api/implementation_plan.md`:
- Line 2: The frontmatter contains a typo: the value for the status key is
written as "compelete"; update the frontmatter so the status key reads "status:
complete" (correct spelling) to match tooling expectations and canonical values,
ensure YAML/frontmatter formatting remains valid and, if present elsewhere in
the file, replace any other instances of "compelete" with "complete".
In `@specs/projects/document_api/project_overview.md`:
- Around line 186-194: The markdown has two issues: inconsistent task paths
(write endpoints use /task/{task_id} while reads use /tasks/{task_id}) and
malformed table blocks lacking blank lines which breaks rendering; update all
task-scoped path strings (e.g., POST /api/projects/{project_id}/task, PATCH
/api/projects/{project_id}/task/{task_id}, DELETE
/api/projects/{project_id}/task/{task_id}, POST
/api/projects/{project_id}/task/{task_id}/prompt, GET
/api/projects/{project_id}/task/{task_id}/prompts, GET
/api/projects/{project_id}/task/{task_id}/gen_prompt/{prompt_id}) to use the
plural form /tasks/{task_id} consistently, and add a blank line before and after
each table block (and move any non-table prose outside the table) in the
affected sections (including the examples around lines referenced) so
markdownlint and rendering treat tables correctly.
- Line 5: Remove the empty H1 heading (the lone "#" markdown token) from the
document to avoid creating a blank section anchor and cluttered navigation;
locate the stray "#" in specs/projects/document_api/project_overview.md and
delete that line (or replace it with an actual title if intended) so no empty
top-level heading remains.
---
Outside diff comments:
In `@app/desktop/studio_server/eval_api.py`:
- Around line 524-552: The update_eval handler currently assigns
request.train_set_filter_id into eval.train_set_filter_id without validating it;
before setting it (inside update_eval and after the existing "already set"
guard), call the same resolver used elsewhere (dataset_filter_from_id) with the
project_id and request.train_set_filter_id to ensure the filter exists/loads
successfully, and if that resolver fails or returns None raise an HTTPException
400 with a clear message; only assign eval.train_set_filter_id after successful
validation so typos or invalid IDs fail fast on PATCH.
In `@app/desktop/studio_server/settings_api.py`:
- Around line 46-77: The open_project_folder handler currently catches all
exceptions and returns HTTP 500 with detail=str(e), which masks intended
HTTPExceptions (e.g., from project_from_id) and leaks internals; change the
error handling in open_project_folder so that it does not convert existing
HTTPException instances—either remove the broad try/except or re-raise if
isinstance(e, HTTPException)—and for unexpected errors raise a new
HTTPException(status_code=500, detail="Internal server error") (do not include
str(e)); keep the call to project_from_id and open_folder unchanged so
project_from_id can raise its own 404 as designed.
In `@libs/server/kiln_server/project_api.py`:
- Around line 107-109: Guard against Config.shared().projects being None or
non-list before filtering: retrieve projects_before = Config.shared().projects,
if not isinstance(projects_before, list) then normalize it to an empty list (or
to list(projects_before) if you expect other iterables), compute projects_after
by filtering that normalized list (e.g., [p for p in normalized if p !=
str(project.path)]), and then call Config.shared().save_setting("projects",
projects_after); update the code around projects_before/projects_after to use
the normalized variable so deletion is resilient to None or wrong types.
- Around line 65-68: The code calls Project.model_validate(...) but ignores its
return value, so coercions/validators from Project aren't applied before saving;
replace the sequence that uses
updated_project.model_copy(update=project_updates) followed by a discarded
Project.model_validate(...) by validating and using the validated instance—call
Project.model_validate(...) (or updated_project.model_validate() if available)
and assign its result to updated_project, then call
updated_project.save_to_file() so the validated/coerced model is what's
persisted; reference updated_project, Project.model_validate, model_copy, and
save_to_file when making the change.
In `@libs/server/kiln_server/test_task_api.py`:
- Around line 96-104: The test's HTTPException side effect on project_from_id
isn't applied because the client.post call is outside the with patch(...)
context; move the request into the same with
patch("kiln_server.task_api.project_from_id") as mock_load: block so
mock_load.side_effect = HTTPException(...) is active when calling
client.post("/api/projects/FAKEPROJECTID/tasks", json=task_data), then assert
response.status_code and response.json()["message"] as written.
---
Nitpick comments:
In `@app/desktop/studio_server/settings_api.py`:
- Around line 23-67: Add explicit return type annotations to the modified route
handlers: annotate update_settings to return dict[str, Any] (it returns
Config.shared().settings(...)), read_setting_item to return dict[str,
Optional[Any]] (it returns {item_id: settings.get(item_id, None)}), and annotate
open_logs and open_project_folder to return dict[str, str] (they return
{"message": "opened"} or raise HTTPException). Ensure necessary typing imports
(e.g., Any, Optional, Dict) are present and update the function signatures for
update_settings, read_setting_item, open_logs, and open_project_folder
accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7210cf07-3a8b-4972-9ce1-6af528a42ce4
⛔ Files ignored due to path filters (1)
app/web_ui/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (74)
app/desktop/studio_server/api_models/copilot_models.pyapp/desktop/studio_server/copilot_api.pyapp/desktop/studio_server/data_gen_api.pyapp/desktop/studio_server/eval_api.pyapp/desktop/studio_server/finetune_api.pyapp/desktop/studio_server/import_api.pyapp/desktop/studio_server/prompt_api.pyapp/desktop/studio_server/prompt_optimization_job_api.pyapp/desktop/studio_server/provider_api.pyapp/desktop/studio_server/repair_api.pyapp/desktop/studio_server/run_config_api.pyapp/desktop/studio_server/settings_api.pyapp/desktop/studio_server/skill_api.pyapp/desktop/studio_server/test_eval_api.pyapp/desktop/studio_server/test_prompt_api.pyapp/desktop/studio_server/test_provider_api.pyapp/desktop/studio_server/test_repair_api.pyapp/desktop/studio_server/test_run_config_api.pyapp/desktop/studio_server/test_server.pyapp/desktop/studio_server/tool_api.pyapp/web_ui/src/lib/api_schema.d.tsapp/web_ui/src/lib/stores.tsapp/web_ui/src/lib/stores/evals_store.tsapp/web_ui/src/lib/stores/prompts_store.tsapp/web_ui/src/lib/stores/run_configs_store.tsapp/web_ui/src/routes/(app)/docs/rag_configs/[project_id]/table_rag_config_row.svelteapp/web_ui/src/routes/(app)/generate/[project_id]/[task_id]/synth_data_guidance_datamodel.test.tsapp/web_ui/src/routes/(app)/generate/[project_id]/[task_id]/synth_data_guidance_datamodel.tsapp/web_ui/src/routes/(app)/optimize/[project_id]/[task_id]/+page.svelteapp/web_ui/src/routes/(app)/prompt_optimization/[project_id]/[task_id]/create_prompt_optimization_job/+page.svelteapp/web_ui/src/routes/(app)/prompts/[project_id]/[task_id]/create/+page.svelteapp/web_ui/src/routes/(app)/prompts/[project_id]/[task_id]/edit_base_prompt/+page.svelteapp/web_ui/src/routes/(app)/run/output_repair_edit_form.svelteapp/web_ui/src/routes/(app)/run/run.svelteapp/web_ui/src/routes/(app)/specs/[project_id]/[task_id]/[spec_id]/+page.svelteapp/web_ui/src/routes/(app)/specs/[project_id]/[task_id]/[spec_id]/[eval_id]/+page.svelteapp/web_ui/src/routes/(app)/specs/[project_id]/[task_id]/[spec_id]/[eval_id]/[eval_config_id]/[run_config_id]/run_result/+page.svelteapp/web_ui/src/routes/(app)/specs/[project_id]/[task_id]/[spec_id]/[eval_id]/compare_run_configs/+page.svelteapp/web_ui/src/routes/(app)/specs/[project_id]/[task_id]/[spec_id]/[eval_id]/create_eval_config/+page.svelteapp/web_ui/src/routes/(app)/specs/[project_id]/[task_id]/[spec_id]/[eval_id]/eval_configs/+page.svelteapp/web_ui/src/routes/(app)/specs/[project_id]/[task_id]/compare/+page.svelteapp/web_ui/src/routes/(app)/specs/[project_id]/[task_id]/spec_builder/+page.svelteapp/web_ui/src/routes/(fullscreen)/setup/(setup)/connect_providers/connect_providers.svelteapp/web_ui/src/routes/(fullscreen)/setup/(setup)/create_task/edit_task.sveltelibs/core/kiln_ai/datamodel/basemodel.pylibs/core/kiln_ai/datamodel/chunk.pylibs/core/kiln_ai/datamodel/datamodel_enums.pylibs/core/kiln_ai/datamodel/embedding.pylibs/core/kiln_ai/datamodel/eval.pylibs/core/kiln_ai/datamodel/extraction.pylibs/core/kiln_ai/datamodel/prompt_id.pylibs/core/kiln_ai/datamodel/rag.pylibs/core/kiln_ai/datamodel/reranker.pylibs/core/kiln_ai/datamodel/task.pylibs/core/kiln_ai/datamodel/task_output.pylibs/core/kiln_ai/datamodel/task_run.pylibs/core/kiln_ai/datamodel/tool_id.pylibs/core/kiln_ai/datamodel/vector_store.pylibs/server/kiln_server/document_api.pylibs/server/kiln_server/project_api.pylibs/server/kiln_server/prompt_api.pylibs/server/kiln_server/run_api.pylibs/server/kiln_server/server.pylibs/server/kiln_server/spec_api.pylibs/server/kiln_server/task_api.pylibs/server/kiln_server/test_prompt_api.pylibs/server/kiln_server/test_spec_api.pylibs/server/kiln_server/test_task_api.pyspecs/projects/document_api/architecture.mdspecs/projects/document_api/functional_spec.mdspecs/projects/document_api/implementation_plan.mdspecs/projects/document_api/phase_plans/phase_3.mdspecs/projects/document_api/phase_plans/phase_4.mdspecs/projects/document_api/project_overview.md
| @@ -0,0 +1,105 @@ | |||
| --- | |||
| status: compelete | |||
Contributor
There was a problem hiding this comment.
Fix frontmatter status typo to avoid tooling mismatch.
status: compelete is misspelled and may break any automation expecting canonical status values.
✏️ Proposed fix
-status: compelete
+status: complete📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| status: compelete | |
| status: complete |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@specs/projects/document_api/implementation_plan.md` at line 2, The
frontmatter contains a typo: the value for the status key is written as
"compelete"; update the frontmatter so the status key reads "status: complete"
(correct spelling) to match tooling expectations and canonical values, ensure
YAML/frontmatter formatting remains valid and, if present elsewhere in the file,
replace any other instances of "compelete" with "complete".
Collaborator
Author
There was a problem hiding this comment.
Spec file — no automation reads this field, not worth a commit.
Contributor
There was a problem hiding this comment.
@scosman Understood — since no tooling depends on that field, the typo is harmless. Happy to drop it.
🧠 Learnings used
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.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/desktop/studio_server/tool_api.py (1)
725-736:⚠️ Potential issue | 🟡 MinorMissing OpenAPI tag and parameter descriptions for consistency.
This endpoint is missing the
tags=["Tools & MCP"]decorator andAnnotated[..., Path(description=...)]parameter annotations that were added to all other tool server endpoints in this file.🔧 Proposed fix
- `@app.post`("/api/projects/{project_id}/tool_servers/{tool_server_id}/archive") + `@app.post`( + "/api/projects/{project_id}/tool_servers/{tool_server_id}/archive", + tags=["Tools & MCP"], + ) async def archive_tool_server( - project_id: str, - tool_server_id: str, + project_id: Annotated[ + str, Path(description="The unique identifier of the project.") + ], + tool_server_id: Annotated[ + str, Path(description="The unique identifier of the tool server.") + ], archive_request: ToolServerArchiveRequest, ) -> ExternalToolServer:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/desktop/studio_server/tool_api.py` around lines 725 - 736, Add the missing OpenAPI metadata to archive_tool_server: decorate the endpoint with tags=["Tools & MCP"] and change the parameters to use Annotated types with Path descriptions (e.g., annotate project_id and tool_server_id with Annotated[str, Path(description="Project ID")] and Annotated[str, Path(description="Tool server ID")]) and ensure archive_request keeps its request body typing; update the function signature and decorator for archive_tool_server to match the other tool server endpoints (keeping the return type ExternalToolServer).libs/core/kiln_ai/datamodel/basemodel.py (1)
43-44:⚠️ Potential issue | 🟡 MinorRemove the unused
ID_FIELDconstant.The
ID_FIELDconstant on line 43 is no longer used after theidfield was inlined on line 298. It's dead code and should be removed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/core/kiln_ai/datamodel/basemodel.py` around lines 43 - 44, Delete the unused constant ID_FIELD (the line "ID_FIELD = Field(default_factory=generate_model_id)") from basemodel.py; ensure no other code references ID_FIELD and, if removing it makes generate_model_id or its import unused, also remove the now-unreferenced import or definition to keep the module clean.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@app/desktop/studio_server/tool_api.py`:
- Around line 725-736: Add the missing OpenAPI metadata to archive_tool_server:
decorate the endpoint with tags=["Tools & MCP"] and change the parameters to use
Annotated types with Path descriptions (e.g., annotate project_id and
tool_server_id with Annotated[str, Path(description="Project ID")] and
Annotated[str, Path(description="Tool server ID")]) and ensure archive_request
keeps its request body typing; update the function signature and decorator for
archive_tool_server to match the other tool server endpoints (keeping the return
type ExternalToolServer).
In `@libs/core/kiln_ai/datamodel/basemodel.py`:
- Around line 43-44: Delete the unused constant ID_FIELD (the line "ID_FIELD =
Field(default_factory=generate_model_id)") from basemodel.py; ensure no other
code references ID_FIELD and, if removing it makes generate_model_id or its
import unused, also remove the now-unreferenced import or definition to keep the
module clean.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 542c45c1-e1b4-4e9a-8a19-7a3c92f1a826
📒 Files selected for processing (5)
app/desktop/studio_server/test_server.pyapp/desktop/studio_server/tool_api.pyapp/web_ui/src/lib/api_schema.d.tsapp/web_ui/src/routes/(app)/prompt_optimization/[project_id]/[task_id]/create_prompt_optimization_job/+page.sveltelibs/core/kiln_ai/datamodel/basemodel.py
✅ Files skipped from review due to trivial changes (1)
- app/web_ui/src/routes/(app)/prompt_optimization/[project_id]/[task_id]/create_prompt_optimization_job/+page.svelte
🚧 Files skipped from review as they are similar to previous changes (1)
- app/desktop/studio_server/test_server.py
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
app/desktop/studio_server/tool_api.py (3)
120-121: Consider addingField(description=...)for consistency.Other request models in this file have been updated with
Field(description=...)metadata, butToolServerArchiveRequestwas not updated. For API documentation consistency:♻️ Optional enhancement
class ToolServerArchiveRequest(BaseModel): - is_archived: bool + """Request to archive or unarchive a tool server.""" + + is_archived: bool = Field(description="Whether to archive (true) or unarchive (false) the tool server.")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/desktop/studio_server/tool_api.py` around lines 120 - 121, Add a Field description to the ToolServerArchiveRequest model so it matches other request models: update the is_archived attribute in class ToolServerArchiveRequest to use Field(description="...") with a short description explaining the boolean flag (e.g., whether the tool server should be archived or unarchived) to ensure consistent OpenAPI docs and schema metadata.
729-732: MissingPath(description=...)annotations for consistency.Other endpoints in this file annotate
project_idandtool_server_idwithAnnotated[str, Path(description=...)], but this endpoint uses plainstrparameters.♻️ Suggested fix for consistency
async def archive_tool_server( - project_id: str, - tool_server_id: str, + project_id: Annotated[ + str, Path(description="The unique identifier of the project.") + ], + tool_server_id: Annotated[ + str, Path(description="The unique identifier of the tool server.") + ], archive_request: ToolServerArchiveRequest, ) -> ExternalToolServer:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/desktop/studio_server/tool_api.py` around lines 729 - 732, The archive_tool_server endpoint parameters project_id and tool_server_id are missing the Annotated Path descriptions; update the function signature in archive_tool_server to annotate both project_id and tool_server_id using Annotated[str, Path(description="...")] (matching the description text style used elsewhere in this file) so they mirror other endpoints' parameter metadata.
900-910: Consider removing redundant Args documentation.The docstring's
Args:section (lines 907-909) duplicates thePath(description=...)annotations. Since OpenAPI will use thePathdescriptions, the docstring Args can be removed to avoid duplication and maintenance burden.♻️ Optional cleanup
async def get_tool_definition( ... ) -> ToolDefinitionResponse: """ Get the actual OpenAI tool definition for a specific tool ID. This returns the real function name and parameters that would be used in OpenAI function calls, not the display names from ToolSetApiDescription. - - Args: - project_id: The project ID - task_id: The task ID for tools that require task context - tool_id: The tool ID to get the definition for """🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/desktop/studio_server/tool_api.py` around lines 900 - 910, The docstring for the "Get the actual OpenAI tool definition..." function duplicates the Path(description=...) metadata for project_id, task_id, and tool_id; remove the redundant "Args:" section (the lines documenting project_id, task_id, tool_id) from that function's docstring so OpenAPI uses the Path annotations as the single source of truth and leave only the short summary and any non-parameter notes; locate the docstring by searching for that summary string in tool_api.py and update it (e.g., in the function handling tool retrieval that mentions project_id, task_id, tool_id).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/desktop/studio_server/tool_api.py`:
- Around line 120-121: Add a Field description to the ToolServerArchiveRequest
model so it matches other request models: update the is_archived attribute in
class ToolServerArchiveRequest to use Field(description="...") with a short
description explaining the boolean flag (e.g., whether the tool server should be
archived or unarchived) to ensure consistent OpenAPI docs and schema metadata.
- Around line 729-732: The archive_tool_server endpoint parameters project_id
and tool_server_id are missing the Annotated Path descriptions; update the
function signature in archive_tool_server to annotate both project_id and
tool_server_id using Annotated[str, Path(description="...")] (matching the
description text style used elsewhere in this file) so they mirror other
endpoints' parameter metadata.
- Around line 900-910: The docstring for the "Get the actual OpenAI tool
definition..." function duplicates the Path(description=...) metadata for
project_id, task_id, and tool_id; remove the redundant "Args:" section (the
lines documenting project_id, task_id, tool_id) from that function's docstring
so OpenAPI uses the Path annotations as the single source of truth and leave
only the short summary and any non-parameter notes; locate the docstring by
searching for that summary string in tool_api.py and update it (e.g., in the
function handling tool retrieval that mentions project_id, task_id, tool_id).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e84bacfc-93cc-44e7-af89-ec6bb1be519e
📒 Files selected for processing (3)
app/desktop/studio_server/test_server.pyapp/desktop/studio_server/tool_api.pylibs/server/kiln_server/server.py
- Move api_key from query params to JSON request body for OpenAI compatible provider endpoint to prevent secret leakage in logs/URLs - Fix docker_model_runner connect call missing params.query nesting - Fix eval_config_id path param type from str | None to str - Update run_calibration endpoint summary to match path - Fix splits form field description to match validation (numeric proportions) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/desktop/studio_server/eval_api.py (1)
843-845: Add explicitoperation_idto align the calibration endpoint with its path.The route handler
run_eval_config_evaldoes not match the endpoint path/run_calibration. FastAPI derives the defaultoperationIdfrom the handler name, so the generated OpenAPI schema showsrun_eval_config_evalas the operation identifier. Settingoperation_id="run_calibration"keeps the schema semantically consistent with the endpoint path.🛠️ Suggested fix
`@app.get`( "/api/projects/{project_id}/tasks/{task_id}/evals/{eval_id}/run_calibration", summary="Run Calibration", + operation_id="run_calibration", tags=["Evals"], )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/desktop/studio_server/eval_api.py` around lines 843 - 845, The OpenAPI operationId is derived from the handler name and currently becomes "run_eval_config_eval" which doesn't match the endpoint path; update the route decorator for the calibration endpoint (the route that uses the handler run_eval_config_eval) to include operation_id="run_calibration" so the generated schema uses a semantically consistent operationId that matches "/run_calibration".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/desktop/studio_server/eval_api.py`:
- Around line 743-745: The frontend component run_eval.svelte still calls the
old backend endpoints (".../eval/.../eval_config/.../run_task_run_eval" and
".../eval/.../run_eval_config_eval"); update those fetch/XHR calls to the
renamed routes (".../evals/.../run_comparison" and
".../evals/.../run_calibration") so the run and calibration flows hit the new
backend handlers; search for any other occurrences in run_eval.svelte (and
related UI callers) and replace the old path segments ("eval/" and the two
specific action names) with "evals/" and the corresponding new action names to
avoid 404s.
---
Nitpick comments:
In `@app/desktop/studio_server/eval_api.py`:
- Around line 843-845: The OpenAPI operationId is derived from the handler name
and currently becomes "run_eval_config_eval" which doesn't match the endpoint
path; update the route decorator for the calibration endpoint (the route that
uses the handler run_eval_config_eval) to include operation_id="run_calibration"
so the generated schema uses a semantically consistent operationId that matches
"/run_calibration".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f8f05693-1f82-493a-9f8c-dccf58f95220
📒 Files selected for processing (6)
app/desktop/studio_server/eval_api.pyapp/desktop/studio_server/provider_api.pyapp/desktop/studio_server/test_provider_api.pyapp/web_ui/src/lib/api_schema.d.tsapp/web_ui/src/routes/(fullscreen)/setup/(setup)/connect_providers/connect_providers.sveltelibs/server/kiln_server/run_api.py
✅ Files skipped from review due to trivial changes (1)
- app/desktop/studio_server/test_provider_api.py
🚧 Files skipped from review as they are similar to previous changes (2)
- app/desktop/studio_server/provider_api.py
- libs/server/kiln_server/run_api.py
Update 3 frontend files that still referenced old API paths after the Phase 4 rename (eval→evals, run_config→run_configs, endpoint renames). Fix wrong error message in task update endpoint. Correct misleading passthrough_mimetypes and gen_type descriptions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds a regression test that checks every path/query parameter in the OpenAPI schema has a description. Fixes the 5 parameters that were missing descriptions (gen_prompt and archive_tool_server endpoints). Also adds docstrings to 5 ambiguous endpoints (execute run, generate/save repair, eval comparison/calibration). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… docs - Fix runtime bug: task deletion URL used old /task/ path instead of /tasks/ - Fix /api/project singular paths to /api/projects for consistency - Remove unused ID_FIELD constant from basemodel.py - Restore "should not be changed after creation" contract on Document.name - Improve tautological Field descriptions across API models - Fix docstring nits (ambiguous "model" wording, echoed descriptions) - Fix test assertion for corrected "Failed to update task" error message - Add trailing slash rule to API code review guidelines Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ity with /run_configs/{run_config_id}
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
sfierro
approved these changes
Mar 31, 2026
chiang-daniel
approved these changes
Apr 1, 2026
|
|
||
| **Naming and path conventions:** | ||
|
|
||
| - **Always use plural nouns** in path segments: `/tasks/{task_id}`, never `/task/{task_id}`. Same for `/projects`, `/specs`, `/evals`, `/runs`, `/prompts`, `/documents`, `/skills`, `/run_configs`, etc. We had inconsistencies where GET used plural but POST/PATCH/DELETE used singular — this is confusing and must be caught. |
Contributor
There was a problem hiding this comment.
Would've love this to be a separate change to isolate updating API docs from changing the actual end points.
Collaborator
Author
There was a problem hiding this comment.
yeah.... to late but good point.
leonardmq
approved these changes
Apr 1, 2026
| return await connect_ollama(custom_ollama_url) | ||
|
|
||
| @app.get("/api/provider/docker_model_runner/connect") | ||
| @app.post( |
Collaborator
There was a problem hiding this comment.
We changed the method here from GET to POST.
Was there a reason for it being GET?
…penAPI docs, and our agent docs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Overhaul of the Kiln OpenAPI spec to make it usable for AI agents. Previously 87% of endpoints had no description, no parameters were documented, 65% of schemas were undescribed, paths were inconsistent (singular/plural mixed), and nothing was tagged.
Changes
Documentation-only (no behavior changes):
Breaking API changes (frontend updated in same PR):
Summary by CodeRabbit
New Features
Bug Fixes
Documentation