Skip to content

Exclude only conversation turns from a read's thread window - #277

Merged
sanil-23 merged 1 commit into
tinyhumansai:mainfrom
sanil-23:fix/exclude-thread-conversations-only
Oct 15, 2026
Merged

sanil-23 merged 1 commit into
tinyhumansai:mainfrom
sanil-23:fix/exclude-thread-conversations-only

Conversation

@sanil-23

@sanil-23 sanil-23 commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes a bug in #276: exclude_thread is documented as leaving out the calling thread's conversation turns, but the engines matched any item carrying the thread id. OpenHuman stamps the calling thread on what its agent learns in that thread, so with #276 the agent's recall and fetch hid its own learnings from that conversation. OpenHuman's memory tool tests caught it while I was wiring #276 into OpenHuman.

Related issue

Follow-up to #276. Context: tinyhumansai/openhuman-benchmarks#38.

API or behavior changes

  • New: ThreadWindow::excludes(kind, meta), true only for a Conversation item inside the window.
  • Behaviour fix: the reference engine and the CortexDB engine's fetch hits, recall citations and evidence-mode rendering now use excludes, so learnings and documents stamped with the excluded thread stay readable. covers / covers_meta are unchanged, and the pre-turn HolisticRecall exclusion is unchanged.
  • Conformance: excluded_thread now also requires a learning stamped with the excluded thread to be returned by every fetch mode and cited by recall. It runs after recall, so a recall fault is still reported by the recall check (each_fault_is_caught_by_its_check).

Validation

Commands actually run, with their outcome:

  • cargo fmt --all -- --check: clean
  • cargo clippy --all-targets --all-features -- -D warnings: clean
  • cargo build --all-targets --all-features: ok
  • cargo test --all-features: 1476 passed, 0 failed
  • RUSTDOCFLAGS="-D warnings" cargo doc --no-deps --all-features: ok

Supply chain CI fails on main too: rustls 0.23.45, RUSTSEC-2026-0346 and RUSTSEC-2026-0347. It is unrelated; the fix is cargo update -p rustls (0.23.46).

Tests

  • query::tests::a_thread_window_excludes_conversation_turns_only
  • cortex::engine::evidence::tests::a_learning_made_in_an_excluded_thread_stays_in_the_evidence
  • The conformance excluded_thread learning case, run against the reference engine and both CortexDB engines on the mock.

Documentation

ThreadWindow's rustdoc points to excludes. The request field docs already said "conversation turns".

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

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Thread exclusions now filter matching conversation turns from recall, fetch results, and gathered evidence without hiding associated learning records or documents.
    • Recall citations and fetch modes now preserve learning records linked to an excluded thread.

exclude_thread is documented as leaving the calling thread's
conversation turns out, but the engines matched any item carrying the
thread id. A host that stamps the thread on what the agent learns in it
(OpenHuman's memory tool does) then lost those learnings from its own
recall and fetch, which OpenHuman's memory tool tests caught.

ThreadWindow::excludes(kind, meta) is true only for a conversation turn
inside the window; the reference engine and the CortexDB engine's fetch,
recall citations and evidence use it. The pre-turn pack's
HolisticRecall exclusion is unchanged. Conformance now requires a
learning stamped with the excluded thread to stay readable, and runs the
exclusion check after recall so a recall fault is still reported by the
recall check.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@tinysweeper

tinysweeper Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Tiny Sweeper review

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

State: Ready for maintainer review
Priority: none
Reviewed head: f6a1b3cdae51
Updated: 2026-10-14T19:29:01Z

Review snapshot

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

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

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.

Before merge

None.

How this fits together

flowchart LR
  n0["excluded_thread<br/>changed"]:::changed
  n1["fetch_filters<br/>changed"]:::changed
  n2["RecallRequest<br/>changed"]:::changed
  n3["ThreadWindow<br/>changed"]:::changed
  n4["gather<br/>changed"]:::changed
  n5["turns_are_separated_and_attributed_by_role<br/>changed"]:::changed
  n6["whole"]:::impacted
  n7["map"]:::impacted
  n8["clone"]:::impacted
  n9["recall"]:::impacted
  n10["all"]:::impacted
  n11["list_all"]:::impacted
  n0 -->|calls| n6
  n0 -->|calls| n7
  n0 -->|calls| n8
  n0 -->|calls| n9
  n0 -->|calls| n10
  n1 -->|calls| n7
  n1 -->|calls| n8
  n1 -->|calls| n10
  n1 -->|calls| n11
  n2 -->|uses| n3
  n4 -->|calls| n8
  n5 -->|calls| n7
  n5 -->|tests| n7
  n9 -->|calls| n8
  n9 -->|calls| n11
  n10 -->|calls| n0
  n10 -->|calls| n1
  n10 -->|calls| n9
  n11 -->|calls| n8
  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: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: Reviewed 7 files; 0 findings. _The code index is behind this pull request (indexed at `d693a6e974af`), so retrieved context may be out of date._

security

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: Reviewed 7 files; 0 findings. _The code index is behind this pull request (indexed at `d693a6e974af`), so retrieved context may be out of date._

tests

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: The change narrows `exclude_thread` from "anything whose metadata matches the thread window" to conversation turns only, adding `ThreadWindow::excludes`, switching the reference engine, the Cortex evidence gatherer and the Cortex fetch ranker to it, and extending the conformance check so fetch and recall must still return a learning stamped with the excluded thread. The new predicate is pinned by a unit test (`a_thread_window_excludes_conversation_turns_only`), an evidence test (`a_learning_made_in_an_excluded_thread_stays_in_the_evidence`), and the widened `excluded_thread` conformance check, so the invariant it states is backed; I found no defect to report. _The code index is behind this pull request (indexed at `d693a6e974af`), so retrieved context may be out of date._

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 change adds `ThreadWindow::excludes`, which narrows `exclude_thread` to `ItemKind::Conversation` items, and switches the reference engine's fetch/recall filtering and the CortexDB engine's fetch/evidence filtering to it; the conformance `excluded_thread` check and two unit tests cover the new behaviour. The description matches the diff, and the remaining call sites that still use `covers` (`HolisticRecall::excludes` for the request-level exclusion, plus `TimeHint::covers`) are outside the window being changed, so I found no reportable defect. _The code index is behind this pull request (indexed at `d693a6e974af`), so retrieved context may be out of date._
Evidence and run details
  • Models: ladder/vectors-oai3, glm-5.3-flash, deepseek-v4.1-flash
  • Spend: $0.004575
  • Tokens: 600267 input · 33630 output · 98688 cached · 412 embedding
Head State Pass summary
f6a1b3cdae51 ready for maintainer review 0 active finding(s), 0 resolved finding(s) (at 2026-10-14T19:29:01Z)

tinysweeper 0.1.2

@coderabbitai

coderabbitai Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

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: 76230a76-fdc9-4da8-9ae3-facc8ea58a0f
📥 Commits

Reviewing files that changed from the base of the PR and between 9fcf47e and f6a1b3c.

📒 Files selected for processing: 7
  • crates/tinymemory-api/src/conformance/reference/mod.rs
  • crates/tinymemory-api/src/conformance/suite/checks.rs
  • crates/tinymemory-api/src/query/mod.rs
  • crates/tinymemory-api/src/query/mod_tests.rs
  • crates/tinymemory-integrations/src/cortex/engine/evidence.rs
  • crates/tinymemory-integrations/src/cortex/engine/evidence_tests.rs
  • crates/tinymemory-integrations/src/cortex/engine/fetch.rs

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

📝 Walkthrough

Thread-window exclusion now targets conversation items based on their kind and metadata. Learning and document items with matching thread metadata remain eligible for retrieval. Reference and Cortex paths, along with conformance checks, use or verify this behavior.

Changes: Thread exclusion

Layer / File(s) Summary
Define kind-aware exclusion
crates/tinymemory-api/src/query/mod.rs, crates/tinymemory-api/src/query/mod_tests.rs
ThreadWindow::excludes checks thread coverage only for conversation items. Tests cover matching conversation, learning, and document items.
Apply exclusion in retrieval paths
crates/tinymemory-api/src/conformance/reference/mod.rs, crates/tinymemory-api/src/conformance/suite/checks.rs, crates/tinymemory-integrations/src/cortex/engine/*
Reference and Cortex retrieval paths use kind-aware exclusion. Conformance checks verify conversation exclusion and the continued availability of a learning stamped with the excluded thread.

Priority: ⬇️ Low

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

Change: Bug fix

Suggested reviewers: senamakel

Merge Risk: ⚪ Minimal · up to f6a1b

Thread-window filtering now excludes covered conversation turns while keeping matching learnings and documents eligible. No actionable merge-blocking risk is established in the supplied review context.

Security architecture risk: ⚪ Minimal · up to f6a1b

The change restores access to learnings and documents associated with an excluded conversation while preserving the existing query-scope filters. No material security risk introduced or worsened by this change was identified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The direct exposure change is additional same-thread learning and document content becoming eligible for fetch results and recall evidence within the existing retrieval scope. The inspected change does not broaden namespace selection or grant additional service privileges.

Trust Boundaries and Controls

  • observed — Reference ranking applies MetaFilter before thread exclusion. Cortex scope selection separately checks admitted kinds and namespace reach, and decoded-event filtering retains metadata checks before exclusion. These are preserved query-scope controls; they do not establish authorization enforcement outside the retrieval layer.
Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 76.92% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 7 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed 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: thread-window exclusions now apply only to conversation turns.
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
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

A rabbit checks the thread with care,
And leaves its learning waiting there.
The turns step out of view,
While documents stay in view too.
A carrot cheers the tests anew.

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

@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. Approving.

             $0.0046 · 600,267 in / 33,630 out · 98,688 cached (16%) · ladder/vectors-oai3, glm-5.3-flash, deepseek-v4.1-flash · 412 embedded
critique:    $0.0021 · 265,711 in / 14,111 out · 45,376 cached (17%) · glm-5.3-flash
security:    $0.0018 · 240,598 in / 11,944 out · 45,376 cached (19%) · glm-5.3-flash
tests:       $0.0004 · 54,718 in  / 5,119 out  · 2,048 cached (4%)   · deepseek-v4.1-flash, glm-5.3-flash
description: $0.0001 · 34,577 in  / 833 out    · 5,888 cached (17%)  · deepseek-v4.1-flash

@tinysweeper tinysweeper Bot added the priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect. label Oct 14, 2026
@sanil-23
sanil-23 merged commit bb781ca into tinyhumansai:main Oct 15, 2026
23 of 24 checks passed
sanil-23 added a commit to sanil-23/openhuman that referenced this pull request Oct 15, 2026
…#277

Same tree as the fork commit f6a1b3cd the previous commit pinned; now
reachable from tinyhumansai/tinymemory.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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