Skip to content

Conversation

@tisnik
Copy link
Contributor

@tisnik tisnik commented Sep 19, 2025

Description

LCORE-298: improved logging of code that deals with conversation history

Type of change

  • Refactor
  • New feature
  • Bug fix
  • CVE fix
  • Optimization
  • Documentation Update
  • Configuration Update
  • Bump-up service version
  • Bump-up dependent library
  • Bump-up library or tool used for development (does not change the final image)
  • CI configuration change
  • Konflux configuration change
  • Unit tests improvement
  • Integration tests improvement
  • End to end tests improvement

Related Tickets & Documents

  • Related Issue #LCORE-298

Summary by CodeRabbit

  • Chores
    • Enhanced telemetry and debug logging for database initialization (SQLite and PostgreSQL), including configuration visibility.
    • Added debug logs in query handling to indicate presence or absence of conversation IDs.
    • Expanded logging in agent retrieval to trace conversation reuse versus new session creation; standardized logger usage across utilities.
    • No functional or API changes; end-user behavior and performance remain unchanged.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 19, 2025

Walkthrough

Added telemetry/debug logging across database initialization, query endpoint handling, and agent retrieval utilities. Updated utils to use a custom logger factory. No changes to control flow, error handling, data models, or public APIs.

Changes

Cohort / File(s) Summary
Database init telemetry
src/app/database.py
Inserted info/debug logs in initialize_database() for SQLite and PostgreSQL, logging start and configuration objects. No functional changes.
Query endpoint logging
src/app/endpoints/query.py
Added debug logs indicating presence/absence of conversation_id before existing ownership validation. No changes to logic or responses.
Logger integration + agent path tracing
src/utils/endpoints.py
Switched from standard logging to custom get_logger(name). Added debug logs in get_agent for reuse vs. new conversation paths, logging IDs. No behavioral changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I thump my paw—logs light the night,
Tiny breadcrumbs, bytes in flight.
SQLite whispers, Postgres replies,
Queries blink their little eyes.
Agents hop on tracked terrain—
With softer prints, we still remain.
Debug carrots for the clever brain. 🥕🐇

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "LCORE-298: improved logging of code that deals with conversation history" accurately reflects the PR objectives and the raw summary: the changes add logging in src/app/database.py, src/app/endpoints/query.py, and src/utils/endpoints.py to surface conversation-related behavior and configuration details. It is concise, focused on the primary change (improved logging) and includes the issue key for traceability. A teammate scanning history will understand the main intent of the change.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 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 db21c10 and 2f95dd7.

📒 Files selected for processing (3)
  • src/app/database.py (1 hunks)
  • src/app/endpoints/query.py (2 hunks)
  • src/utils/endpoints.py (3 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
src/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Use absolute imports for internal modules (e.g., from auth import get_auth_dependency)

Files:

  • src/app/endpoints/query.py
  • src/app/database.py
  • src/utils/endpoints.py
src/app/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Use standard FastAPI imports (from fastapi import APIRouter, HTTPException, Request, status, Depends) in FastAPI app code

Files:

  • src/app/endpoints/query.py
  • src/app/database.py
**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.py: All modules start with descriptive module-level docstrings explaining purpose
Use logger = logging.getLogger(name) for module logging after import logging
Define type aliases at module level for clarity
All functions require docstrings with brief descriptions
Provide complete type annotations for all function parameters and return types
Use typing_extensions.Self in model validators where appropriate
Use modern union syntax (str | int) and Optional[T] or T | None consistently
Function names use snake_case with descriptive, action-oriented prefixes (get_, validate_, check_)
Avoid in-place parameter modification; return new data structures instead of mutating arguments
Use appropriate logging levels: debug, info, warning, error with clear messages
All classes require descriptive docstrings explaining purpose
Class names use PascalCase with conventional suffixes (Configuration, Error/Exception, Resolver, Interface)
Abstract base classes should use abc.ABC and @AbstractMethod for interfaces
Provide complete type annotations for all class attributes
Follow Google Python docstring style for modules, classes, and functions, including Args, Returns, Raises, Attributes sections as needed

Files:

  • src/app/endpoints/query.py
  • src/app/database.py
  • src/utils/endpoints.py
src/{app/**/*.py,client.py}

📄 CodeRabbit inference engine (CLAUDE.md)

Use async def for I/O-bound operations and external API calls

Files:

  • src/app/endpoints/query.py
  • src/app/database.py
src/app/endpoints/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

In API endpoints, raise FastAPI HTTPException with appropriate status codes for error handling

Files:

  • src/app/endpoints/query.py
🧠 Learnings (1)
📚 Learning: 2025-08-06T06:02:21.060Z
Learnt from: eranco74
PR: lightspeed-core/lightspeed-stack#348
File: src/utils/endpoints.py:91-94
Timestamp: 2025-08-06T06:02:21.060Z
Learning: The direct assignment to `agent._agent_id` in `src/utils/endpoints.py` is a necessary workaround for the missing agent rehydration feature in the LLS client SDK. This allows preserving conversation IDs when handling existing agents.

Applied to files:

  • src/utils/endpoints.py
🧬 Code graph analysis (2)
src/app/database.py (1)
src/models/config.py (2)
  • config (132-138)
  • SQLiteDatabaseConfiguration (73-76)
src/utils/endpoints.py (1)
src/log.py (1)
  • get_logger (7-13)
⏰ 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). (2)
  • GitHub Check: build-pr
  • GitHub Check: e2e_tests
🔇 Additional comments (4)
src/app/database.py (1)

117-127: LGTM! Proper telemetry logging implementation.

The logging statements follow best practices by providing clear, descriptive messages at appropriate levels (info for initialization events, debug for configuration details). The configuration object logging on debug level is particularly helpful for troubleshooting database connection issues.

src/utils/endpoints.py (2)

168-190: LGTM! Enhanced debugging visibility for conversation handling.

The debug logging statements provide excellent visibility into the conversation flow:

  • Lines 168-169 log existing conversation and agent IDs when reusing conversations
  • Lines 188-190 log new conversation and session IDs when creating fresh conversations

This follows good logging practices by using appropriate debug level and including contextual information that will be valuable for troubleshooting conversation handling issues.


18-20: Align logging usage: prefer logging.getLogger(name) or standardize the custom get_logger

Repository shows mixed usage — custom get_logger used in this PR and files like src/app/main.py and src/app/database.py, while many modules (src/authentication/*, src/runners/uvicorn.py, etc.) use logging.getLogger(name). Either convert this PR to use logging.getLogger(name) to follow the coding guideline, or update the project convention/docs and migrate other modules to the custom get_logger.

src/app/endpoints/query.py (1)

187-212: LGTM! Clear debug logging for conversation ID presence.

The debug logging statements provide excellent visibility into whether a conversation ID was provided in the query request:

  • Lines 187-189 log when a conversation ID is specified, including the actual ID value
  • Lines 211-212 log when no conversation ID is provided

This follows good debugging practices and will be helpful for troubleshooting conversation-related issues. The logging is appropriately placed before the ownership validation logic and uses the debug level correctly.

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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.

@tisnik tisnik merged commit 77f9bcb into lightspeed-core:main Sep 19, 2025
18 of 19 checks passed
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.

1 participant