Skip to content

📝 CodeRabbit Chat: Implement requested code changes#119

Open
coderabbitai[bot] wants to merge 1 commit into
issue-25-split-routine-heartbeat-rs-into-multiple-modulesfrom
coderabbitai/chat/fe542de
Open

📝 CodeRabbit Chat: Implement requested code changes#119
coderabbitai[bot] wants to merge 1 commit into
issue-25-split-routine-heartbeat-rs-into-multiple-modulesfrom
coderabbitai/chat/fe542de

Conversation

@coderabbitai
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot commented Apr 7, 2026

Code changes was requested by @leynos.

The following files were modified:

  • docs/testing-strategy.md
  • tests/support/mod.rs
  • tests/support/routines.rs

Summary by Sourcery

Document and validate the shared test helpers for routine and heartbeat end-to-end tests while tightening compiler lints around their usage.

Enhancements:

  • Replace generic dead-code and lint suppressions on routine test helpers with targeted #[expect] attributes that include justifying reasons.
  • Add unit and async integration-style tests to verify the behavior and signatures of routine-related test support functions.
  • Extend the routines test-support symbol reference function to assert the full make_minimal_engine signature while documenting the intentional type complexity.

Documentation:

  • Expand the testing strategy documentation with a dedicated section describing the routine and heartbeat E2E helper module, its data structures, and the recommended pattern for writing new routine tests.

@coderabbitai coderabbitai Bot requested a review from leynos April 7, 2026 10:28
@sourcery-ai
Copy link
Copy Markdown
Contributor

sourcery-ai Bot commented Apr 7, 2026

Reviewer's Guide

Refines the test-support routines API by replacing broad dead-code allowances with reasoned #[expect] attributes, adds unit/integration tests for the helpers to prove usage, and documents the routine/heartbeat E2E helpers and their recommended usage pattern in the testing strategy docs while keeping compile-time signature checks up-to-date.

Sequence diagram for a routine E2E test using the helpers

sequenceDiagram
  actor TestAuthor
  participant TestCode
  participant Database
  participant Workspace
  participant RoutineEngine
  participant TraceLlm
  participant NotificationReceiver

  TestAuthor->>TestCode: create_test_db()
  TestCode->>Database: apply_migrations()
  Database-->>TestCode: db_handle, tempdir_guard

  TestAuthor->>TestCode: create_workspace(db_handle)
  TestCode->>Workspace: new(default, db_handle)
  Workspace-->>TestCode: ws

  TestAuthor->>TestCode: build LlmTrace

  TestAuthor->>TestCode: make_minimal_engine(trace, db_handle, ws)
  TestCode->>TraceLlm: new(trace)
  TraceLlm-->>RoutineEngine: TraceLlm instance
  TestCode->>RoutineEngine: new(db_handle, ws, TraceLlm, ToolRegistry, SafetyLayer)
  RoutineEngine-->>NotificationReceiver: create_mpsc_channel()
  RoutineEngine-->>TestCode: engine, receiver

  TestAuthor->>TestCode: make_routine(name, trigger, prompt)
  TestCode->>Database: create_routine(routine)
  Database-->>TestCode: ok

  TestCode->>RoutineEngine: refresh_event_cache()

  TestAuthor->>RoutineEngine: emit_system_event(spec)
  RoutineEngine->>Database: load_matching_routines()
  Database-->>RoutineEngine: routines
  RoutineEngine->>RoutineEngine: spawn_routine_tasks()

  loop until expected_runs observed
    TestCode->>Database: list_routine_runs(routine_id, 10)
    Database-->>TestCode: runs
  end

  TestCode->>TestCode: assert run_count == expected
Loading

Class diagram for routines E2E test-support helpers

classDiagram
class RoutinesSupport {
  +create_test_db() Result_ArcDatabase_TempDir
  +create_workspace(db Arc_Database) Arc_Workspace
  +make_routine(name String, trigger Trigger, prompt Prompt) Routine
  +make_test_incoming_message(content String) IncomingMessage
  +make_minimal_engine(trace LlmTrace, db Arc_Database, ws Arc_Workspace) EngineAndReceiver
  +register_github_issue_routine(db Arc_Database, engine RoutineEngine) Result_void
  +assert_system_event_count(engine RoutineEngine, spec SystemEventSpec, expected usize, msg str_ref) Result_void
}

class SystemEventSpec {
  +source String
  +event_type String
  +payload serde_json_Value
  +new(source String, event_type String, payload serde_json_Value) SystemEventSpec
}

class Database {
  +create_routine(routine Routine) Result_void
  +list_routine_runs(routine_id RoutineId, limit usize) Vec_RoutineRun
}

class Workspace {
  +id WorkspaceId
  +name String
}

class RoutineEngine {
  +refresh_event_cache() Result_void
  +emit_system_event(spec SystemEventSpec) Result_void
}

class TraceLlm {
  +trace LlmTrace
}

class ToolRegistry
class SafetyLayer
class IncomingMessage
class Routine
class LlmTrace
class NotificationReceiver
class TempDir
class Arc_Database
class Arc_Workspace
class EngineAndReceiver {
  +engine RoutineEngine
  +receiver NotificationReceiver
}
class Result_ArcDatabase_TempDir
class Result_void
class Vec_RoutineRun
class RoutineRun
class RoutineId
class WorkspaceId
class serde_json_Value
class Trigger
class Prompt
class str_ref

RoutinesSupport ..> SystemEventSpec
RoutinesSupport ..> Database
RoutinesSupport ..> Workspace
RoutinesSupport ..> RoutineEngine
RoutinesSupport ..> TraceLlm
RoutinesSupport ..> ToolRegistry
RoutinesSupport ..> SafetyLayer
RoutinesSupport ..> IncomingMessage
RoutinesSupport ..> Routine
RoutinesSupport ..> LlmTrace
RoutinesSupport ..> NotificationReceiver
RoutinesSupport ..> TempDir
RoutinesSupport ..> Arc_Database
RoutinesSupport ..> Arc_Workspace
RoutinesSupport ..> EngineAndReceiver
SystemEventSpec ..> serde_json_Value
RoutineEngine ..> Database
RoutineEngine ..> TraceLlm
RoutineEngine ..> ToolRegistry
RoutineEngine ..> SafetyLayer
EngineAndReceiver ..> RoutineEngine
EngineAndReceiver ..> NotificationReceiver
Database ..> Routine
Database ..> RoutineRun
RoutineRun ..> RoutineId
Workspace ..> WorkspaceId
Loading

File-Level Changes

Change Details Files
Replace broad #[allow(dead_code)] and Clippy allowances on routines helpers with targeted #[expect(..., reason = ...)] annotations to satisfy lint expectations while documenting cross-module test usage.
  • Convert dead-code suppressions on SystemEventSpec, its impl, and all routine/engine helper functions to #[expect(dead_code, reason = ...)] with detailed justifications referencing feature-gated E2E usage.
  • Update the routines_symbol_refs function to use #[expect(dead_code, reason = ...)] instead of #[allow(dead_code)] for the compile-time API assertion shim.
  • Replace #[allow(clippy::type_complexity)] on the make_minimal_engine signature assertion with #[expect(clippy::type_complexity, reason = ...)] that explains the intentional tuple return type.
tests/support/routines.rs
tests/support/mod.rs
Introduce tests for routines helper functions to demonstrate real usage and validate their behavior.
  • Add unit tests to verify make_routine initializes fields (name, description, user_id, enabled, run_count, timestamps, and action prompt) as expected for test routines.
  • Add unit tests to validate make_test_incoming_message sets content, channel, user_id, and optional fields correctly for test messages.
  • Add a constructor test for SystemEventSpec::new ensuring source, event_type, and payload are stored properly.
  • Add async tokio tests that create a test database via create_test_db and assert that the resulting database can be used and that create_workspace wraps it in a workspace named "default".
tests/support/routines.rs
Document the routines test-support module and its helpers in the testing strategy, including guidance for writing new routine E2E tests.
  • Add a new "Routine and heartbeat E2E test helpers" section describing the tests/support/routines module, its feature gating, and its role for E2E submodules.
  • Introduce a table summarizing each helper function (create_test_db, create_workspace, make_routine, make_test_incoming_message, make_minimal_engine, register_github_issue_routine, assert_system_event_count) and how they are intended to be used in tests.
  • Describe SystemEventSpec semantics and example values, and outline a step-by-step recipe for adding a new routine E2E test using these helpers.
  • Adjust the numbering of subsequent sections so the former manual/ignored tests section becomes 2.6 and fix trailing-whitespace/formatting at the end of the document.
docs/testing-strategy.md

Possibly linked issues

  • #: PR accomplishes the requested #[allow] to #[expect] replacements in tests/support/{mod,routines}.rs and enhances related documentation.

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@coderabbitai
Copy link
Copy Markdown
Contributor Author

coderabbitai Bot commented Apr 7, 2026

Important

Review skipped

This PR was authored by the user configured for CodeRabbit reviews. CodeRabbit does not review PRs authored by this user. It's recommended to use a dedicated user account to post CodeRabbit review feedback.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 623a4983-6b71-4af6-8afb-7bb7bf81e028

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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

@github-actions github-actions Bot added size: M 50-199 changed lines scope: docs Documentation risk: low Changes to docs, tests, or low-risk modules contributor: new First-time contributor and removed size: M 50-199 changed lines labels Apr 7, 2026
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - I've found 1 issue

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="docs/testing-strategy.md" line_range="101" />
<code_context>
+| `make_minimal_engine(trace, db, ws)` | `fn` | Constructs a `RoutineEngine` backed by a `TraceLlm` replaying the given `LlmTrace`, a default `ToolRegistry`, and a `SafetyLayer` with injection checking enabled. Returns both the engine and the mpsc notification receiver so tests can observe or drain the channel. |
</code_context>
<issue_to_address>
**issue (review_instructions):** The acronym “mpsc” is introduced without being defined, which violates the requirement to define uncommon acronyms on first use.

Please expand “mpsc” (for example, “multi-producer, single-consumer (mpsc) notification receiver”) the first time it appears in the document.

<details>
<summary>Review instructions:</summary>

**Path patterns:** `**/*.md`

**Instructions:**
Define uncommon acronyms on first use.

</details>
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread docs/testing-strategy.md
| `create_workspace(db)` | `fn` | Wraps a database reference in an `Arc<Workspace>` named `"default"`. |
| `make_routine(name, trigger, prompt)` | `fn` | Builds a `Routine` value with sensible test defaults (zero cooldown, enabled, empty state). The trigger and prompt are supplied by the caller. |
| `make_test_incoming_message(content)` | `fn` | Builds an `IncomingMessage` on channel `"test"` with `user_id = "default"` and the supplied content string. |
| `make_minimal_engine(trace, db, ws)` | `fn` | Constructs a `RoutineEngine` backed by a `TraceLlm` replaying the given `LlmTrace`, a default `ToolRegistry`, and a `SafetyLayer` with injection checking enabled. Returns both the engine and the mpsc notification receiver so tests can observe or drain the channel. |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

issue (review_instructions): The acronym “mpsc” is introduced without being defined, which violates the requirement to define uncommon acronyms on first use.

Please expand “mpsc” (for example, “multi-producer, single-consumer (mpsc) notification receiver”) the first time it appears in the document.

Review instructions:

Path patterns: **/*.md

Instructions:
Define uncommon acronyms on first use.

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

Labels

contributor: new First-time contributor risk: low Changes to docs, tests, or low-risk modules scope: docs Documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant