LCORE-2661: Automatic conversation expiration: the stub#2021
Conversation
|
Warning Review limit reached
Next review available in: 56 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughAdds two design documents for automatic conversation expiration (a spike and a feature design template). Also fixes a type coercion ( ChangesAutomatic Conversation Expiration Docs and Minor Fixes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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 |
tisnik
force-pushed
the
lcore-2661-the-stub
branch
3 times, most recently
from
7ad968f to
9876b24
Compare
tisnik
force-pushed
the
lcore-2661-the-stub
branch
from
9876b24 to
0672ae5
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@docs/design/automatic-conversation-expiration/automatic-conversation-expiration-spike.md`:
- Around line 14-17: Update the recommendation text to fix the grammar and
wording: change the runner description so it “identifies” idle conversations and
rephrase the TTL sentence to say it exceeds the defined duration or has been
idle longer than the defined duration. Also clarify which deletion flow should
be reused by explicitly referencing the appropriate “delete conversation” API
implementation and its semantics, since v1 and v2 differ; use the same behavior
as the intended endpoint in the expiration runner description.
In
`@docs/design/automatic-conversation-expiration/automatic-conversation-expiration.md`:
- Around line 14-23: The table of contents nesting is incorrect because the
Architecture subitems are rendered as top-level entries instead of children.
Regenerate the TOC in the markdown document so that Trigger mechanism and
Storage / data model changes are properly nested under Architecture, matching
the heading hierarchy used in the document.
- Line 10: The Links entry points to an incorrect spike doc path, so update the
reference in the automatic-conversation-expiration design document to match the
actual location of the spike file. Verify the intended target and replace the
current `docs/automatic-conversation-expiration` link with the correct path used
by the repository, keeping the link text consistent with the existing “Spike
doc” reference.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 5c636c3c-a87d-4f3b-b53e-a0333bdcc558
📒 Files selected for processing (4)
docs/design/automatic-conversation-expiration/automatic-conversation-expiration-spike.mddocs/design/automatic-conversation-expiration/automatic-conversation-expiration.mdscripts/generate_openapi_schema.pytests/e2e/features/steps/common.py
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-0-6-on-pull-request
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
🧰 Additional context used
📓 Path-based instructions (2)
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/**/*.py: Use pytest for all unit and integration tests; do not use unittest
Usepytest.mark.asynciomarker for async tests
Files:
tests/e2e/features/steps/common.py
tests/e2e/**/*.{py,feature}
📄 CodeRabbit inference engine (AGENTS.md)
Use behave (BDD) framework for end-to-end testing with Gherkin feature files
Files:
tests/e2e/features/steps/common.py
🧠 Learnings (3)
📚 Learning: 2026-04-07T09:20:26.590Z
Learnt from: radofuchs
Repo: lightspeed-core/lightspeed-stack PR: 1467
File: tests/e2e/features/steps/common.py:36-49
Timestamp: 2026-04-07T09:20:26.590Z
Learning: For Behave-based Python tests, rely on Behave’s Context layered stack for attribute lifecycle: Behave pushes a new Context layer when entering feature scope (before_feature) and again for scenario scope (before_scenario). Attributes assigned inside given/when/then steps live on the current scenario layer and are automatically removed when the scenario ends. As a result, step-set attributes should not be expected to persist across scenarios or features, and manual cleanup in after_scenario/after_feature is generally unnecessary for attributes set in step functions. Only perform manual cleanup for attributes that you set explicitly in before_feature/before_scenario, since those live on the respective feature/scenario layers.
Applied to files:
tests/e2e/features/steps/common.py
📚 Learning: 2026-04-13T13:39:54.963Z
Learnt from: radofuchs
Repo: lightspeed-core/lightspeed-stack PR: 1490
File: tests/e2e/features/environment.py:206-211
Timestamp: 2026-04-13T13:39:54.963Z
Learning: In lightspeed-stack E2E tests under tests/e2e/features, it is intentional to set context.feature_config inside Background/step functions (scenario-scoped Behave layer). The environment.py after_scenario restore logic should only restore configuration when context.scenario_lightspeed_override_active is True; this flag is set by configure_service only when a real config switch occurs (so restore does not run for scenarios without a switch). Additionally, steps/common.py’s module-level _active_lightspeed_stack_config_basename is used to prevent re-applying the same config across subsequent scenarios, ensuring scenario_lightspeed_override_active stays False after the first apply. Therefore, reviewers should not “fix” this flow as if feature_config were incorrectly scoped or if after_scenario restoration is missing—config switching and restoration are meant to happen exactly once per actual switch, not redundantly per scenario.
Applied to files:
tests/e2e/features/steps/common.py
📚 Learning: 2026-06-24T13:45:37.249Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 1971
File: src/utils/markdown_repair.py:31-36
Timestamp: 2026-06-24T13:45:37.249Z
Learning: In the lightspeed-stack repository, docstrings must use the section header name "Parameters:" (not "Args:") for function arguments, even if the project references Google Python docstring conventions. Ensure docstrings follow the project’s established "Parameters:" header format for any documented function parameters.
Applied to files:
tests/e2e/features/steps/common.pyscripts/generate_openapi_schema.py
🔇 Additional comments (2)
scripts/generate_openapi_schema.py (1)
40-43: LGTM!tests/e2e/features/steps/common.py (1)
82-82: LGTM!
| **The recommendation**: Implement a new runner that identify idle conversations | ||
| and if their time to live (TTL) is longer than defined duration, the | ||
| conversation will be deleted using the same code that is implemented in "delete | ||
| conversation" REST API endpoint. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Grammar and clarity issues in the recommendation.
Two issues in this sentence:
- Subject-verb agreement: "a new runner that identify" → "identifies".
- Awkward phrasing: "if their time to live (TTL) is longer than defined duration" → "if their TTL exceeds the defined duration" or "if they have been idle longer than the defined duration".
Additionally, there are two different "delete conversation" REST API implementations (v1 and v2) with different deletion semantics. Please clarify which endpoint's code the expiration runner should reuse, or if it needs to handle both local DB and remote Llama Stack deletion like v1, or just cache deletion like v2.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@docs/design/automatic-conversation-expiration/automatic-conversation-expiration-spike.md`
around lines 14 - 17, Update the recommendation text to fix the grammar and
wording: change the runner description so it “identifies” idle conversations and
rephrase the TTL sentence to say it exceeds the defined duration or has been
idle longer than the defined duration. Also clarify which deletion flow should
be reused by explicitly referencing the appropriate “delete conversation” API
implementation and its semantics, since v1 and v2 differ; use the same behavior
as the intended endpoint in the expiration runner description.
| | **Authors** | Pavel Tišnovský | | ||
| | **Feature** | [LCORE-1475](https://redhat.atlassian.net/browse/LCORE-1475) | | ||
| | **Spike** | [LCORE-2661](https://redhat.atlassian.net/browse/LCORE-2661) | | ||
| | **Links** | Spike doc: `docs/automatic-conversation-expiration` | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Incorrect spike document path.
The referenced path docs/automatic-conversation-expiration does not match the actual file location. Based on the repository structure, this should likely be docs/design/automatic-conversation-expiration/automatic-conversation-expiration-spike.md or similar. Please verify and correct the link.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@docs/design/automatic-conversation-expiration/automatic-conversation-expiration.md`
at line 10, The Links entry points to an incorrect spike doc path, so update the
reference in the automatic-conversation-expiration design document to match the
actual location of the spike file. Verify the intended target and replace the
current `docs/automatic-conversation-expiration` link with the correct path used
by the repository, keeping the link text consistent with the existing “Spike
doc” reference.
1 participant
Description
LCORE-2661: Automatic conversation expiration: the stub
Type of change
Tools used to create PR
Related Tickets & Documents
Summary by CodeRabbit
Documentation
Chores