scope peer dialectic call

[Rajat-Ahuja1997]

No description provided.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

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

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch rajat/scope-dialectic-calls-for-peer

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.

---

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

[Rajat-Ahuja1997]

@claude can you review this PR

[claude]

Claude finished @Rajat-Ahuja1997's task in 2m 16s —— View job

---

PR Review: Scope Peer Dialectic Calls

• Gather context and read changed files
• Review src/crud/message.py changes
• Review src/dialectic/core.py changes
• Review src/utils/agent_tools.py changes
• Review workspace-level changes (workspace.py, prompts.py, document.py)
• Review new test files
• Post final review

---

Summary

This PR implements peer-scoped visibility for dialectic tool calls. The core idea: when a peer (observer) uses dialectic to query information, they should only see messages from sessions they were actually a member of, and only during the time window they were present. This is a solid security/privacy improvement.

The PR also refactors the workspace-level search from an "omniscient" cross-peer document search (query_documents_workspace) to a scoped approach where the agent must specify explicit observer/observed pairs when querying memory.

---

Architecture & Approach

The overall approach is clean:

1. peer_visibility_condition() (src/crud/message.py:21-43) — A reusable SQL EXISTS subquery that checks session membership with temporal windowing (joined_at / left_at). This is the right abstraction.

2. Consistent threading — The peer_perspective parameter is threaded through all message query functions (get_messages, search_messages, grep_messages, get_messages_by_date_range, search_messages_temporal, _build_merged_snippets), and through the agent tool handlers.

3. Workspace scoping — Replacing query_documents_workspace (unscoped cross-peer search) with scoped query_documents calls requiring explicit observer/observed is a good design decision.

---

Issues Found

1. Workspace fallback search is not scoped by peer perspective

In _handle_search_memory_workspace (src/utils/agent_tools.py:1930-1939), when no documents are found, the fallback search_messages call does not pass peer_perspective:

snippets = await crud.search_messages(
    ctx.db,
    workspace_name=ctx.workspace_name,
    session_name=ctx.session_name,
    query=tool_input["query"],
    limit=min(tool_input.get("top_k", 20), 20),
    context_window=0,
    # peer_perspective is missing here
)

This is intentional for the workspace-level agent (which is designed to have a cross-peer view), but worth noting explicitly in a comment since every other search_messages call in this PR carefully passes peer_perspective. It could be confusing to future readers.

Add a comment clarifying this is intentional →

2. peer_visibility_condition uses correlated subquery on every row

The peer_visibility_condition function (src/crud/message.py:21-43) generates an EXISTS subquery that correlates on models.Message.session_name and models.Message.created_at. This means the subquery runs for every candidate message row. For queries scanning many messages across many sessions, this could become a performance bottleneck.

This is functionally correct and probably fine at current scale, but worth monitoring. If it becomes slow, an index on session_peers_table(workspace_name, session_name, peer_name, joined_at) would help (if not already present).

3. Line length in peer_visibility_condition

src/crud/message.py:33 exceeds the 88-char Black-compatible line limit:

.where(models.session_peers_table.c.session_name == models.Message.session_name)

This should be wrapped for consistency with the project's style.

Fix line length →

---

Test Coverage

Test coverage is thorough:

• tests/crud/test_message_perspective.py — 5 tests covering all CRUD message functions (search_messages, get_messages, grep_messages, get_messages_by_date_range, search_messages_temporal) with both scoped and unscoped assertions. Good use of temporal windowing (messages before/after join, messages in sessions the peer never joined).

• tests/dialectic/test_core.py — Tests that DialecticAgent._initialize_session_history() respects peer perspective visibility, including an idempotency check.

• tests/utils/test_agent_tools.py — New tests verify peer_perspective is correctly threaded through _handle_search_messages, _handle_get_recent_history, _handle_search_memory (dialectic fallback), _handle_extract_preferences, and the get_recent_history no-session path.

• tests/unified/test_cases/peer_isolation_test.json — End-to-end test: peer Z cannot access peer X's secret shared only with peer Y in a separate session, while workspace-level chat can.

One minor note: the CRUD tests create MessageEmbedding records with synthetic embeddings (e.g., [0.1] * 1536) which works for testing the filtering logic but won't exercise the actual semantic ordering. This is fine for unit tests.

---

Other Notes

• Deleted query_documents_workspace (src/crud/document.py) — Good cleanup. The workspace-level search now routes through the existing query_documents with explicit observer/observed, which is more consistent.

• Deleted .github/workflows/claude.yml — This file deletion is unrelated to the PR's purpose. If intentional, it should probably be in a separate commit/PR for clarity.

• Prompt changes (src/dialectic/prompts.py) — The workspace agent prompt is updated to guide the LLM toward explicitly querying specific peer pairs rather than expecting omniscient results. The instructions are clear and well-structured.

• Prefetch change (src/dialectic/workspace.py:53-58) — The workspace agent's _prefetch_relevant_observations now prefetches the peer list instead of doing a cross-workspace observation search. This aligns with the new model where the agent must discover peers and search individually.

---

Verdict

The core implementation is solid — peer_visibility_condition is well-designed, the parameter threading is consistent across all code paths, and test coverage is comprehensive. The main suggestion is adding a clarifying comment on the intentionally-unscoped workspace fallback search.