Conversation
Delete tessl.json, .tessl/, and .cursor/rules/.gitignore. Drop the tessl-managed block from AGENTS.md and the tessl MCP server entry from .cursor/mcp.json. Adds the root_cleanup project spec (phase 1).
Consolidates root-level test and coverage config into pyproject.toml: - Merged pytest.ini contents into [tool.pytest.ini_options] - Added [tool.coverage.run] with the existing omit list - Deleted .coveragerc and pytest.ini - Updated kiln-add-model skill docs to reference pyproject.toml - Ad-hoc `uv run pytest` now runs parallel by default (checks.sh already passed -n auto explicitly, so CI behavior is unchanged).
Relocates the three dev-bootstrap scripts (setup_env.sh, setup_claude.sh, pre-commit-hook) under .config/utils/ to reduce root-level clutter. Fixes PROJECT_ROOT resolution in setup_env.sh for the new two-deep location, and updates references in CONTRIBUTING.md, .config/wt.toml, and .config/wt/README.md.
Relocates MCP sessions planning docs into the canonical specs tree, removing the ad-hoc .planning/ directory from the repo root. Content-only change with no code references to update. Marks Phase 4 of the root cleanup project complete.
Cursor picks up rules from root AGENTS.md, so this file is stale. With it gone the .cursor/rules/ directory is empty and drops out of the tree. Phase 5 of the root_cleanup project.
Move tracked skill content from .cursor/skills/ to .agents/skills/ so there is a single canonical source. Fix the three hardcoded .cursor/skills/... script path references inside kiln-check-deprecation and gitignore .cursor/skills/ so it becomes a setup-script artifact (parallel to how .claude/ is handled). Phase 7 will add the setup scripts that regenerate .cursor/skills/ and .claude/skills/ from this new canonical location.
Relocates setup_claude.sh from .config/utils/ to .agents/claude/setup.sh, adds a parallel .agents/cursor/setup.sh, and points both at the canonical .agents/skills/ source established in Phase 6. Updates wt.toml and .config/wt/README.md for the new paths.
Relocate HooksMCP config to .config/hooks_mcp.yaml, rewrite prompt-file paths with ../ prefix, update .cursor/mcp.json args and CONTRIBUTING.md reference link.
Relocate test asset files and associated pytest fixtures closer to their primary consumers in libs/core, with a bridge conftest for libs/server.
Phase 10: relocate guides directory, remove 3.5 MB kiln_preview.avif, and update internal references in README.md and cross-links.
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:
📝 WalkthroughWalkthroughReorganizes repository root: consolidates agent skills under Changes
Sequence Diagram(s)(Skipped — changes are primarily file relocations, config updates, doc additions, and small setup scripts; no multi-component control-flow requiring diagram was produced.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 reorganizes the repository root by moving configuration files, utility scripts, and documentation into a .config directory, while also relocating test assets and fixtures to library-specific directories. Additionally, it removes Tessl integration and centralizes agent skills under .agents/. Feedback identifies an opportunity to reduce code duplication in libs/server/conftest.py by importing shared fixtures and suggests using a glob pattern in pyproject.toml to exclude all conftest.py files from coverage reports.
Comment on lines
+1
to
+72
| import shutil | ||
| import uuid | ||
| from pathlib import Path | ||
| from typing import Callable | ||
|
|
||
| import pytest | ||
|
|
||
| from libs.core.conftest import MockFileFactoryMimeType | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def test_data_dir() -> Path: | ||
| """ | ||
| The directory that contains test files with various mime types. | ||
| """ | ||
| return Path(__file__).resolve().parent.parent / "core" / "tests" / "assets" | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def mock_file_factory( | ||
| tmp_path, test_data_dir | ||
| ) -> Callable[[MockFileFactoryMimeType], Path]: | ||
| """ | ||
| Create a mock file factory that creates a mock file for the given mime type. | ||
| The file is copied to the tmp path so it is safe to alter it without contaminating the original file. | ||
| """ | ||
|
|
||
| def create_file(mime_type: MockFileFactoryMimeType) -> Path: | ||
| match mime_type: | ||
| # document | ||
| case MockFileFactoryMimeType.PDF: | ||
| filename = test_data_dir / "document_paper.pdf" | ||
| case MockFileFactoryMimeType.CSV: | ||
| filename = test_data_dir / "document_people.csv" | ||
| case MockFileFactoryMimeType.TXT: | ||
| filename = test_data_dir / "document_ice_cubes.txt" | ||
| case MockFileFactoryMimeType.HTML: | ||
| filename = test_data_dir / "document_ice_cubes.html" | ||
| case MockFileFactoryMimeType.MD: | ||
| filename = test_data_dir / "document_ice_cubes.md" | ||
|
|
||
| # images | ||
| case MockFileFactoryMimeType.PNG: | ||
| filename = test_data_dir / "image_kodim23.png" | ||
| case MockFileFactoryMimeType.JPG: | ||
| filename = test_data_dir / "image_nasa.jpg" | ||
| case MockFileFactoryMimeType.JPEG: | ||
| filename = test_data_dir / "image_nasa.jpeg" | ||
|
|
||
| # audio | ||
| case MockFileFactoryMimeType.OGG: | ||
| filename = test_data_dir / "audio_ice_cubes.ogg" | ||
| case MockFileFactoryMimeType.MP3: | ||
| filename = test_data_dir / "audio_ice_cubes.mp3" | ||
| case MockFileFactoryMimeType.WAV: | ||
| filename = test_data_dir / "audio_ice_cubes.wav" | ||
|
|
||
| # video | ||
| case MockFileFactoryMimeType.MP4: | ||
| filename = test_data_dir / "video_tv_bars.mp4" | ||
| case MockFileFactoryMimeType.MOV: | ||
| filename = test_data_dir / "video_tv_bars.mov" | ||
|
|
||
| case _: | ||
| raise ValueError(f"No test file found for mime type: {mime_type}") | ||
|
|
||
| path_copy = tmp_path / f"{uuid.uuid4()!s}.{filename.suffix}" | ||
| shutil.copy(filename, path_copy) | ||
|
|
||
| return path_copy | ||
|
|
||
| return create_file |
Contributor
There was a problem hiding this comment.
This file contains significant code duplication from libs/core/conftest.py (specifically the mock_file_factory fixture and test_data_dir). Additionally, it is missing the mock_attachment_factory fixture which was part of the implementation plan. Since MockFileFactoryMimeType is already successfully imported from libs.core.conftest on line 8, all required fixtures should be imported directly to ensure consistency and reduce maintenance overhead.
Suggested change
| import shutil | |
| import uuid | |
| from pathlib import Path | |
| from typing import Callable | |
| import pytest | |
| from libs.core.conftest import MockFileFactoryMimeType | |
| @pytest.fixture | |
| def test_data_dir() -> Path: | |
| """ | |
| The directory that contains test files with various mime types. | |
| """ | |
| return Path(__file__).resolve().parent.parent / "core" / "tests" / "assets" | |
| @pytest.fixture | |
| def mock_file_factory( | |
| tmp_path, test_data_dir | |
| ) -> Callable[[MockFileFactoryMimeType], Path]: | |
| """ | |
| Create a mock file factory that creates a mock file for the given mime type. | |
| The file is copied to the tmp path so it is safe to alter it without contaminating the original file. | |
| """ | |
| def create_file(mime_type: MockFileFactoryMimeType) -> Path: | |
| match mime_type: | |
| # document | |
| case MockFileFactoryMimeType.PDF: | |
| filename = test_data_dir / "document_paper.pdf" | |
| case MockFileFactoryMimeType.CSV: | |
| filename = test_data_dir / "document_people.csv" | |
| case MockFileFactoryMimeType.TXT: | |
| filename = test_data_dir / "document_ice_cubes.txt" | |
| case MockFileFactoryMimeType.HTML: | |
| filename = test_data_dir / "document_ice_cubes.html" | |
| case MockFileFactoryMimeType.MD: | |
| filename = test_data_dir / "document_ice_cubes.md" | |
| # images | |
| case MockFileFactoryMimeType.PNG: | |
| filename = test_data_dir / "image_kodim23.png" | |
| case MockFileFactoryMimeType.JPG: | |
| filename = test_data_dir / "image_nasa.jpg" | |
| case MockFileFactoryMimeType.JPEG: | |
| filename = test_data_dir / "image_nasa.jpeg" | |
| # audio | |
| case MockFileFactoryMimeType.OGG: | |
| filename = test_data_dir / "audio_ice_cubes.ogg" | |
| case MockFileFactoryMimeType.MP3: | |
| filename = test_data_dir / "audio_ice_cubes.mp3" | |
| case MockFileFactoryMimeType.WAV: | |
| filename = test_data_dir / "audio_ice_cubes.wav" | |
| # video | |
| case MockFileFactoryMimeType.MP4: | |
| filename = test_data_dir / "video_tv_bars.mp4" | |
| case MockFileFactoryMimeType.MOV: | |
| filename = test_data_dir / "video_tv_bars.mov" | |
| case _: | |
| raise ValueError(f"No test file found for mime type: {mime_type}") | |
| path_copy = tmp_path / f"{uuid.uuid4()!s}.{filename.suffix}" | |
| shutil.copy(filename, path_copy) | |
| return path_copy | |
| return create_file | |
| from libs.core.conftest import ( | |
| MockFileFactoryMimeType, | |
| mock_attachment_factory, | |
| mock_file_factory, | |
| test_data_dir, | |
| ) |
Collaborator
Author
There was a problem hiding this comment.
Reviewed but not implemented.
conftest.py files are pytest plugins, not regular importable modules. They're auto-discovered by directory hierarchy and aren't designed for cross-package imports. The shared type (MockFileFactoryMimeType) is correctly exported via kiln_ai.pytest_mock_files, while fixtures remain local to each package's conftest — this is the intended pattern.
Reviewed by AI coding agent via /spec pr
![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: 7
🧹 Nitpick comments (1)
libs/core/conftest.py (1)
106-114: Minor: pass the string value of the enum tofrom_datafor clarity.
MockFileFactoryMimeTypeis astrEnum so this works today, butKilnAttachmentModel.from_data(data, mime_type: str)is typed as plainstr. Passingmime_type.value(or annotating the accepted types) avoids future surprises iffrom_dataever validates viaisinstance(..., str)strictly or is switched to a stricter type.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/core/conftest.py` around lines 106 - 114, The create_attachment helper currently passes the MockFileFactoryMimeType enum directly to KilnAttachmentModel.from_data; change it to pass the enum's string value (e.g., mime_type.value) so the call to from_data(data, mime_type: str) always receives a plain str; update the branch that calls KilnAttachmentModel.from_data in create_attachment to use mime_type.value to avoid future isinstance/type issues with the MockFileFactoryMimeType enum.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@libs/core/conftest.py`:
- Around line 91-92: The generated filename adds an extra dot because
filename.suffix already includes the leading '.', so change the construction of
path_copy (currently using f"{uuid.uuid4()!s}.{filename.suffix}") to concatenate
the UUID and the suffix without inserting an extra dot (i.e., use uuid +
filename.suffix or strip the leading dot from filename.suffix before adding one)
so that path_copy uses "<uuid>.pdf" instead of "<uuid>..pdf"; update the
expression where path_copy is set and keep the subsequent shutil.copy(filename,
path_copy) unchanged.
In `@libs/server/conftest.py`:
- Line 67: The generated filename adds a redundant dot because filename.suffix
already includes the leading '.'; update the expression that sets path_copy so
it does not insert an extra dot (use the uuid string concatenated directly with
filename.suffix, or strip the leading dot from filename.suffix before adding a
dot), locating the change where path_copy is assigned (path_copy, tmp_path,
uuid.uuid4(), filename.suffix).
In `@pyproject.toml`:
- Around line 83-89: Update the coverage omit pattern for pytest fixtures in the
[tool.coverage.run] omit list: replace the bare "conftest.py" entry with a
recursive glob "**/conftest.py" so all nested conftest.py files (e.g.,
libs/core/conftest.py and libs/server/conftest.py) are omitted from coverage;
modify the omit array containing entries like "**/test_*.py",
"libs/core/kiln_ai/adapters/ml_model_list.py", "conftest.py",
"*/kiln_ai_server_client/*" to use "**/conftest.py" instead of "conftest.py".
In `@specs/projects/root_cleanup/architecture.md`:
- Line 43: The architecture doc claims 2 hardcoded path references to fix in
kiln-check-deprecation/SKILL.md while the functional_spec.md claims 3; reconcile
them by opening kiln-check-deprecation/SKILL.md, counting actual hardcoded path
refs and updating either architecture.md or functional_spec.md so both state the
verified count (and optionally add a short note listing the three paths if 3),
referencing the file name kiln-check-deprecation/SKILL.md and the two docs
architecture.md and functional_spec.md to locate where to change.
In `@specs/projects/root_cleanup/implementation_plan.md`:
- Around line 25-27: Phase 3 and Phase 7 disagree on the intermediate location
of setup_claude.sh; pick one consistent intermediate path and update both phases
and any references accordingly (either keep setup_claude.sh excluded from the
utils/ move and change Phase 7 to use utils/setup_claude.sh, or include
setup_claude.sh in the git mv to .config/utils/ in Phase 3 so Phase 7 can
continue to expect .config/utils/setup_claude.sh before moving to
.agents/claude/setup.sh); update the specific mentions of setup_claude.sh,
utils/, .config/utils/, Phase 3, Phase 7 and any other referenced lines in the
plan so they all reference the same chosen intermediate path.
In `@specs/projects/root_cleanup/phase_plans/phase_2.md`:
- Around line 30-35: Update the doc text to stop claiming pyproject.toml only
contains addopts and instead state that pyproject.toml already contains
asyncio_mode, asyncio_default_fixture_loop_scope, markers, and filterwarnings;
instruct the reader to reconcile/verify the pytest.ini settings into the
existing [tool.pytest.ini_options] table (preserving addopts="-n auto" as
commented), ensure asyncio_default_fixture_loop_scope is unquoted as a TOML
string, and confirm markers and filterwarnings are represented as TOML lists;
reference pyproject.toml, pytest.ini, and the keys asyncio_mode,
asyncio_default_fixture_loop_scope, markers, filterwarnings, addopts in the
updated wording.
In `@specs/projects/root_cleanup/phase_plans/phase_9.md`:
- Around line 9-10: The phase doc references fixtures/imports that aren't
defined in the root conftest; locate the actual pre-move definitions of
MockFileFactoryMimeType, test_data_dir, mock_file_factory, and
mock_attachment_factory in the repository (where they currently live), then
update the phase plan text to list that actual pre-move location instead of
"root conftest.py" and adjust the import rewrite instructions accordingly (also
update the same corrections mentioned for lines 17–19); reference the fixture
names above so reviewers can verify the doc matches the real code location.
---
Nitpick comments:
In `@libs/core/conftest.py`:
- Around line 106-114: The create_attachment helper currently passes the
MockFileFactoryMimeType enum directly to KilnAttachmentModel.from_data; change
it to pass the enum's string value (e.g., mime_type.value) so the call to
from_data(data, mime_type: str) always receives a plain str; update the branch
that calls KilnAttachmentModel.from_data in create_attachment to use
mime_type.value to avoid future isinstance/type issues with the
MockFileFactoryMimeType enum.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8612e83a-897b-48cf-a724-a812d7e87a07
⛔ Files ignored due to path filters (11)
.config/legacy_guides/kiln_preview.gifis excluded by!**/*.giflibs/core/tests/assets/audio_ice_cubes.mp3is excluded by!**/*.mp3libs/core/tests/assets/audio_ice_cubes.oggis excluded by!**/*.ogglibs/core/tests/assets/audio_ice_cubes.wavis excluded by!**/*.wavlibs/core/tests/assets/document_paper.pdfis excluded by!**/*.pdflibs/core/tests/assets/document_people.csvis excluded by!**/*.csvlibs/core/tests/assets/image_kodim23.pngis excluded by!**/*.pnglibs/core/tests/assets/image_nasa.jpegis excluded by!**/*.jpeglibs/core/tests/assets/image_nasa.jpgis excluded by!**/*.jpglibs/core/tests/assets/video_tv_bars.movis excluded by!**/*.movlibs/core/tests/assets/video_tv_bars.mp4is excluded by!**/*.mp4
📒 Files selected for processing (61)
.agents/claude/setup.sh.agents/cursor/setup.sh.agents/skills/kiln-add-model/SKILL.md.agents/skills/kiln-check-deprecation/SKILL.md.agents/skills/kiln-check-deprecation/scripts/check_provider.py.agents/skills/kiln-check-deprecation/scripts/extract_models.py.config/hooks_mcp.yaml.config/legacy_guides/Collaborating with Kiln.md.config/legacy_guides/Fine Tuning LLM Models Guide.md.config/legacy_guides/Model Support.md.config/legacy_guides/Synthetic Data Generation.md.config/utils/pre-commit-hook.config/utils/setup_env.sh.config/wt.toml.config/wt/README.md.coveragerc.cursor/mcp.json.cursor/rules/.gitignore.cursor/rules/project.mdc.gitignore.tessl/.gitignoreAGENTS.mdCONTRIBUTING.mdREADME.mdconftest.pyguides/kiln_preview.aviflibs/core/conftest.pylibs/core/kiln_ai/adapters/extractors/test_encoding.pylibs/core/kiln_ai/adapters/extractors/test_extractor_runner.pylibs/core/kiln_ai/adapters/extractors/test_litellm_extractor.pylibs/core/kiln_ai/datamodel/test_attachment.pylibs/core/kiln_ai/utils/test_pdf_utils.pylibs/core/tests/assets/README.mdlibs/core/tests/assets/document_ice_cubes.htmllibs/core/tests/assets/document_ice_cubes.mdlibs/core/tests/assets/document_ice_cubes.txtlibs/server/conftest.pylibs/server/kiln_server/test_document_api.pypyproject.tomlpytest.inispecs/projects/mcp_sessions/01_codebase_findings.mdspecs/projects/mcp_sessions/02_design_report.mdspecs/projects/mcp_sessions/03_refinement_general_session_id.mdspecs/projects/mcp_sessions/04_refinement_no_ephemeral_fallback.mdspecs/projects/mcp_sessions/implementation_plan.mdspecs/projects/root_cleanup/architecture.mdspecs/projects/root_cleanup/functional_spec.mdspecs/projects/root_cleanup/implementation_plan.mdspecs/projects/root_cleanup/phase_plans/phase_1.mdspecs/projects/root_cleanup/phase_plans/phase_10.mdspecs/projects/root_cleanup/phase_plans/phase_2.mdspecs/projects/root_cleanup/phase_plans/phase_3.mdspecs/projects/root_cleanup/phase_plans/phase_4.mdspecs/projects/root_cleanup/phase_plans/phase_5.mdspecs/projects/root_cleanup/phase_plans/phase_6.mdspecs/projects/root_cleanup/phase_plans/phase_7.mdspecs/projects/root_cleanup/phase_plans/phase_8.mdspecs/projects/root_cleanup/phase_plans/phase_9.mdspecs/projects/root_cleanup/project_overview.mdtessl.jsonutils/setup_claude.sh
💤 Files with no reviewable changes (9)
- .cursor/rules/.gitignore
- .tessl/.gitignore
- .cursor/rules/project.mdc
- .coveragerc
- tessl.json
- utils/setup_claude.sh
- pytest.ini
- conftest.py
- AGENTS.md
|
|
||
| Phase order inside item 8: | ||
| 1. Create `.agents/skills/` and copy tracked content from `.cursor/skills/`. | ||
| 2. Fix hardcoded path refs inside the moved files (`kiln-check-deprecation/SKILL.md` × 2). |
Contributor
There was a problem hiding this comment.
Unify hardcoded-reference count with functional spec.
This file says there are 2 path refs to fix in kiln-check-deprecation/SKILL.md, while functional_spec.md says 3. Please reconcile both docs to a single verified count.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@specs/projects/root_cleanup/architecture.md` at line 43, The architecture doc
claims 2 hardcoded path references to fix in kiln-check-deprecation/SKILL.md
while the functional_spec.md claims 3; reconcile them by opening
kiln-check-deprecation/SKILL.md, counting actual hardcoded path refs and
updating either architecture.md or functional_spec.md so both state the verified
count (and optionally add a short note listing the three paths if 3),
referencing the file name kiln-check-deprecation/SKILL.md and the two docs
architecture.md and functional_spec.md to locate where to change.
Collaborator
Author
There was a problem hiding this comment.
Reviewed but not implemented.
These are planning docs that served their purpose during implementation. The code is the source of truth — we don't retroactively fix spec artifacts after the work is done.
Reviewed by AI coding agent via /spec pr
Contributor
There was a problem hiding this comment.
@scosman, understood — planning docs are ephemeral artifacts and the implemented code is the source of truth. I'll avoid flagging inconsistencies in completed planning/spec docs going forward.
🧠 Learnings used
Learnt from: scosman
Repo: Kiln-AI/Kiln PR: 1182
File: specs/projects/document_api/functional_spec.md:27-39
Timestamp: 2026-03-31T01:50:03.759Z
Learning: In the Kiln-AI/Kiln repo, do not flag or request fixes for markdownlint issues in spec/documentation Markdown files under `specs/` (e.g., MD040 fenced-code-language). These markdownlint findings are intentionally treated as acceptable and are not worth churn for files like `specs/**/*.md`.
Learnt from: sfierro
Repo: Kiln-AI/Kiln PR: 1239
File: app/web_ui/src/routes/(app)/specs/[project_id]/[task_id]/[spec_id]/+page.svelte:161-166
Timestamp: 2026-04-07T22:57:02.847Z
Learning: In the Kiln AI codebase (app/web_ui/src/routes/(app)/specs/), the Archive/Unarchive toggle for specs always restores to "active" status on unarchive, regardless of the prior status (e.g., "future" or "deprecated"). This is intentional and consistent with the bulk archive/unarchive behavior on the spec list page.
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.
Comment on lines
+30
to
+35
| 2. **Edit `pyproject.toml`**: merge `pytest.ini` contents into the existing `[tool.pytest.ini_options]` table. Current state only has `addopts="-n auto"` (commented-out in pytest.ini, so we keep it as-is in pyproject). Add: | ||
| - `asyncio_mode = "auto"` | ||
| - `asyncio_default_fixture_loop_scope = "function"` (unquote the INI value — in TOML the string needs one layer of quoting, no inner quotes) | ||
| - `markers = [...]` as a list (TOML equivalent of the INI multi-line list) | ||
| - `filterwarnings = [...]` as a list | ||
|
|
Contributor
There was a problem hiding this comment.
Update stale statement about current pytest config state.
Line 30 claims only addopts="-n auto" exists, but current pyproject.toml already includes asyncio_mode, asyncio_default_fixture_loop_scope, markers, and filterwarnings. Please adjust this section to describe reconciliation/verification instead of fresh addition.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@specs/projects/root_cleanup/phase_plans/phase_2.md` around lines 30 - 35,
Update the doc text to stop claiming pyproject.toml only contains addopts and
instead state that pyproject.toml already contains asyncio_mode,
asyncio_default_fixture_loop_scope, markers, and filterwarnings; instruct the
reader to reconcile/verify the pytest.ini settings into the existing
[tool.pytest.ini_options] table (preserving addopts="-n auto" as commented),
ensure asyncio_default_fixture_loop_scope is unquoted as a TOML string, and
confirm markers and filterwarnings are represented as TOML lists; reference
pyproject.toml, pytest.ini, and the keys asyncio_mode,
asyncio_default_fixture_loop_scope, markers, filterwarnings, addopts in the
updated wording.
Collaborator
Author
There was a problem hiding this comment.
Reviewed but not implemented.
These are planning docs that served their purpose during implementation. The code is the source of truth — we don't retroactively fix spec artifacts after the work is done.
Reviewed by AI coding agent via /spec pr
Contributor
There was a problem hiding this comment.
@scosman, understood — planning docs are historical artifacts and the code is the source of truth. I'll avoid flagging stale content in completed phase plan documents going forward.
🐇 Noted for future reviews — historical docs stay historical.
✏️ Learnings added
Learnt from: scosman
Repo: Kiln-AI/Kiln PR: 1339
File: specs/projects/root_cleanup/phase_plans/phase_2.md:30-35
Timestamp: 2026-04-24T01:23:39.041Z
Learning: In the Kiln-AI/Kiln repo, planning/phase-plan documents under `specs/projects/*/phase_plans/` (e.g., `specs/projects/root_cleanup/phase_plans/`) are treated as historical artifacts after implementation is complete. Do not flag or request fixes for stale or inaccurate content in these files — the code is the source of truth, and retroactive corrections to spec artifacts are intentionally not made.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: leonardmq
Repo: Kiln-AI/Kiln PR: 487
File: libs/server/kiln_server/test_document_api.py:1799-1873
Timestamp: 2025-09-10T08:17:31.156Z
Learning: The project uses pytest with asyncio_mode = auto in pytest.ini, which automatically detects and runs async test functions without requiring pytest.mark.asyncio decorators. This means async test functions in the codebase don't need the explicit asyncio marker.
Learnt from: scosman
Repo: Kiln-AI/Kiln PR: 1182
File: specs/projects/document_api/functional_spec.md:27-39
Timestamp: 2026-03-31T01:50:03.759Z
Learning: In the Kiln-AI/Kiln repo, do not flag or request fixes for markdownlint issues in spec/documentation Markdown files under `specs/` (e.g., MD040 fenced-code-language). These markdownlint findings are intentionally treated as acceptable and are not worth churn for files like `specs/**/*.md`.
Learnt from: leonardmq
Repo: Kiln-AI/Kiln PR: 487
File: libs/server/kiln_server/test_document_api.py:1799-1873
Timestamp: 2025-09-10T08:17:31.156Z
Learning: The project uses pytest with asyncio_mode = auto in pytest.ini, which automatically detects and runs async test functions without requiring pytest.mark.asyncio decorators.
Learnt from: sfierro
Repo: Kiln-AI/Kiln PR: 1239
File: app/web_ui/src/routes/(app)/specs/[project_id]/[task_id]/[spec_id]/+page.svelte:161-166
Timestamp: 2026-04-07T22:57:02.847Z
Learning: In the Kiln AI codebase (app/web_ui/src/routes/(app)/specs/), the Archive/Unarchive toggle for specs always restores to "active" status on unarchive, regardless of the prior status (e.g., "future" or "deprecated"). This is intentional and consistent with the bulk archive/unarchive behavior on the spec list page.
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.
Comment on lines
+9
to
+10
| Moves test asset files from the root `tests/assets/` directory into `libs/core/tests/assets/`, co-locating them with the core library that primarily consumes them. Migrates the associated pytest fixtures (`MockFileFactoryMimeType`, `test_data_dir`, `mock_file_factory`, `mock_attachment_factory`) from the root `conftest.py` into `libs/core/conftest.py`, and creates a bridge `libs/server/conftest.py` for the one server-side consumer. | ||
|
|
Contributor
There was a problem hiding this comment.
Phase steps reference fixtures/imports that are not present in root conftest.py.
The plan says these fixtures are moved/removed from root and imports are rewritten from conftest, but the provided conftest.py context does not define MockFileFactoryMimeType, test_data_dir, mock_file_factory, or mock_attachment_factory. Please update this phase doc to match the real pre-move location of those symbols.
Also applies to: 17-19
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@specs/projects/root_cleanup/phase_plans/phase_9.md` around lines 9 - 10, The
phase doc references fixtures/imports that aren't defined in the root conftest;
locate the actual pre-move definitions of MockFileFactoryMimeType,
test_data_dir, mock_file_factory, and mock_attachment_factory in the repository
(where they currently live), then update the phase plan text to list that actual
pre-move location instead of "root conftest.py" and adjust the import rewrite
instructions accordingly (also update the same corrections mentioned for lines
17–19); reference the fixture names above so reviewers can verify the doc
matches the real code location.
Collaborator
Author
There was a problem hiding this comment.
Reviewed but not implemented.
These are planning docs that served their purpose during implementation. The code is the source of truth — we don't retroactively fix spec artifacts after the work is done.
Reviewed by AI coding agent via /spec pr
Contributor
There was a problem hiding this comment.
@scosman, understood — completed phase plan docs are historical artifacts and the implementation code is the source of truth. I'll drop this.
✏️ Learnings added
Learnt from: scosman
Repo: Kiln-AI/Kiln PR: 1339
File: specs/projects/root_cleanup/phase_plans/phase_9.md:9-10
Timestamp: 2026-04-24T01:23:38.309Z
Learning: In the Kiln-AI/Kiln repo, do not flag issues in completed planning/phase-plan spec documents under `specs/projects/*/phase_plans/` (or similar paths). These docs serve their purpose during implementation and are not retroactively corrected after the work is done; the code is the source of truth.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: scosman
Repo: Kiln-AI/Kiln PR: 1182
File: specs/projects/document_api/functional_spec.md:27-39
Timestamp: 2026-03-31T01:50:03.759Z
Learning: In the Kiln-AI/Kiln repo, do not flag or request fixes for markdownlint issues in spec/documentation Markdown files under `specs/` (e.g., MD040 fenced-code-language). These markdownlint findings are intentionally treated as acceptable and are not worth churn for files like `specs/**/*.md`.
Learnt from: sfierro
Repo: Kiln-AI/Kiln PR: 1239
File: app/web_ui/src/routes/(app)/specs/[project_id]/[task_id]/[spec_id]/+page.svelte:161-166
Timestamp: 2026-04-07T22:57:02.847Z
Learning: In the Kiln AI codebase (app/web_ui/src/routes/(app)/specs/), the Archive/Unarchive toggle for specs always restores to "active" status on unarchive, regardless of the prior status (e.g., "future" or "deprecated"). This is intentional and consistent with the bulk archive/unarchive behavior on the spec list page.
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.
📊 Coverage ReportOverall Coverage: 27% Diff: origin/main...HEADNo lines with coverage information in this diff.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@libs/core/conftest.py`:
- Around line 45-48: The case branch for MockFileFactoryMimeType.JPEG is
unreachable because JPEG is an alias of MockFileFactoryMimeType.JPG (both map to
"image/jpeg"), so callers requesting JPEG will never hit the JPEG case and will
get image_nasa.jpg; to fix, remove the dead `case MockFileFactoryMimeType.JPEG`
branch or refactor MockFileFactory to switch on the underlying string value (or
use a dict keyed by the mime string) so you can map "image/jpeg" to distinct
filenames (e.g., choose between image_nasa.jpg and image_nasa.jpeg) and update
the filename assignment logic around the filename variable/test_data_dir
accordingly, ensuring MockFileFactoryMimeType and the factory switch/dict are
consistent.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a54480ab-0419-4972-b658-401e8ae320e2
📒 Files selected for processing (4)
.config/hooks_mcp.yamlconftest.pylibs/core/conftest.pylibs/server/conftest.py
💤 Files with no reviewable changes (1)
- conftest.py
✅ Files skipped from review due to trivial changes (2)
- libs/server/conftest.py
- .config/hooks_mcp.yaml
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
… anything, but complexity.
chiang-daniel
approved these changes
Apr 24, 2026
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
2 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
tessl.json,.tessl/, stale Cursor rule, and MCP entrypyproject.toml— merge.coveragercandpytest.ini, delete both filesutils/→.config/utils/(dev-bootstrap scripts).planning/→specs/projects/mcp_sessions/(canonical spec location).cursor/rules/project.mdc— Cursor reads rootAGENTS.mdnatively.cursor/skills/→.agents/skills/(single source of truth, gitignore old paths).agents/{claude,cursor}/setup.shto materialize per-agent copies from canonical sourcehooks_mcp.yaml→.config/hooks_mcp.yaml(rewrite prompt paths, update MCP args)tests/assets/→libs/core/tests/assets/(closer to the 8/9 consumers inlibs/core)guides/→.config/legacy_guides/, delete unused 3.5 MB.avifNet: ~10 fewer entries at repo root, no functionality changes, all tests pass (12867 count preserved).
Manual validation checklist
.coderabbit.yamlconfig into CodeRabbit dashboard UITest plan
uv run ./checks.sh --agent-modepasses on every phaseuv run pytest --collect-only -qshows 12867 tests before and afteruv run coverage debug configconfirms pyproject.toml is the active coverage configdiff -r .agents/skills .cursor/skillsempty)🤖 Generated with Claude Code
Summary by CodeRabbit
Chores
Documentation
Tests