Replace #[allow(...)] with narrowly scoped #[expect(..., reason = "...")] in test support modules

[coderabbitai]

Background

Test support code must be placed in scoped and cfg-guarded modules so that only the code needed by a given test compilation is included, rather than relying on broad lint suppressions.

Raised during review of PR #116 by @leynos.

Required changes

tests/support/mod.rs (around lines 362–385)

Replace the two forbidden #[allow(...)] attributes in the routines_symbol_refs function with narrowly scoped #[expect(..., reason = "...")] attributes:

1. Change #[allow(dead_code)] above routines_symbol_refs to:

   #[expect(dead_code, reason = "compile-time symbol reference used for API stability checks in tests")]

2. Change #[allow(clippy::type_complexity)] immediately before the complex const type assertion for routines::make_minimal_engine to:

   #[expect(clippy::type_complexity, reason = "explicit compile-time type assertion for RoutineEngine/Receiver tuple in tests")]

Keep attributes local to the function. Ensure attribute names and reasons reference the symbols involved (routines_symbol_refs, routines::create_workspace, routines::make_minimal_engine, routines::make_routine, routines::make_test_incoming_message) so the intent is clear.

tests/support/routines.rs (lines 7–8 and 27–43)

1. Remove the crate-level #![allow(dead_code)] attribute (lines 7–8).

2. Add #[expect(dead_code, reason = "used only in e2e_traces")] directly on the specific items (functions/structs/consts) that are only referenced by e2e_traces. If an item cannot be identified immediately, apply the expect attribute to the smallest enclosing module or individual item rather than a crate-wide attribute.

3. Replace every remaining #[allow(dead_code)] occurrence (lines 27–43 and elsewhere) with a narrowly scoped:

   #[expect(dead_code, reason = "<meaningful justification for the specific symbol>")]

   Specifically update:

   • The struct and impl for SystemEventSpec (symbols: SystemEventSpec, new)
   • Every other occurrence at the noted lines

   Each attribute must be converted individually with a meaningful reason text for that symbol.

Acceptance criteria

• No #[allow(dead_code)] or #[allow(clippy::...)] attributes remain in tests/support/mod.rs or tests/support/routines.rs.
• All replacements use #[expect(..., reason = "...")] with a non-empty, symbol-specific reason string.
• The #![allow(dead_code)] crate-level attribute is removed from tests/support/routines.rs.
• Lint suppression is scoped to the smallest applicable item, not a whole module or crate.

References

• PR: Refactor: split routine_heartbeat.rs into focused submodules (#25) #116
• Comment: Refactor: split routine_heartbeat.rs into focused submodules (#25) #116 (comment)

[leynos]

Here is the revised approach. The principle is: make the compiler not see an item in a compilation unit where it would be unused, rather than suppressing the lint. Two mechanisms achieve this:

1. Local type alias to dissolve clippy::type_complexity without suppression.
2. Narrow inline pub mod blocks inside routines.rs so items that are only consumed by one submodule are not visible to the rest of the binary.

---

tests/support/mod.rs — remove both #[allow] attributes

The routines_symbol_refs function is already referenced by const _: fn() = routines_symbol_refs;, so it is not dead. Drop #[allow(dead_code)] entirely. Replace #[allow(clippy::type_complexity)] with a local type alias inside the function body.

--- a/tests/support/mod.rs
+++ b/tests/support/mod.rs
@@ -362,9 +362,6 @@ const _: fn() = test_rig_symbol_refs;
 #[cfg(feature = "libsql")]
 const _: fn() = routines_symbol_refs;

-#[cfg(feature = "libsql")]
-#[allow(dead_code)]
-fn routines_symbol_refs() {
+#[cfg(feature = "libsql")]
+fn routines_symbol_refs() {
     // Compile-time type assertions for routines module helpers.
     // These ensure the public API signatures remain stable.
     const _: fn(
         &std::sync::Arc<dyn ironclaw::db::Database>,
     ) -> std::sync::Arc<ironclaw::workspace::Workspace> = routines::create_workspace;
     const _: fn(
         &str,
         ironclaw::agent::routine::Trigger,
         &str,
     ) -> ironclaw::agent::routine::Routine = routines::make_routine;
     const _: fn(&str) -> ironclaw::channels::IncomingMessage = routines::make_test_incoming_message;
-    #[allow(clippy::type_complexity)]
-    const _: fn(
+
+    type MinimalEngineFn = fn(
         trace_llm::LlmTrace,
         std::sync::Arc<dyn ironclaw::db::Database>,
         std::sync::Arc<ironclaw::workspace::Workspace>,
     ) -> (
         std::sync::Arc<ironclaw::agent::routine_engine::RoutineEngine>,
         tokio::sync::mpsc::Receiver<ironclaw::channels::OutgoingResponse>,
-    ) = routines::make_minimal_engine;
+    );
+    const _: MinimalEngineFn = routines::make_minimal_engine;
 }

---

tests/support/routines.rs — remove all #[allow], group by usage with narrow inline pub mod

Remove every #[allow(dead_code)] attribute. Items used across all routine submodules (create_test_db, create_workspace, make_routine, make_test_incoming_message, make_minimal_engine) are genuinely referenced and need nothing. Items that are only consumed by routine_system_event.rs (SystemEventSpec, register_github_issue_routine, assert_system_event_count) move into a narrow pub mod system_events inline block. The #![cfg(feature = "libsql")] file-level gate already ensures nothing in this file is compiled without the feature; the sub-module provides the further scoping.

--- a/tests/support/routines.rs
+++ b/tests/support/routines.rs
@@ -1,7 +1,7 @@
 //! Shared test-support utilities for routine and heartbeat tests.

 #![cfg(feature = "libsql")]

 use std::sync::Arc;
 use std::time::Duration;

@@ -19,21 +19,6 @@ use crate::support::trace_llm::{LlmTrace, TraceLlm};

-/// Describes a system event to be emitted in tests.
-#[allow(dead_code)]
-pub struct SystemEventSpec<'a> {
-    pub source: &'a str,
-    pub event_type: &'a str,
-    pub payload: serde_json::Value,
-}
-
-#[allow(dead_code)]
-impl<'a> SystemEventSpec<'a> {
-    pub fn new(source: &'a str, event_type: &'a str, payload: serde_json::Value) -> Self {
-        Self { source, event_type, payload }
-    }
-}
-
 /// Create a temp libSQL database with migrations applied.
-#[allow(dead_code)]
 pub async fn create_test_db() -> Result<(Arc<dyn Database>, TempDir), Box<dyn std::error::Error>> {
     use ironclaw::db::libsql::LibSqlBackend;

@@ -46,7 +31,6 @@ pub async fn create_test_db() -> Result<(Arc<dyn Database>, TempDir), Box<dyn s
 }

 /// Create a workspace backed by the test database.
-#[allow(dead_code)]
 pub fn create_workspace(db: &Arc<dyn Database>) -> Arc<Workspace> {
     Arc::new(Workspace::new_with_db("default", db.clone()))
 }

 /// Helper to insert a routine directly into the database.
-#[allow(dead_code)]
 pub fn make_routine(name: &str, trigger: Trigger, prompt: &str) -> Routine {
     // ... existing body unchanged ...
 }

 /// Build a minimal IncomingMessage for event-trigger tests.
-#[allow(dead_code)]
 pub fn make_test_incoming_message(content: &str) -> IncomingMessage {
     // ... existing body unchanged ...
 }

 /// Build a minimal RoutineEngine, returning the engine and the notify receiver.
-#[allow(dead_code)]
 pub fn make_minimal_engine(
     trace: LlmTrace,
     db: Arc<dyn Database>,
     ws: Arc<Workspace>,
 ) -> (
     Arc<RoutineEngine>,
     tokio::sync::mpsc::Receiver<ironclaw::channels::OutgoingResponse>,
 ) {
     // ... existing body unchanged ...
 }
+
+/// Helpers used exclusively by `routine_system_event` tests.
+///
+/// Grouping them in a narrow inline module avoids dead-code visibility
+/// in compilation units that do not import this module.
+pub mod system_events {
+    use super::*;
+
+    /// Describes a system event to be emitted in a test.
+    pub struct SystemEventSpec<'a> {
+        pub source: &'a str,
+        pub event_type: &'a str,
+        pub payload: serde_json::Value,
+    }
+
+    impl<'a> SystemEventSpec<'a> {
+        pub fn new(source: &'a str, event_type: &'a str, payload: serde_json::Value) -> Self {
+            Self { source, event_type, payload }
+        }
+    }
+
+    /// Register a GitHub issue routine for system event tests.
+    pub async fn register_github_issue_routine(
+        db: &Arc<dyn Database>,
+        engine: &RoutineEngine,
+    ) -> Routine {
+        let mut filters = std::collections::HashMap::new();
+        filters.insert("repository".to_string(), "nearai/ironclaw".to_string());
+        let routine = super::make_routine(
+            "github-issue-opened",
+            Trigger::SystemEvent {
+                source: "github".to_string(),
+                event_type: "issue.opened".to_string(),
+                filters,
+            },
+            "Summarize the issue and propose an implementation plan.",
+        );
+        db.create_routine(&routine).await.expect("create_routine");
+        engine.refresh_event_cache().await;
+        routine
+    }
+
+    /// Assert that a system event fires the expected number of routines.
+    pub async fn assert_system_event_count(
+        engine: &RoutineEngine,
+        spec: SystemEventSpec<'_>,
+        expected: usize,
+        msg: &str,
+    ) {
+        let fired = engine
+            .emit_system_event(spec.source, spec.event_type, &spec.payload, Some("default"))
+            .await;
+        assert_eq!(fired, expected, "{msg}");
+    }
+}

---

tests/e2e_traces/routine_system_event.rs — update import path

Update the use statement in routine_system_event.rs to reference the new sub-module path:

-use crate::support::routines::{
-    SystemEventSpec, assert_system_event_count, create_test_db, create_workspace,
-    make_minimal_engine, register_github_issue_routine,
-};
+use crate::support::routines::{
+    create_test_db, create_workspace, make_minimal_engine,
+};
+use crate::support::routines::system_events::{
+    SystemEventSpec, assert_system_event_count, register_github_issue_routine,
+};

Update the routines_symbol_refs const-assertion block in tests/support/mod.rs similarly if it references those symbols directly — or remove those specific assertions given the items are now unambiguously referenced from a single, named consumer module.

---

Summary of what this achieves

Problem	Old fix	New fix
dead_code on routines_symbol_refs	#[allow(dead_code)]	Removed — function is not dead (const _ references it)
clippy::type_complexity on make_minimal_engine assertion	#[allow(clippy::type_complexity)]	Local type MinimalEngineFn = ... alias
dead_code on system-event helpers	#[allow(dead_code)] per item	Narrow pub mod system_events { } inline block — only compiled when imported
dead_code on shared helpers	#[allow(dead_code)] per item	Removed — items are genuinely referenced by callers