Skip to content

test: move inline tests into *_tests.rs files - #180

Merged
senamakel merged 4 commits into
mainfrom
tests-to-files
Oct 1, 2026
Merged

senamakel merged 4 commits into
mainfrom
tests-to-files

Conversation

@senamakel

@senamakel senamakel commented Oct 1, 2026 •

Copy link
Copy Markdown
Member

Summary

Moves the inline #[cfg(test)] mod blocks into 17 sibling <module>_tests.rs files, each declared with #[cfg(test)] + #[path] above mod tests;, and records the rule in CLAUDE.md/AGENTS.md. The moves are mechanical: bodies are copied verbatim (dedented, rustfmt'd) and the module keeps its place in the tree, so use super::*, privacy and relative paths are unchanged. Produced with OpenHuman's scripts/externalize-inline-tests.mjs.

Related issue

None.

API or behavior changes

None. Test-only code moved; no public API or runtime behavior changes.

Validation

Commands actually run, with their outcome:

  • cargo fmt --all -- --check (clean)
  • cargo clippy --all-targets --all-features -- -D warnings (left to CI)
  • cargo check --workspace --tests (passes; cargo build/cargo test left to CI)
  • cargo test --all-features (left to CI)

Tests

No tests added or changed; 17 test modules relocated. Existing test.rs / *_test.rs files are not renamed here.

Documentation

CLAUDE.md / AGENTS.md updated with the *_tests.rs rule.

Checklist

  • The change is focused on one logical change
  • No new #[allow(...)], #[ignore], or relaxed lints
  • No secrets, tokens, or .env contents in the diff or the description

Summary by CodeRabbit

  • Tests
    • Expanded coverage for memory graph operations, contacts and identity resolution, scoring, synchronization, workspace ingestion, and summaries.
    • Added checks for edge cases including duplicate data, malformed records, permission errors, and repeated operations.
  • Documentation
    • Clarified guidance for organizing Rust unit and integration tests.

senamakel and others added 2 commits October 1, 2026 14:16
Extract each inline `#[cfg(test)] mod` into a sibling `<module>_tests.rs`
declared with `#[path]`, and record the rule in the repo guidance.

Co-authored-by: Medulla <medulla@tinyhumans.ai>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-01T13:30:10.457629Z 16047f3 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used
📚 Code guidelines (1)
AGENTS.md — auto-discovered

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 85d58613-b562-4b68-83b1-c2797a076dab

📥 Commits

Reviewing files that changed from the base of the PR and between 16047f3 and d27a8c1.

📒 Files selected for processing (1)
  • src/memory/store/kv.rs

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.


📝 Walkthrough

Walkthrough

Rust unit tests across memory graph, people, sync, and tree modules now reside in sibling test files loaded through path-based module declarations. AGENTS.md documents the test-file convention and an externalization script. Production implementations remain unchanged.

Changes

Rust unit-test externalization

Layer / File(s) Summary
Document the convention and externalize graph tests
AGENTS.md, src/memory/graph/*, src/memory/store/kv.rs
AGENTS.md specifies sibling test-file placement and naming and documents the externalization script. Graph tests cover bounded BFS and edge canonicalization, weights, and symmetry. The key-value store safety documentation link now uses an intra-doc reference.
Externalize people tests
src/memory/people/*
Sibling test files cover address-book reading, migrations, identity resolution and seeding, scoring, storage, and handle canonicalization.
Externalize sync and tree tests
src/memory/sync/*, src/memory/tree/*
Sibling test files cover sync audit and state, response decoding, GitHub parsing, cadence, rebuild, status and workspace behavior, plus direct ingestion.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Refactor

Merge Risk: ⚪ Minimal · up to d27a8

The PR relocates existing tests and documents their convention; no actionable merge risk remains. It is mergeable subject to routine CI checks.

Architecture Summary

Architecture risk: 🔵 Low · up to d27a8

The change affects 2 systems.

Changed systems: src, AGENTS.md

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — src (service) was modified; 35 changed files map to changed impact.
  • observed — AGENTS.md (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in AGENTS.md: Adds rules against inline unit-test modules; directs authors to declare sibling test files with #[path], specifies their imports, naming, and placement, and documents the externalization script and its --write behavior.
  • observed — Modified behavior in src/memory/graph/bfs.rs: The inline test module, including its bounded two-hop BFS test, was removed from this file and replaced by a module declaration that loads tests from bfs_tests.rs.
  • observed — Modified behavior in src/memory/graph/bfs_tests.rs: Adds bounded_bfs_finds_two_hop_pair, which creates two edges (alice–bob and bob–carol) and asserts that a bound of 1 yields no result while a bound of 2 yields the pair at distance 2.
  • observed — Modified behavior in src/memory/graph/edge_store.rs: Replaced the inline edge-store test module with a module declaration that loads tests from edge_store_tests.rs.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 59.76% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 82 functions across 35 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: moving inline tests into sibling *_tests.rs files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


A rabbit files tests beside their code
With paths declared to guide the load
Graph hops and people scores now shine
Sync checks sit in files aligned
The source stays still; the tests take flight

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

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 16047f3a69

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@@ -0,0 +1,60 @@
use std::collections::HashMap;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Start every extracted test file with use super::*

Reorder the imports here and in status_tests.rs and workspace_tests.rs: these three newly extracted test files begin with standard-library or crate imports instead of use super::*;, contrary to the test-file convention introduced by this commit.

AGENTS.md reference: AGENTS.md:L46-L48

Useful? React with 👍 / 👎.

@tinysweeper

tinysweeper Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Tiny Sweeper review

Tiny Sweeper reviewed this change across 6 lane(s) and found 0 active actionable finding(s). Detailed lane evidence and any incomplete work are listed below.

State: Incomplete
Priority: low
Reviewed head: d27a8c14f768
Updated: 1790869305 (Unix time)

Review snapshot

Change surface Files Review signal Count
Production 18 Active findings 0
Tests 17 Noted findings 0
Documentation 1 Resolved findings 0
Configuration 0 Pending checks/questions 2

Completeness: Incomplete
Test assessment: No supported feature-to-test mapping was available; this does not mean tests are absent or passed.

What changed

The review could not produce a supported behavioral summary; inspect the cited changed surface and lane details below.

Features

None identified with supported citations.

Tests

No supported feature-to-test mapping was produced. Test execution is not inferred.

Findings

No active actionable findings.

Could not review: src/memory/store/kv.rs

Before merge

  • Complete the critique review for src/memory/store/kv.rs.
  • Complete the security review for src/memory/store/kv.rs.

How this fits together

flowchart LR
  n0["pair_distances<br/>changed"]:::changed
  n1["count_edges<br/>changed"]:::changed
  n2["MemoryConfig"]:::impacted
  n3["push"]:::impacted
  n0 -->|uses| n2
  n0 -->|calls| n3
  n1 -->|uses| n2
  classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
  classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
  classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
  classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Loading
Agent review details

critique

  • Conclusion: Neutral
  • Scope reviewed: incomplete; unanswered: src/memory/store/kv.rs
  • Lane summary: Reviewed 0 files; 0 findings. 1 file could not be reviewed: src/memory/store/kv.rs.

security

  • Conclusion: Neutral
  • Scope reviewed: incomplete; unanswered: src/memory/store/kv.rs
  • Lane summary: Reviewed 0 files; 0 findings. 1 file could not be reviewed: src/memory/store/kv.rs.

tests

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: The change corrects an intra-doc link in `src/memory/store/kv.rs` from an absolute path to a relative reference, fixing a broken link. No behavioural code change; no test needed. Safe to merge. _The code index is behind this pull request (indexed at `16047f3a69b4`), so retrieved context may be out of date._ _Memory was unavailable (model: cortex: v1/recall: timed out after 10s), so this review ran without it._

commits

  • Conclusion: Neutral
  • Scope reviewed: all assigned evidence
  • Lane summary: Nothing sensitive found in what this pull request commits.

description

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: The test moves are clean, but the doc link fix in `src/memory/store/kv.rs` breaks the intra-doc link by removing the path; it should be restored to the fully qualified form. _The code index is behind this pull request (indexed at `16047f3a69b4`), so retrieved context may be out of date._ _Memory was unavailable (model: cortex: v1/recall: timed out after 10s), so this review ran without it._

e2e

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: This pull request moves inline test modules into sibling `*_tests.rs` files and adds a project convention documenting the pattern. No external behaviour changes — no routes, commands, persisted formats, or queue messages are affected. The refactor is safe to merge. _The code index is behind this pull request (indexed at `16047f3a69b4`), so retrieved context may be out of date._ _Memory was unavailable (model: cortex: v1/recall: timed out after 10s), so this review ran without it._
Evidence and run details
  • Models: ladder/vectors, deepseek/deepseek-v4-flash
  • Spend: $0.012467
  • Tokens: 175484 input · 7350 output · 1280 cached · 1272 embedding
Head State Pass summary
16047f3a69b4 incomplete 0 active finding(s), 0 resolved finding(s) (at 1790862631)
d27a8c14f768 incomplete 0 active finding(s), 0 resolved finding(s) (at 1790869305)

tinysweeper 0.1.0

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

tinysweeper found nothing blocking, but could not review everything, so this is not an approval: AGENTS.md, src/memory/graph/bfs.rs, src/memory/graph/bfs_tests.rs, src/memory/graph/edge_store.rs, src/memory/graph/edge_store_tests.rs, src/memory/people/address_book.rs, src/memory/people/address_book_tests.rs, src/memory/people/migrations.rs and 27 more.

             $0.0124 · 180,152 in / 6,263 out · 0 cached (0%) · flash, ladder/vectors, deepseek/deepseek-v4-flash · 1,264 embedded
tests:       $0.0044 · 48,134 in  / 605 out   · 0 cached (0%) · deepseek/deepseek-v4-flash
description: $0.0037 · 38,357 in  / 1,330 out · 0 cached (0%) · deepseek/deepseek-v4-flash
e2e:         $0.0021 · 49,966 in  / 381 out   · 0 cached (0%) · deepseek/deepseek-v4-flash

@tinysweeper tinysweeper Bot added the priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect. label Oct 1, 2026
Changed the intra-doc link for the safety guard from an absolute path to a relative one using `super::safety`, fixing a broken documentation reference that would have failed to resolve in the rendered docs.

Auto-committed-on: dragonfly
The doc comment for the KvStore module used a fully qualified path to reference the `safety` guard, but the link is within the same parent module so the shorter form works and avoids a stale reference if the module is ever reorganised.

Auto-committed-on: dragonfly
@senamakel
senamakel merged commit 21c1235 into main Oct 1, 2026
8 checks passed

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

tinysweeper found nothing blocking, but could not review everything, so this is not an approval: src/memory/store/kv.rs.

             $0.0125 · 175,484 in / 7,350 out · 1,280 cached (1%) · ladder/vectors, deepseek/deepseek-v4-flash · 1,272 embedded
tests:       $0.0020 · 47,406 in  / 412 out   · 0 cached (0%)     · deepseek/deepseek-v4-flash
description: $0.0040 · 37,658 in  / 3,834 out · 1,280 cached (3%) · deepseek/deepseek-v4-flash
e2e:         $0.0045 · 49,144 in  / 394 out   · 0 cached (0%)     · deepseek/deepseek-v4-flash

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

Labels

priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant