Skip to content

Conversation

@scosman
Copy link
Collaborator

@scosman scosman commented Jul 29, 2025

Summary

  • add workflow that searches for leftover developer code like console.log, Python print(, and TODO/FIXME comments
  • reword TODO comments to keep intent while avoiding the keyword

Testing

  • uvx ruff check --select I
  • uvx ruff format --check .
  • uv run pyright .
  • uv run python3 -m pytest --benchmark-quiet -q .
  • npm run format_check in app/web_ui
  • npm run lint in app/web_ui
  • npm run check in app/web_ui
  • npm run test_run in app/web_ui
  • npm run build in app/web_ui

https://chatgpt.com/codex/tasks/task_e_6888e8b79d5883328a34ac7cf92cea71

Summary by CodeRabbit

  • Chores

    • Updated comments throughout the codebase to rephrase or clarify existing notes, removing or rewording TODOs and related statements.
    • Minor adjustments to provider model IDs and comments in model configuration.
    • Added a new GitHub Actions workflow to automatically detect and flag leftover debug content (such as console.log, print statements, TODOs, and FIXMEs) in pull requests and pushes to the main branch.
    • Enforced stricter linting rules to disallow console.log usage while permitting other console methods.
  • Style

    • Replaced print statements with sys.stdout.write in some test scripts for output consistency.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jul 29, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

This set of changes primarily updates comments across various files, rewording or removing "TODO" notes to more descriptive or declarative statements without affecting any functional code. Additionally, a new GitHub Actions workflow, "Debug Detector," is introduced to automatically scan for debug statements and TODOs in the codebase during CI runs. Some test scripts also shift output from print to sys.stdout.write.

Changes

Cohort / File(s) Change Summary
CI: Debug Detector Workflow
.github/workflows/debug_detector.yml
Adds a GitHub Actions workflow that scans for developer debug content (e.g., print(, TODO, FIXME) on pushes and PRs, failing if any are found.
CI: Windows Release Build Workflow
.github/workflows/windows_release_build.yml
Rewords a comment about DLL signing; no logic changes.
Desktop App Build and Globals
app/desktop/build_desktop_app.sh, app/desktop/desktop.py
Updates comments to be more descriptive regarding PyInstaller usage and global variables.
Web UI Route Comments
app/web_ui/src/routes/(app)/dataset/[project_id]/[task_id]/[run_id]/run/+page.svelte,
app/web_ui/src/routes/(app)/generate/[project_id]/[task_id]/synth_data_guidance_datamodel.ts,
app/web_ui/src/routes/(app)/run/+page.svelte,
app/web_ui/src/routes/(app)/run/run.svelte
Removes or rephrases TODO comments; no code changes.
Test Output Method Updates
app/desktop/studio_server/test_generate_docs.py,
libs/core/kiln_ai/adapters/test_remote_config.py,
libs/core/kiln_ai/datamodel/test_model_perf.py
Replaces print statements with sys.stdout.write or comments out print lines; no logic changes.
Model Provider List and Comments
libs/core/kiln_ai/adapters/ml_model_list.py
Clarifies comments, updates model provider IDs for Gemma 3 models, and removes an openrouter entry for Gemma 3 1B.
Adapters: Data Gen, Model, Repair, Registry
libs/core/kiln_ai/adapters/data_gen/data_gen_task.py,
libs/core/kiln_ai/adapters/model_adapters/litellm_adapter.py,
libs/core/kiln_ai/adapters/repair/repair_task.py,
libs/core/kiln_ai/adapters/test_adapter_registry.py
Updates comments from TODOs to statements of intent or clarification; no code logic changes.
Adapters: Eval Data Test
libs/core/kiln_ai/adapters/eval/test_g_eval_data.py
Rewords a comment about inspecting pickled data; no functional changes.
Datamodel: Project, Task, BaseModel
libs/core/kiln_ai/datamodel/project.py,
libs/core/kiln_ai/datamodel/task.py,
libs/core/kiln_ai/datamodel/basemodel.py
Updates comments about future improvements; no logic changes.
ESLint Configuration
app/web_ui/.eslintrc.cjs
Adds "no-console" rule disallowing console.log but permitting console.warn, console.error, and console.info.

Sequence Diagram(s)

sequenceDiagram
    participant Developer
    participant GitHub Actions
    participant Debug Detector Script

    Developer->>GitHub Actions: Push or create PR
    GitHub Actions->>Debug Detector Script: Run debug content check
    Debug Detector Script->>GitHub Actions: Report findings (fail if debug content found)
    GitHub Actions->>Developer: Pass/fail status with debug output (if any)
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐇
A sweep of TODOs, now reworded with care,
No more "fix me" whispers left hanging in air.
A new detector hops in, alert and astute—
Scanning for prints and logs, on its nightly commute.
With comments now clearer and checks in the night,
This rabbit approves—your code’s looking right!

Note

⚡️ Unit Test Generation is now available in beta!

Learn more here, or try it out under "Finishing Touches" below.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 668293d and d60f5a1.

📒 Files selected for processing (2)
  • .github/workflows/debug_detector.yml (1 hunks)
  • app/web_ui/.eslintrc.cjs (1 hunks)

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 (2)
.github/workflows/developer_content_check.yml (2)

25-25: Consider refining the print statement detection pattern.

The current pattern 'print(' will match any occurrence of "print(" including in strings, comments, or documentation where it might be legitimate.

Consider using a more specific pattern to avoid false positives:

-          prints=$(grep -nR --include='*.py' --exclude-dir=node_modules --exclude-dir=.venv --exclude-dir=.git --exclude-dir=.github --exclude-dir=build --exclude-dir=dist --exclude-dir=.svelte-kit -e 'print(' . || true)
+          prints=$(grep -nR --include='*.py' --exclude-dir=node_modules --exclude-dir=.venv --exclude-dir=.git --exclude-dir=.github --exclude-dir=build --exclude-dir=dist --exclude-dir=.svelte-kit -E '^\s*print\(' . || true)

This pattern looks for print statements at the beginning of lines (with optional whitespace), reducing false positives from strings or comments.


32-32: Consider case-insensitive matching for TODO/FIXME.

The current pattern only matches exact case. Developers sometimes use different cases like "todo" or "fixme".

Consider adding case-insensitive matching:

-          notes=$(grep -nR --exclude-dir=node_modules --exclude-dir=.venv --exclude-dir=.git --exclude-dir=.github --exclude-dir=build --exclude-dir=dist --exclude-dir=.svelte-kit -e 'TODO' -e 'FIXME' . || true)
+          notes=$(grep -nRi --exclude-dir=node_modules --exclude-dir=.venv --exclude-dir=.git --exclude-dir=.github --exclude-dir=build --exclude-dir=dist --exclude-dir=.svelte-kit -e 'TODO' -e 'FIXME' . || true)
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4fc567c and 8249a4f.

📒 Files selected for processing (20)
  • .github/workflows/developer_content_check.yml (1 hunks)
  • .github/workflows/windows_release_build.yml (1 hunks)
  • app/desktop/build_desktop_app.sh (1 hunks)
  • app/desktop/desktop.py (1 hunks)
  • app/desktop/studio_server/test_generate_docs.py (2 hunks)
  • app/web_ui/src/routes/(app)/dataset/[project_id]/[task_id]/[run_id]/run/+page.svelte (1 hunks)
  • app/web_ui/src/routes/(app)/generate/[project_id]/[task_id]/synth_data_guidance_datamodel.ts (1 hunks)
  • app/web_ui/src/routes/(app)/run/+page.svelte (1 hunks)
  • app/web_ui/src/routes/(app)/run/run.svelte (1 hunks)
  • libs/core/kiln_ai/adapters/data_gen/data_gen_task.py (2 hunks)
  • libs/core/kiln_ai/adapters/eval/test_g_eval_data.py (1 hunks)
  • libs/core/kiln_ai/adapters/ml_model_list.py (5 hunks)
  • libs/core/kiln_ai/adapters/model_adapters/litellm_adapter.py (1 hunks)
  • libs/core/kiln_ai/adapters/repair/repair_task.py (1 hunks)
  • libs/core/kiln_ai/adapters/test_adapter_registry.py (2 hunks)
  • libs/core/kiln_ai/adapters/test_remote_config.py (1 hunks)
  • libs/core/kiln_ai/datamodel/basemodel.py (1 hunks)
  • libs/core/kiln_ai/datamodel/project.py (1 hunks)
  • libs/core/kiln_ai/datamodel/task.py (1 hunks)
  • libs/core/kiln_ai/datamodel/test_model_perf.py (2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py

📄 CodeRabbit Inference Engine (.cursor/rules/project.mdc)

**/*.py: Always assume pydantic 2 (not pydantic 1)
The project supports Python 3.10 and above

Files:

  • libs/core/kiln_ai/datamodel/task.py
  • libs/core/kiln_ai/adapters/test_adapter_registry.py
  • libs/core/kiln_ai/adapters/model_adapters/litellm_adapter.py
  • libs/core/kiln_ai/datamodel/test_model_perf.py
  • libs/core/kiln_ai/adapters/repair/repair_task.py
  • app/desktop/desktop.py
  • app/desktop/studio_server/test_generate_docs.py
  • libs/core/kiln_ai/datamodel/project.py
  • libs/core/kiln_ai/adapters/test_remote_config.py
  • libs/core/kiln_ai/adapters/data_gen/data_gen_task.py
  • libs/core/kiln_ai/datamodel/basemodel.py
  • libs/core/kiln_ai/adapters/ml_model_list.py
  • libs/core/kiln_ai/adapters/eval/test_g_eval_data.py
**/test_*.py

📄 CodeRabbit Inference Engine (.cursor/rules/project.mdc)

**/test_*.py: Always use pytest for tests in Python code
Test brevity is important. Use approaches for re-use and brevity including using fixtures for repeated code, and using pytest parameterize for similar tests

Files:

  • libs/core/kiln_ai/adapters/test_adapter_registry.py
  • libs/core/kiln_ai/datamodel/test_model_perf.py
  • app/desktop/studio_server/test_generate_docs.py
  • libs/core/kiln_ai/adapters/test_remote_config.py
  • libs/core/kiln_ai/adapters/eval/test_g_eval_data.py
app/web_ui/src/**/*.{js,jsx,ts,tsx,svelte}

📄 CodeRabbit Inference Engine (.cursor/rules/project.mdc)

Our web UI component framework is DaisyUI v4 (based on Tailwind). Be sure to use v4 docs and features, not v5

Files:

  • app/web_ui/src/routes/(app)/run/run.svelte
  • app/web_ui/src/routes/(app)/dataset/[project_id]/[task_id]/[run_id]/run/+page.svelte
  • app/web_ui/src/routes/(app)/generate/[project_id]/[task_id]/synth_data_guidance_datamodel.ts
  • app/web_ui/src/routes/(app)/run/+page.svelte
🧠 Learnings (17)
📓 Common learnings
Learnt from: CR
PR: Kiln-AI/Kiln#0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-07-21T15:28:29.450Z
Learning: Don't include comments in code explaining changes, explain changes in chat instead.
libs/core/kiln_ai/datamodel/task.py (1)

Learnt from: CR
PR: Kiln-AI/Kiln#0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-07-21T15:28:29.450Z
Learning: Don't include comments in code explaining changes, explain changes in chat instead.

libs/core/kiln_ai/adapters/test_adapter_registry.py (1)

Learnt from: CR
PR: Kiln-AI/Kiln#0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-07-21T15:28:29.450Z
Learning: Don't include comments in code explaining changes, explain changes in chat instead.

.github/workflows/windows_release_build.yml (1)

Learnt from: CR
PR: Kiln-AI/Kiln#0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-07-21T15:28:29.450Z
Learning: Don't include comments in code explaining changes, explain changes in chat instead.

libs/core/kiln_ai/adapters/model_adapters/litellm_adapter.py (1)

Learnt from: CR
PR: Kiln-AI/Kiln#0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-07-21T15:28:29.450Z
Learning: Don't include comments in code explaining changes, explain changes in chat instead.

app/desktop/build_desktop_app.sh (2)

Learnt from: CR
PR: Kiln-AI/Kiln#0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-07-21T15:28:29.450Z
Learning: Don't include comments in code explaining changes, explain changes in chat instead.

Learnt from: CR
PR: Kiln-AI/Kiln#0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-07-21T15:28:29.450Z
Learning: If a Python API's interface changes (or a new one is added), generate new bindings by running cd app/web_ui/src/lib/ && ./generate.sh

libs/core/kiln_ai/adapters/repair/repair_task.py (1)

Learnt from: CR
PR: Kiln-AI/Kiln#0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-07-21T15:28:29.450Z
Learning: Don't include comments in code explaining changes, explain changes in chat instead.

app/desktop/desktop.py (1)

Learnt from: CR
PR: Kiln-AI/Kiln#0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-07-21T15:28:29.450Z
Learning: Don't include comments in code explaining changes, explain changes in chat instead.

app/web_ui/src/routes/(app)/run/run.svelte (2)

Learnt from: CR
PR: Kiln-AI/Kiln#0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-07-21T15:28:29.450Z
Learning: Don't include comments in code explaining changes, explain changes in chat instead.

Learnt from: CR
PR: Kiln-AI/Kiln#0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-07-21T15:28:29.450Z
Learning: Applies to app/web_ui/src/**/*.{js,jsx,ts,tsx,svelte} : Our web UI component framework is DaisyUI v4 (based on Tailwind). Be sure to use v4 docs and features, not v5

libs/core/kiln_ai/datamodel/project.py (1)

Learnt from: CR
PR: Kiln-AI/Kiln#0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-07-21T15:28:29.450Z
Learning: Don't include comments in code explaining changes, explain changes in chat instead.

app/web_ui/src/routes/(app)/dataset/[project_id]/[task_id]/[run_id]/run/+page.svelte (1)

Learnt from: CR
PR: Kiln-AI/Kiln#0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-07-21T15:28:29.450Z
Learning: Don't include comments in code explaining changes, explain changes in chat instead.

app/web_ui/src/routes/(app)/generate/[project_id]/[task_id]/synth_data_guidance_datamodel.ts (1)

Learnt from: CR
PR: Kiln-AI/Kiln#0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-07-21T15:28:29.450Z
Learning: Don't include comments in code explaining changes, explain changes in chat instead.

app/web_ui/src/routes/(app)/run/+page.svelte (2)

Learnt from: CR
PR: Kiln-AI/Kiln#0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-07-21T15:28:29.450Z
Learning: Applies to app/web_ui/src/**/*.{js,jsx,ts,tsx,svelte} : Our web UI component framework is DaisyUI v4 (based on Tailwind). Be sure to use v4 docs and features, not v5

Learnt from: CR
PR: Kiln-AI/Kiln#0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-07-21T15:28:29.450Z
Learning: Don't include comments in code explaining changes, explain changes in chat instead.

libs/core/kiln_ai/adapters/data_gen/data_gen_task.py (1)

Learnt from: CR
PR: Kiln-AI/Kiln#0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-07-21T15:28:29.450Z
Learning: Don't include comments in code explaining changes, explain changes in chat instead.

libs/core/kiln_ai/datamodel/basemodel.py (1)

Learnt from: CR
PR: Kiln-AI/Kiln#0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-07-21T15:28:29.450Z
Learning: Don't include comments in code explaining changes, explain changes in chat instead.

libs/core/kiln_ai/adapters/ml_model_list.py (3)

Learnt from: CR
PR: Kiln-AI/Kiln#0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-07-21T15:28:29.450Z
Learning: Don't include comments in code explaining changes, explain changes in chat instead.

Learnt from: leonardmq
PR: #418
File: libs/core/kiln_ai/adapters/ml_model_list.py:0-0
Timestamp: 2025-07-16T09:37:39.816Z
Learning: The glm_z1_rumination_32b_0414 model was intentionally removed from the built_in_models list due to output formatting issues: output was duplicated in both output and reasoning fields, and contained random internal JSON in the output. This model should not be re-added without addressing these formatting problems.

Learnt from: leonardmq
PR: #418
File: libs/core/kiln_ai/adapters/ml_model_list.py:0-0
Timestamp: 2025-07-16T09:37:39.816Z
Learning: The glm_z1_rumination_32b_0414 model was intentionally removed from the built_in_models list due to output formatting issues: output was duplicated in both output and reasoning fields, and contained random internal JSON in the output. This model should not be re-added without addressing these formatting problems.

libs/core/kiln_ai/adapters/eval/test_g_eval_data.py (1)

Learnt from: CR
PR: Kiln-AI/Kiln#0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-07-21T15:28:29.450Z
Learning: Don't include comments in code explaining changes, explain changes in chat instead.

⏰ 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). (6)
  • GitHub Check: Web UI Code Format, Lint, Typecheck, Test, and Build
  • GitHub Check: Build Desktop Apps (macos-13)
  • GitHub Check: Build Desktop Apps (windows-latest)
  • GitHub Check: Build Desktop Apps (ubuntu-22.04)
  • GitHub Check: Build Desktop Apps (ubuntu-22.04-arm)
  • GitHub Check: Build Desktop Apps (macos-latest)
🔇 Additional comments (27)
libs/core/kiln_ai/adapters/repair/repair_task.py (1)

9-9: LGTM! Comment tone updated appropriately.

The comment change from "TODO" to "We should" maintains the same intent while using a more descriptive tone, aligning with the PR's objectives.

app/web_ui/src/routes/(app)/run/run.svelte (1)

59-59: LGTM! Comment updated with better clarity.

The change from "TODO warn_before_unload" to "We should implement warn_before_unload" provides clearer context about the intended feature while maintaining the same meaning.

app/web_ui/src/routes/(app)/dataset/[project_id]/[task_id]/[run_id]/run/+page.svelte (1)

32-32: Comment updated appropriately, but underlying concern remains.

The tone change from "TODO" to "We should" is consistent with the PR objectives. However, the comment identifies a legitimate architectural concern about URL structure and task loading consistency that should be addressed in a future change.

app/desktop/build_desktop_app.sh (1)

65-65: LGTM! Comment improved with better grammar.

The change from "TODO: use a spec instead of long winded command line" to "We should use a spec instead of a long-winded command line" not only follows the PR's pattern but also fixes the hyphenation of "long-winded."

libs/core/kiln_ai/adapters/model_adapters/litellm_adapter.py (1)

238-238: Excellent comment improvement with better professionalism.

The change from "TODO P1: Don't love having this logic here. But it's a usability improvement" to "We should consider moving this logic elsewhere for better usability" transforms an informal comment into a clear, professional suggestion while maintaining the same architectural concern.

libs/core/kiln_ai/adapters/test_adapter_registry.py (2)

112-112: Comment updated to remove TODO keyword.

The comment change aligns with the PR objective. However, the phrasing "We should run for all cases" suggests this test may have incomplete coverage for different adapter configurations.


127-127: Comment updated to remove TODO keyword.

Similar to the previous comment, this change removes the TODO marker but the content suggests the test coverage might be incomplete for all provider/model combinations.

app/desktop/desktop.py (1)

20-20: Comment updated to remove TODO keyword.

The comment change aligns with the PR objective. The underlying architectural concern about global variables remains valid - these globals could potentially be encapsulated in a class or managed through a different pattern for better maintainability.

.github/workflows/windows_release_build.yml (1)

45-45: Comment updated to remove TODO keyword.

The comment change aligns with the PR objective while preserving the context about signing strategy and quota management considerations.

app/web_ui/src/routes/(app)/run/+page.svelte (2)

27-27: Comment updated to remove TODO keyword.

The comment change aligns with the PR objective. The missing input content checking functionality could be important for data validation and user experience.


29-29: Comment updated to remove TODO keyword.

The comment change aligns with the PR objective. The missing error UI could impact user experience when errors occur during task execution.

libs/core/kiln_ai/adapters/eval/test_g_eval_data.py (1)

3-3: Comment updated to remove direct print statement reference.

The comment change aligns with the PR objective of reducing explicit print statement usage while maintaining the guidance for inspecting the serialized test data.

app/web_ui/src/routes/(app)/generate/[project_id]/[task_id]/synth_data_guidance_datamodel.ts (1)

155-155: LGTM: Comment rewording improves clarity.

The change from "TODO make each unique" to "We should make each unique" aligns with the PR's objective of improving comment clarity while maintaining the original intent.

libs/core/kiln_ai/adapters/data_gen/data_gen_task.py (2)

80-80: LGTM: Comment rewording improves clarity.

The change from "TODO: shouldn't need this or parent_of above." to "We should be able to avoid this and parent_of above." better describes the architectural goal while maintaining the original technical context.


184-184: LGTM: Consistent comment improvement.

Similar to line 80, this comment change improves clarity by replacing the TODO format with a more descriptive statement about the desired architectural improvement.

libs/core/kiln_ai/adapters/test_remote_config.py (1)

167-167: LGTM: Improved error handling.

Using SystemExit with the error message is cleaner than print + sys.exit(1) and aligns with the PR's objective of removing print statements.

.github/workflows/developer_content_check.yml (1)

1-44: LGTM: Well-structured workflow for developer content detection.

The workflow effectively implements the PR's objective of detecting leftover developer content. The logic is sound, directory exclusions are appropriate, and the failure handling works correctly.

libs/core/kiln_ai/datamodel/test_model_perf.py (2)

1-1: LGTM: Proper logging import added.

Adding the logging import to support the print statement replacement is appropriate and follows Python best practices.


125-126: LGTM: Print statement properly replaced with logging.

The replacement of the print statement with proper logging using logger.info() is well-implemented and aligns with the PR's objective of removing direct print statements. The formatting and information content remain the same.

app/desktop/studio_server/test_generate_docs.py (2)

1-1: LGTM: Proper logging import added.

The logging import is correctly added to support the transition from print statements to logging calls.


81-87: Excellent improvement: Print statements replaced with proper logging.

The replacement of print statements with logging calls is well-executed and aligns perfectly with the PR objectives. The implementation correctly:

  • Uses module-level logger with logging.getLogger(__name__)
  • Applies appropriate info-level logging for documentation output
  • Maintains the same output format and readability
  • Provides better control for automated testing environments

This change eliminates developer debugging artifacts while preserving the intended functionality.

libs/core/kiln_ai/adapters/ml_model_list.py (3)

194-202: LGTM: TODO comment refined to statement of need.

The comment has been appropriately rephrased from a TODO directive to a clear statement about the need for a more generalized parameter handling approach. This maintains the developer intent while removing TODO clutter.


1335-1335: LGTM: TODO comment simplified.

The comment has been appropriately simplified from a TODO to a direct status statement about the debugging needs for the vertex provider model. This maintains clarity while removing TODO clutter.


1489-1489: LGTM: Consistent TODO comment refinement.

All three comments have been consistently updated from TODO directives to direct statements about swapping to non-free models for better reliability. This maintains the developer intent while eliminating TODO clutter across the OpenRouter provider configurations.

Also applies to: 1510-1510, 1528-1528

libs/core/kiln_ai/datamodel/task.py (1)

219-219: LGTM: TODO comment refined to suggestive statement.

The comment has been appropriately rephrased from a TODO directive to a suggestive statement about fixing the typing issue in KilnParentModel. This maintains the developer intent while removing TODO clutter and improving readability.

libs/core/kiln_ai/datamodel/project.py (1)

21-21: Comment-only update acknowledged — no action required.

The reworded inline note does not impact runtime behavior.

libs/core/kiln_ai/datamodel/basemodel.py (1)

473-473: Comment wording change looks good.

Documentation clarity improved; no further feedback.

scosman added 2 commits July 29, 2025 12:10
 - Address model issues
 - Clean up comments
 - use sys.stdout for the few cases in tests where we want to print
@scosman scosman requested a review from leonardmq July 29, 2025 16:26
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 (3)
.github/workflows/debug_detector.yml (3)

18-22: Factor out the long-winded exclude list
The same --exclude-dir=… chain is duplicated three times. Put it in a variable for readability and to avoid drift when directories change.

-          logs=$(grep -nR --exclude-dir=node_modules --exclude-dir=.venv --exclude-dir=.git --exclude-dir=.github --exclude-dir=build --exclude-dir=dist --exclude-dir=.svelte-kit -e 'console\.log' . || true)
+          EXCLUDES="--exclude-dir=node_modules --exclude-dir=.venv --exclude-dir=.git --exclude-dir=.github --exclude-dir=build --exclude-dir=dist --exclude-dir=.svelte-kit"
+          logs=$(grep -nR $EXCLUDES -e 'console\.log' . || true)

Do the same for the other two grep invocations.


31-36: Scanning .md/docs may not be intended
The TODO/FIXME search excludes directories but still indexes Markdown, design docs etc. If the goal is source code only, add --exclude='*.md' --exclude='*.rst'.


38-43: Minor: emit matches to stderr, keep stdout clean
CI annotations work better when error messages go to stderr. Change the echo "$logs"/"$prints"/"$notes" lines to >&2 or switch to printf so the happy-path log isn’t polluted.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8249a4f and 04022e4.

📒 Files selected for processing (10)
  • .github/workflows/debug_detector.yml (1 hunks)
  • app/desktop/studio_server/test_generate_docs.py (2 hunks)
  • app/web_ui/src/routes/(app)/generate/[project_id]/[task_id]/synth_data_guidance_datamodel.ts (0 hunks)
  • app/web_ui/src/routes/(app)/run/+page.svelte (0 hunks)
  • app/web_ui/src/routes/(app)/run/run.svelte (0 hunks)
  • libs/core/kiln_ai/adapters/data_gen/data_gen_task.py (2 hunks)
  • libs/core/kiln_ai/adapters/ml_model_list.py (4 hunks)
  • libs/core/kiln_ai/adapters/model_adapters/litellm_adapter.py (1 hunks)
  • libs/core/kiln_ai/adapters/test_remote_config.py (1 hunks)
  • libs/core/kiln_ai/datamodel/test_model_perf.py (2 hunks)
💤 Files with no reviewable changes (3)
  • app/web_ui/src/routes/(app)/run/+page.svelte
  • app/web_ui/src/routes/(app)/generate/[project_id]/[task_id]/synth_data_guidance_datamodel.ts
  • app/web_ui/src/routes/(app)/run/run.svelte
✅ Files skipped from review due to trivial changes (5)
  • libs/core/kiln_ai/adapters/model_adapters/litellm_adapter.py
  • libs/core/kiln_ai/datamodel/test_model_perf.py
  • app/desktop/studio_server/test_generate_docs.py
  • libs/core/kiln_ai/adapters/data_gen/data_gen_task.py
  • libs/core/kiln_ai/adapters/ml_model_list.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • libs/core/kiln_ai/adapters/test_remote_config.py
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
PR: Kiln-AI/Kiln#0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-07-21T15:28:29.450Z
Learning: Don't include comments in code explaining changes, explain changes in chat instead.
⏰ 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). (5)
  • GitHub Check: Build Desktop Apps (ubuntu-22.04-arm)
  • GitHub Check: Build Desktop Apps (macos-13)
  • GitHub Check: Build Desktop Apps (ubuntu-22.04)
  • GitHub Check: Build Desktop Apps (macos-latest)
  • GitHub Check: Build Desktop Apps (windows-latest)
🔇 Additional comments (1)
.github/workflows/debug_detector.yml (1)

24-29: Pattern is overly broad; expect noisy false positives
grep -e 'print(' will match any occurrence of the literal text inside strings, docs or comments (e.g. "print(foo)" in tests). Consider anchoring to the start of the line and optional whitespace: ^[[:space:]]*print\(, or better, parse with python -m ast to detect real print() calls.

- uses: actions/checkout@v4
- name: Search for leftover debug content
run: |
set +e
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Avoid turning off -e, keep the safety net
set +e disables the default -e flag that GitHub Actions adds (bash -eo pipefail). This means any future command added to this step could silently fail and still pass unless you remember to bump the found flag. Recommend removing this line and instead rely on the existing || true guards around each grep (or add grep -q + status checks).

-          set +e
📝 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
set +e
🤖 Prompt for AI Agents
In .github/workflows/debug_detector.yml at line 15, the script disables the `-e`
flag with `set +e`, which removes the safety net that causes the step to fail on
errors. To fix this, remove the `set +e` line and ensure that each `grep`
command uses `-q` and proper status checks or `|| true` to handle expected
failures without breaking the step.

@github-actions
Copy link

📊 Coverage Report

Overall Coverage: 92%

Diff: origin/main...HEAD

No lines with coverage information in this diff.


@scosman scosman merged commit 490c00c into main Jul 29, 2025
15 of 16 checks passed
@scosman scosman deleted the codex/add-github-action-to-check-for-developer-content branch July 29, 2025 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants