Skip to content

Conversation

@leonardmq
Copy link
Collaborator

@leonardmq leonardmq commented Nov 12, 2025

This reverts commit 9d64b82. Double revert.

What does this PR do?

Bring back logging config.

WIP:

  • breaking on Windows, need to test there

Summary by CodeRabbit

  • New Features

    • Logging levels now configurable via KILN_LOG_LEVEL environment variable (defaults to INFO).
    • Logs organized into separate files in ~/.kiln_ai/logs/ directory.
    • Improved log formatting with enhanced readability.
  • Documentation

    • Added logging configuration details to CONTRIBUTING.md.
  • Tests

    • Added validation tests for log level configuration.

@leonardmq leonardmq marked this pull request as draft November 12, 2025 12:55
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 12, 2025

Walkthrough

This PR introduces comprehensive logging configuration to the Kiln AI desktop application. Changes include a new custom log formatter, environment-based log level validation (via KILN_LOG_LEVEL), separate log files for the desktop app and LiteLLM model calls, and integration across the desktop server, dev server, and core logging utilities. Documentation updates explain the logging defaults and configuration options.

Changes

Cohort / File(s) Summary
Documentation
CONTRIBUTING.md
Added "Logs" subsection describing log level defaults (INFO), KILN_LOG_LEVEL environment variable override, DEBUG example, and log output locations (stdout and ~/.kiln_ai/logs/\*.log).
Logging Configuration Module
app/desktop/log_config.py
Introduced PrettyPrintDictFormatter class for enhanced log output; added LogLevel enum and validate_log_level() function for log level normalization; refactored log_config() to accept log_level and log_file_name parameters; reworked LogDestination enum with CONSOLE, FILE, ALL values; restructured logging configuration with dynamic handler selection.
Desktop & Dev Server Integration
app/desktop/desktop_server.py, app/desktop/dev_server.py
Updated make_app() signature to accept optional litellm_log_filename parameter; wired litellm logging into setup; configured dev and prod servers with environment-based log levels and distinct log file names (kiln_desktop.log, dev_kiln_desktop.log, model_calls.log, dev_model_calls.log).
Test Suite
app/desktop/test_log_config.py
Added TestValidateLogLevel class with parameterized tests for valid log levels (case-insensitive) and ValueError coverage for invalid inputs.
Core Logging Utilities
libs/core/kiln_ai/utils/logging.py
Renamed get_default_formatter() to get_default_log_file_formatter(); updated setup_litellm_logging() signature to accept optional filename parameter with fallback to "model_calls.log"; updated log path handling and formatter calls.

Sequence Diagram

sequenceDiagram
    participant App as App Start
    participant DevServer as Dev Server Init
    participant Desktop as Desktop Server
    participant LogConfig as log_config()
    participant Logging as Core Logging
    
    App->>DevServer: Start dev server
    DevServer->>Desktop: make_app(litellm_log_filename="dev_model_calls.log")
    Desktop->>Logging: setup_litellm_logging("dev_model_calls.log")
    Logging->>Logging: get_log_file_path() + get_default_log_file_formatter()
    
    DevServer->>LogConfig: log_config(log_level=validate_log_level(KILN_LOG_LEVEL), log_file_name="dev_kiln_desktop.log")
    LogConfig->>LogConfig: Create handlers & formatters<br/>(PrettyPrintDictFormatter)
    LogConfig-->>DevServer: Uvicorn log config dict
    
    Note over LogConfig: Logs emitted to:<br/>~/.kiln_ai/logs/dev_kiln_desktop.log<br/>~/.kiln_ai/logs/dev_model_calls.log<br/>stdout (via console handler)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • PrettyPrintDictFormatter implementation: Verify the custom formatter correctly extends uvicorn's DefaultFormatter and handles the extra dict attribute without breaking existing log format.
  • validate_log_level() validation logic: Ensure case-insensitivity, uppercase normalization, and ValueError messages are correct for invalid inputs; verify test coverage.
  • log_config() structural refactor: Review the complete restructuring of logging configuration (formatters, handlers, loggers, root logger) and confirm dynamic handler selection via get_handlers() works correctly.
  • Signature changes across modules: Verify litellm_log_filename threading through make_app → setup_litellm_logging is correct; confirm optional parameter handling.
  • Integration with environment variables: Test that KILN_LOG_LEVEL fallback (default INFO for dev, WARNING for prod) behaves as intended.

Possibly related PRs

Suggested reviewers

  • sfierro
  • aiyakitori
  • scosman

Poem

🐰 Logs flow like carrots now so bright,
With DEBUG hops and INFO light,
A custom formatter, clean and neat,
Environment vars make logging sweet,
No more silent hops in the dark—
Each model call leaves a glowing mark! ✨

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description check ⚠️ Warning The description is largely incomplete. It lacks 'Related Issues', 'Contributor License Agreement' confirmation, and proper test verification. The WIP status and Windows breakage note indicate the PR is unfinished. Complete the description template by adding related issues, CLA confirmation, and clear test results. Resolve Windows breaking issues before marking as ready for review.
Docstring Coverage ⚠️ Warning Docstring coverage is 21.43% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title is partially related to the changeset - it describes the PR as reapplying a logging config, which is accurate, but is somewhat vague about what logging improvements are included.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch leonard/kil-64-bring-back-reverted-logging-pr

📜 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 3d1ed18 and 9d5d00c.

📒 Files selected for processing (6)
  • CONTRIBUTING.md (1 hunks)
  • app/desktop/desktop_server.py (3 hunks)
  • app/desktop/dev_server.py (2 hunks)
  • app/desktop/log_config.py (3 hunks)
  • app/desktop/test_log_config.py (1 hunks)
  • libs/core/kiln_ai/utils/logging.py (4 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
app/desktop/**/*.py

📄 CodeRabbit inference engine (.cursor/rules/project.mdc)

Use Python 3.13 for all desktop app Python code, including studio_server

Python code in app/desktop must target Python 3.13

Files:

  • app/desktop/desktop_server.py
  • app/desktop/log_config.py
  • app/desktop/test_log_config.py
  • app/desktop/dev_server.py
{libs,app/desktop}/**/*.py

📄 CodeRabbit inference engine (.cursor/rules/project.mdc)

{libs,app/desktop}/**/*.py: Use Pydantic v2 (not v1) in Python code that models/validates data
Use explicit type hints for functions, classes, and public APIs in Python code

Files:

  • app/desktop/desktop_server.py
  • app/desktop/log_config.py
  • app/desktop/test_log_config.py
  • app/desktop/dev_server.py
  • libs/core/kiln_ai/utils/logging.py
{libs/core,libs/server,app/desktop}/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

{libs/core,libs/server,app/desktop}/**/*.py: Use Pydantic v2 APIs (e.g., BaseModel v2, model_validate/model_dump) and avoid v1-only patterns
Maintain strong typing in Python code (add type hints and run type checking)

Files:

  • app/desktop/desktop_server.py
  • app/desktop/log_config.py
  • app/desktop/test_log_config.py
  • app/desktop/dev_server.py
  • libs/core/kiln_ai/utils/logging.py
libs/{core,server}/**/*.py

📄 CodeRabbit inference engine (.cursor/rules/project.mdc)

Use Python 3.10+ for library and server code under libs/core and libs/server

Files:

  • libs/core/kiln_ai/utils/logging.py
libs/core/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

Python code in libs/core must be compatible with Python 3.10+

Files:

  • libs/core/kiln_ai/utils/logging.py
🧠 Learnings (3)
📓 Common learnings
Learnt from: leonardmq
Repo: Kiln-AI/Kiln PR: 546
File: app/web_ui/src/routes/(app)/docs/library/[project_id]/upload_file_dialog.svelte:107-120
Timestamp: 2025-09-10T08:32:18.688Z
Learning: leonardmq prefers to work within the constraints of their SDK codegen for API calls, even when typing is awkward (like casting FormData to match expected types), rather than using alternative approaches like native fetch that would cause compiler errors with their generated types.
Learnt from: leonardmq
Repo: Kiln-AI/Kiln PR: 413
File: libs/core/kiln_ai/datamodel/chunk.py:138-160
Timestamp: 2025-08-22T11:17:56.862Z
Learning: leonardmq prefers to avoid redundant validation checks when upstream systems already guarantee preconditions are met. He trusts the attachment system to ensure paths are properly formatted and prefers letting the attachment method (resolve_path) handle any edge cases rather than adding defensive precondition checks.
Learnt from: leonardmq
Repo: Kiln-AI/Kiln PR: 390
File: libs/core/kiln_ai/adapters/provider_tools.py:494-499
Timestamp: 2025-07-23T08:58:45.769Z
Learning: leonardmq prefers to keep tightly coupled implementation details (like API versions) hardcoded when they are not user-facing and could break other modules if changed. The shared Config class is reserved for user-facing customizable values, not internal implementation details.
Learnt from: leonardmq
Repo: Kiln-AI/Kiln PR: 341
File: libs/server/kiln_server/document_api.py:44-51
Timestamp: 2025-06-18T08:22:58.510Z
Learning: leonardmq prefers to defer fixing blocking I/O in async handlers when: the operation is very fast (milliseconds), user-triggered rather than automated, has no concurrency concerns, and would require additional testing to fix properly. He acknowledges such issues as valid but makes pragmatic decisions about timing the fixes.
Learnt from: leonardmq
Repo: Kiln-AI/Kiln PR: 487
File: libs/core/kiln_ai/datamodel/rag.py:33-35
Timestamp: 2025-09-04T06:45:44.212Z
Learning: leonardmq requires vector_store_config_id to be a mandatory field in RagConfig (similar to extractor_config_id, chunker_config_id, embedding_config_id) for consistency. He prefers fixing dependent code that breaks due to missing required fields rather than making fields optional to accommodate incomplete data.
Learnt from: leonardmq
Repo: Kiln-AI/Kiln PR: 654
File: app/web_ui/src/routes/(app)/docs/rag_configs/[project_id]/create_rag_config/edit_rag_config_form.svelte:141-152
Timestamp: 2025-09-25T06:38:14.854Z
Learning: leonardmq prefers simple onMount initialization patterns over reactive statements when possible, and is cautious about maintaining internal state for idempotency in Svelte components. He values simplicity and safety in component lifecycle management.
Learnt from: leonardmq
Repo: Kiln-AI/Kiln PR: 402
File: libs/core/kiln_ai/adapters/embedding/litellm_embedding_adapter.py:0-0
Timestamp: 2025-07-14T03:43:07.283Z
Learning: leonardmq prefers to keep defensive validation checks even when they're technically redundant, viewing them as useful "quick sanity checks" that provide additional safety nets. He values defensive programming over strict DRY (Don't Repeat Yourself) principles when the redundant code serves as a safeguard.
Learnt from: leonardmq
Repo: Kiln-AI/Kiln PR: 546
File: app/web_ui/src/lib/api_schema.d.ts:0-0
Timestamp: 2025-09-10T08:27:47.227Z
Learning: leonardmq correctly identifies auto-generated files and prefers not to manually edit them. When suggesting changes to generated TypeScript API schema files, the fix should be made in the source OpenAPI specification in the backend, not in the generated TypeScript file.
Learnt from: leonardmq
Repo: Kiln-AI/Kiln PR: 487
File: libs/core/kiln_ai/adapters/vector_store/lancedb_adapter.py:136-150
Timestamp: 2025-09-04T06:34:12.318Z
Learning: leonardmq prefers brute force re-insertion of all chunks when partial chunks exist in the LanceDB vector store, rather than selectively inserting only missing chunks. His reasoning is that documents typically have only dozens of chunks (making overwriting cheap), and partial insertion likely indicates data corruption or conflicts that should be resolved by re-inserting all chunks to ensure consistency.
Learnt from: leonardmq
Repo: Kiln-AI/Kiln PR: 487
File: libs/core/kiln_ai/adapters/vector_store/lancedb_adapter.py:98-103
Timestamp: 2025-09-04T07:08:04.248Z
Learning: leonardmq prefers to let TableNotFoundError bubble up in delete_nodes_by_document_id operations rather than catching it, as this error indicates operations are being done in the wrong order on the caller side and should be surfaced as a programming error rather than handled gracefully.
📚 Learning: 2025-08-25T18:52:41.046Z
Learnt from: CR
Repo: Kiln-AI/Kiln PR: 0
File: .cursor/rules/project.mdc:0-0
Timestamp: 2025-08-25T18:52:41.046Z
Learning: Applies to {libs,app/desktop}/**/tests/**/*.py : Write and run Python tests with pytest

Applied to files:

  • app/desktop/test_log_config.py
📚 Learning: 2025-08-29T21:43:55.028Z
Learnt from: CR
Repo: Kiln-AI/Kiln PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-08-29T21:43:55.028Z
Learning: Applies to app/desktop/studio_server/**/*.py : Extend the core server with FastAPI endpoints in app/desktop/studio_server

Applied to files:

  • app/desktop/dev_server.py
🧬 Code graph analysis (5)
app/desktop/desktop_server.py (1)
app/desktop/log_config.py (2)
  • log_config (88-157)
  • validate_log_level (51-57)
app/desktop/log_config.py (1)
libs/core/kiln_ai/utils/logging.py (2)
  • get_default_log_file_formatter (14-15)
  • get_log_file_path (18-28)
app/desktop/test_log_config.py (1)
app/desktop/log_config.py (2)
  • log_config (88-157)
  • validate_log_level (51-57)
app/desktop/dev_server.py (2)
app/desktop/log_config.py (2)
  • log_config (88-157)
  • validate_log_level (51-57)
app/desktop/desktop_server.py (1)
  • make_app (52-71)
libs/core/kiln_ai/utils/logging.py (1)
libs/core/kiln_ai/utils/config.py (2)
  • Config (31-288)
  • settings_dir (232-236)
⏰ 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: Generate Coverage Report
  • GitHub Check: Build Desktop Apps (ubuntu-22.04-arm)
  • GitHub Check: Build Desktop Apps (ubuntu-22.04)
  • GitHub Check: Build Desktop Apps (windows-latest)
  • GitHub Check: Build Desktop Apps (macos-latest)
  • GitHub Check: Build Desktop Apps (macos-15-intel)

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link

📊 Coverage Report

Overall Coverage: 92%

Diff: origin/main...HEAD

  • app/desktop/desktop_server.py (100%)
  • app/desktop/log_config.py (76.7%): Missing lines 25-27,29-32
  • libs/core/kiln_ai/utils/logging.py (100%)

Summary

  • Total: 37 lines
  • Missing: 7 lines
  • Coverage: 81%

Line-by-line

View line-by-line diff coverage

app/desktop/log_config.py

Lines 21-36

  21 
  22         # check if record has any keys
  23         dict_value = getattr(record, "dict", None)
  24         if dict_value:
! 25             try:
! 26                 if isinstance(dict_value, dict):
! 27                     dict_str = json.dumps(dict_value, ensure_ascii=False, indent=2)
  28                 else:
! 29                     dict_str = str(dict_value)
! 30             except Exception as e:  # never fail logging due to serialization
! 31                 dict_str = f"<unserializable extra.dict: {e.__class__.__name__}>"
! 32             formatted += f"\n{dict_str}"
  33 
  34         return formatted
  35 


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants