Skip to content

fix: resolve get_effective_observe me race condition, default peer config#176

Merged
VVoruganti merged 3 commits into
mainfrom
ben/fix-get-effective-observe-me-race-condition
Aug 5, 2025
Merged

fix: resolve get_effective_observe me race condition, default peer config#176
VVoruganti merged 3 commits into
mainfrom
ben/fix-get-effective-observe-me-race-condition

Conversation

@dr-frmr

@dr-frmr dr-frmr commented Jul 31, 2025

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • Bug Fixes
    • Improved handling of missing peer configurations to prevent errors when a sender is no longer present, ensuring more robust and error-free operation.
    • Prevented generation of representation tasks for inactive observer peers to optimize session processing.
  • New Features
    • Extended peer session information to include active status, providing better visibility into peer participation.
  • Tests
    • Added extensive tests covering edge cases and race conditions related to peers leaving sessions, ensuring reliable message processing and configuration handling.

@coderabbitai

coderabbitai Bot commented Jul 31, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

The session peer configuration retrieval was updated to include all peers, not just active ones, by adding an is_active flag indicating peer status. The get_peers_with_configuration function now returns this flag, and generate_queue_records skips enqueueing tasks for inactive observer peers. The get_effective_observe_me function was modified to handle missing peers safely. Extensive new tests were added to cover edge cases and race conditions involving peers leaving sessions.

Changes

Cohort / File(s) Change Summary
Session Peer Configuration Update
src/crud/session.py
Modified get_session_peer_configuration to return all peers with an added is_active boolean flag indicating active status. SQL query updated to remove active-only filter and add computed is_active column.
Peers Configuration and Queue Logic
src/deriver/enqueue.py
Updated get_peers_with_configuration to include is_active flag per peer. Modified get_effective_observe_me to safely handle missing sender keys using .get() with a default. Added logic in generate_queue_records to skip enqueueing for inactive observer peers.
Integration and Unit Tests for Enqueue Logic
tests/integration/test_enqueue.py
Added extensive new async integration tests and unit tests covering race conditions, peers leaving sessions, missing configurations, and default observe_me behavior. Introduced helper methods for payload creation and queue item counting.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~40 minutes

Poem

A rabbit hops through session's maze,
Tracking peers in all their ways.
Active or not, now all are known,
Skipping tasks where seeds aren't sown.
With safety checks and flags so bright,
Code hops forward, swift and light!
🐇🌿✨

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch ben/fix-get-effective-observe-me-race-condition

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 1

🔭 Outside diff range comments (1)
src/deriver/enqueue.py (1)

101-127: Consider updating function signature for better type safety.

The function return type should be updated to reflect the new structure including the is_active boolean flag.

Apply this diff to improve type clarity:

-) -> dict[str, list[dict[str, Any]]]:
+) -> dict[str, list[dict[str, Any] | bool]]:

Or more precisely:

-) -> dict[str, list[dict[str, Any]]]:
+) -> dict[str, tuple[dict[str, Any], dict[str, Any], bool]]:

And update the return statement accordingly:

     return {
-        row.peer_name: [
+        row.peer_name: (
             row.peer_configuration,
             row.session_peer_configuration,
             row.is_active,
-        ]
+        )
         for row in peers_with_configuration_list
     }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between db6082f and 62d4e16.

📒 Files selected for processing (2)
  • src/crud/session.py (1 hunks)
  • src/deriver/enqueue.py (3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*.py

📄 CodeRabbit Inference Engine (CLAUDE.md)

src/**/*.py: Follow isort conventions with absolute imports preferred
snake_case for variables/functions; PascalCase for classes
Line length: 88 chars (Black compatible)
Explicit error handling with appropriate exception types
Docstrings: Use Google style docstrings
Use environment variables via python-dotenv (.env)
Use specific exception types (ResourceNotFoundException, ValidationException, etc.)
Proper logging with context instead of print statements

Files:

  • src/crud/session.py
  • src/deriver/enqueue.py
🧠 Learnings (4)
📓 Common learnings
Learnt from: Rajat-Ahuja1997
PR: plastic-labs/honcho#131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:918-942
Timestamp: 2025-06-19T14:32:02.934Z
Learning: The queue and active_queue_sessions tables in the migration script `migrations/versions/d429de0e5338_adopt_peer_paradigm.py` are expected to remain small, so per-row updates are acceptable and performance optimizations like set-based updates are not necessary.
Learnt from: dr-frmr
PR: plastic-labs/honcho#131
File: src/routers/sessions.py:206-213
Timestamp: 2025-06-18T20:42:06.458Z
Learning: The `get_or_create_session` function in this codebase is designed to handle both session creation and adding peers to existing sessions. When called with peers, it will add those peers to an existing session rather than creating a duplicate session.
Learnt from: CR
PR: plastic-labs/honcho#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:51:09.913Z
Learning: Applies to src/models.py : Feature flags on workspace, peer, and session levels
📚 Learning: the `get_or_create_session` function in this codebase is designed to handle both session creation an...
Learnt from: dr-frmr
PR: plastic-labs/honcho#131
File: src/routers/sessions.py:206-213
Timestamp: 2025-06-18T20:42:06.458Z
Learning: The `get_or_create_session` function in this codebase is designed to handle both session creation and adding peers to existing sessions. When called with peers, it will add those peers to an existing session rather than creating a duplicate session.

Applied to files:

  • src/crud/session.py
  • src/deriver/enqueue.py
📚 Learning: the queue and active_queue_sessions tables in the migration script `migrations/versions/d429de0e5338...
Learnt from: Rajat-Ahuja1997
PR: plastic-labs/honcho#131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:918-942
Timestamp: 2025-06-19T14:32:02.934Z
Learning: The queue and active_queue_sessions tables in the migration script `migrations/versions/d429de0e5338_adopt_peer_paradigm.py` are expected to remain small, so per-row updates are acceptable and performance optimizations like set-based updates are not necessary.

Applied to files:

  • src/crud/session.py
  • src/deriver/enqueue.py
📚 Learning: applies to src/models.py : feature flags on workspace, peer, and session levels...
Learnt from: CR
PR: plastic-labs/honcho#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:51:09.913Z
Learning: Applies to src/models.py : Feature flags on workspace, peer, and session levels

Applied to files:

  • src/crud/session.py
  • src/deriver/enqueue.py
🪛 GitHub Check: basedpyright
src/deriver/enqueue.py

[warning] 204-204:
Argument type is partially unknown
  Argument corresponds to parameter "default" in function "get"
  Argument type is "list[bool | dict[Unknown, Unknown]]" (reportUnknownArgumentType)


[warning] 204-204:
Type of "configuration" is partially unknown
  Type of "configuration" is "list[dict[str, Any]] | list[bool | dict[Unknown, Unknown]]" (reportUnknownVariableType)

🪛 GitHub Actions: Static Analysis
src/deriver/enqueue.py

[warning] 204-204: Type of "configuration" is partially unknown. Type is "list[dict[str, Any]] | list[bool | dict[Unknown, Unknown]]" (reportUnknownVariableType)


[warning] 204-204: Argument type is partially unknown. Argument corresponds to parameter "default" in function "get". Argument type is "list[bool | dict[Unknown, Unknown]]" (reportUnknownArgumentType)


[error] 206-206: Argument expression after ** must be a mapping with a "str" key type (reportCallIssue)

🔇 Additional comments (5)
src/crud/session.py (3)

413-413: Return type correctly updated to include activity status.

The function signature properly reflects the new return structure including the is_active boolean flag.


415-416: Clear documentation of behavior change.

The docstring clearly communicates that the function no longer filters for active peers and will return all peers including those who have left the session.


431-431: Correct implementation of activity status flag.

The is_active column correctly uses left_at.is_(None) to determine if a peer is currently in the session, which aligns with the soft delete pattern used throughout the codebase.

src/deriver/enqueue.py (2)

121-126: Dictionary structure correctly updated for new return format.

The dictionary comprehension properly incorporates the new is_active flag as the third element in the configuration list, maintaining consistency with the updated return structure from get_session_peer_configuration.


284-286: Excellent race condition fix for inactive observer peers.

This check correctly prevents enqueueing representation tasks for observer peers who are no longer active in the session, addressing the race condition mentioned in the PR objectives. This ensures that only active peers receive representation tasks.

Comment thread src/deriver/enqueue.py Outdated
Comment on lines +201 to +204
# If the sender is not in peers_with_configuration, they left after sending a message.
# We'll use the default behavior of observing the sender by instantiating the default
# peer-level and session-level configs.
configuration = peers_with_configuration.get(sender_name, [{}, {}, True])

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.

⚠️ Potential issue

Fix type safety issue with default value.

The static analysis warnings indicate type issues with the default value. The default should match the expected structure of [dict, dict, bool].

Apply this diff to fix the type safety issue:

-    # If the sender is not in peers_with_configuration, they left after sending a message.
-    # We'll use the default behavior of observing the sender by instantiating the default
-    # peer-level and session-level configs.
-    configuration = peers_with_configuration.get(sender_name, [{}, {}, True])
+    # If the sender is not in peers_with_configuration, they left after sending a message.
+    # We'll use the default behavior of observing the sender by instantiating the default
+    # peer-level and session-level configs.
+    configuration = peers_with_configuration.get(sender_name, [{}, {}, True])

Actually, the current implementation is logically correct. The issue is with the type annotation. Consider updating the function signature or adding a type cast to resolve the static analysis warnings.

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 GitHub Check: basedpyright

[warning] 204-204:
Argument type is partially unknown
  Argument corresponds to parameter "default" in function "get"
  Argument type is "list[bool | dict[Unknown, Unknown]]" (reportUnknownArgumentType)


[warning] 204-204:
Type of "configuration" is partially unknown
  Type of "configuration" is "list[dict[str, Any]] | list[bool | dict[Unknown, Unknown]]" (reportUnknownVariableType)

🪛 GitHub Actions: Static Analysis

[warning] 204-204: Type of "configuration" is partially unknown. Type is "list[dict[str, Any]] | list[bool | dict[Unknown, Unknown]]" (reportUnknownVariableType)


[warning] 204-204: Argument type is partially unknown. Argument corresponds to parameter "default" in function "get". Argument type is "list[bool | dict[Unknown, Unknown]]" (reportUnknownArgumentType)

🤖 Prompt for AI Agents
In src/deriver/enqueue.py around lines 201 to 204, the default value used in
peers_with_configuration.get has a type safety issue because its type does not
match the expected [dict, dict, bool] structure. To fix this, update the
function's type annotations to reflect that the returned value can be a tuple or
list of two dicts and a bool, or alternatively add an explicit type cast to the
default value to match the expected type and satisfy static analysis.

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 3

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 62d4e16 and ee7f277.

📒 Files selected for processing (2)
  • src/deriver/enqueue.py (3 hunks)
  • tests/integration/test_enqueue.py (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
tests/**/*.py

📄 CodeRabbit Inference Engine (CLAUDE.md)

Tests in pytest with fixtures in tests/conftest.py

Files:

  • tests/integration/test_enqueue.py
src/**/*.py

📄 CodeRabbit Inference Engine (CLAUDE.md)

src/**/*.py: Follow isort conventions with absolute imports preferred
snake_case for variables/functions; PascalCase for classes
Line length: 88 chars (Black compatible)
Explicit error handling with appropriate exception types
Docstrings: Use Google style docstrings
Use environment variables via python-dotenv (.env)
Use specific exception types (ResourceNotFoundException, ValidationException, etc.)
Proper logging with context instead of print statements

Files:

  • src/deriver/enqueue.py
🧠 Learnings (4)
📓 Common learnings
Learnt from: dr-frmr
PR: plastic-labs/honcho#131
File: src/routers/sessions.py:206-213
Timestamp: 2025-06-18T20:42:06.458Z
Learning: The `get_or_create_session` function in this codebase is designed to handle both session creation and adding peers to existing sessions. When called with peers, it will add those peers to an existing session rather than creating a duplicate session.
Learnt from: Rajat-Ahuja1997
PR: plastic-labs/honcho#131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:918-942
Timestamp: 2025-06-19T14:32:02.934Z
Learning: The queue and active_queue_sessions tables in the migration script `migrations/versions/d429de0e5338_adopt_peer_paradigm.py` are expected to remain small, so per-row updates are acceptable and performance optimizations like set-based updates are not necessary.
📚 Learning: the queue and active_queue_sessions tables in the migration script `migrations/versions/d429de0e5338...
Learnt from: Rajat-Ahuja1997
PR: plastic-labs/honcho#131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:918-942
Timestamp: 2025-06-19T14:32:02.934Z
Learning: The queue and active_queue_sessions tables in the migration script `migrations/versions/d429de0e5338_adopt_peer_paradigm.py` are expected to remain small, so per-row updates are acceptable and performance optimizations like set-based updates are not necessary.

Applied to files:

  • tests/integration/test_enqueue.py
  • src/deriver/enqueue.py
📚 Learning: the `get_or_create_session` function in this codebase is designed to handle both session creation an...
Learnt from: dr-frmr
PR: plastic-labs/honcho#131
File: src/routers/sessions.py:206-213
Timestamp: 2025-06-18T20:42:06.458Z
Learning: The `get_or_create_session` function in this codebase is designed to handle both session creation and adding peers to existing sessions. When called with peers, it will add those peers to an existing session rather than creating a duplicate session.

Applied to files:

  • tests/integration/test_enqueue.py
  • src/deriver/enqueue.py
📚 Learning: applies to src/models.py : feature flags on workspace, peer, and session levels...
Learnt from: CR
PR: plastic-labs/honcho#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T14:51:09.913Z
Learning: Applies to src/models.py : Feature flags on workspace, peer, and session levels

Applied to files:

  • src/deriver/enqueue.py
🪛 Ruff (0.12.2)
tests/integration/test_enqueue.py

549-549: Missing return type annotation for public function test_sender_left_session_after_message_sent

Add return type annotation: None

(ANN201)


562-562: Trailing comma missing

Add trailing comma

(COM812)


588-588: Trailing comma missing

Add trailing comma

(COM812)


611-611: Trailing comma missing

Add trailing comma

(COM812)


642-642: Missing return type annotation for public function test_observer_left_session_no_queue_items_generated

Add return type annotation: None

(ANN201)


655-655: Trailing comma missing

Add trailing comma

(COM812)


658-658: Trailing comma missing

Add trailing comma

(COM812)


670-670: Trailing comma missing

Add trailing comma

(COM812)


673-673: Trailing comma missing

Add trailing comma

(COM812)


689-689: Trailing comma missing

Add trailing comma

(COM812)


712-712: Trailing comma missing

Add trailing comma

(COM812)


724-724: Missing return type annotation for public function test_sender_not_in_peer_configuration_uses_defaults

Add return type annotation: None

(ANN201)


737-737: Trailing comma missing

Add trailing comma

(COM812)


773-773: Trailing comma missing

Add trailing comma

(COM812)


804-804: Missing return type annotation for public function test_mixed_active_inactive_peers_complex_scenario

Add return type annotation: None

(ANN201)


817-817: Trailing comma missing

Add trailing comma

(COM812)


820-820: Trailing comma missing

Add trailing comma

(COM812)


823-823: Trailing comma missing

Add trailing comma

(COM812)


826-826: Trailing comma missing

Add trailing comma

(COM812)


835-835: Trailing comma missing

Add trailing comma

(COM812)


846-846: Trailing comma missing

Add trailing comma

(COM812)


849-849: Trailing comma missing

Add trailing comma

(COM812)


852-852: Trailing comma missing

Add trailing comma

(COM812)


855-855: Trailing comma missing

Add trailing comma

(COM812)


872-872: Trailing comma missing

Add trailing comma

(COM812)


898-898: Trailing comma missing

Add trailing comma

(COM812)


937-937: Missing return type annotation for public function test_sender_missing_from_configuration_uses_default

Add return type annotation: None

(ANN201)


949-949: Missing return type annotation for public function test_sender_with_empty_configurations_uses_default

Add return type annotation: None

(ANN201)


955-955: Trailing comma missing

Add trailing comma

(COM812)


963-963: Missing return type annotation for public function test_sender_with_peer_config_observe_me_false

Add return type annotation: None

(ANN201)


968-968: Trailing comma missing

Add trailing comma

(COM812)


975-975: Missing return type annotation for public function test_session_config_overrides_peer_config

Add return type annotation: None

(ANN201)


983-983: Trailing comma missing

Add trailing comma

(COM812)


990-990: Missing return type annotation for public function test_session_config_none_falls_back_to_peer_config

Add return type annotation: None

(ANN201)


998-998: Trailing comma missing

Add trailing comma

(COM812)


1005-1005: Missing return type annotation for public function test_mixed_configurations_with_active_status

Add return type annotation: None

(ANN201)


1033-1033: Trailing comma missing

Add trailing comma

(COM812)


1048-1048: Missing return type annotation for public function create_sample_payload

(ANN201)


1069-1069: Missing return type annotation for public function count_queue_items

(ANN201)


1076-1076: Missing return type annotation for public function test_edge_case_all_peers_left_except_sender

Add return type annotation: None

(ANN201)


1089-1089: Trailing comma missing

Add trailing comma

(COM812)


1092-1092: Trailing comma missing

Add trailing comma

(COM812)


1120-1120: Trailing comma missing

Add trailing comma

(COM812)


1141-1141: Trailing comma missing

Add trailing comma

(COM812)


1152-1152: Missing return type annotation for public function test_edge_case_sender_and_observer_both_left_different_times

Add return type annotation: None

(ANN201)


1164-1164: Trailing comma missing

Add trailing comma

(COM812)


1193-1193: Trailing comma missing

Add trailing comma

(COM812)


1204-1204: Trailing comma missing

Add trailing comma

(COM812)


1227-1227: Trailing comma missing

Add trailing comma

(COM812)


1238-1238: Missing return type annotation for public function test_edge_case_message_from_never_joined_peer

Add return type annotation: None

(ANN201)


1250-1250: Trailing comma missing

Add trailing comma

(COM812)


1285-1285: Trailing comma missing

Add trailing comma

(COM812)

🔇 Additional comments (5)
src/deriver/enqueue.py (2)

121-127: LGTM! Properly includes active status for each peer.

The modification correctly adds the is_active flag as the third element in the configuration list, which enables downstream functions to filter out inactive peers.


284-287: LGTM! Correctly filters out inactive observers.

The implementation properly skips representation tasks for observers who have left the session by checking the is_active flag. This prevents unnecessary queue items from being created.

tests/integration/test_enqueue.py (3)

642-721: Excellent test coverage for inactive observer filtering.

This test thoroughly validates the core functionality of skipping representation tasks for observers who have left the session. The assertions correctly verify that only active observers receive queue items.


724-801: Well-designed test for missing sender configuration.

This test effectively validates the race condition handling when a sender's configuration is missing, ensuring the system gracefully defaults to observe_me=True. This covers an important edge case.


1043-1313: Excellent edge case test coverage.

These advanced tests thoroughly cover complex race conditions including:

  • All observers leaving the session
  • Sender and observer leaving at different times
  • Messages from peers who never joined the session

The test scenarios are well-designed and provide confidence in the robustness of the race condition handling.

Comment thread src/deriver/enqueue.py
Comment on lines +201 to +204
# If the sender is not in peers_with_configuration, they left after sending a message.
# We'll use the default behavior of observing the sender by instantiating the default
# peer-level and session-level configs.
configuration: list[Any] = peers_with_configuration.get(sender_name, [{}, {}])

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.

⚠️ Potential issue

Fix the default value to match the expected structure.

The default value should include the is_active flag as the third element to match the structure returned by get_peers_with_configuration. Without this, the function may fail when trying to access configuration[2] in other parts of the code.

Apply this diff to fix the issue:

-    configuration: list[Any] = peers_with_configuration.get(sender_name, [{}, {}])
+    configuration: list[Any] = peers_with_configuration.get(sender_name, [{}, {}, True])

The default should assume the sender is active (True) when they're missing from the configuration, as they were able to send a message.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# If the sender is not in peers_with_configuration, they left after sending a message.
# We'll use the default behavior of observing the sender by instantiating the default
# peer-level and session-level configs.
configuration: list[Any] = peers_with_configuration.get(sender_name, [{}, {}])
# If the sender is not in peers_with_configuration, they left after sending a message.
# We'll use the default behavior of observing the sender by instantiating the default
# peer-level and session-level configs.
configuration: list[Any] = peers_with_configuration.get(sender_name, [{}, {}, True])
🤖 Prompt for AI Agents
In src/deriver/enqueue.py around lines 201 to 204, the default value for the
configuration variable lacks the third element `is_active` flag, which causes
errors when accessing configuration[2] later. Update the default value in the
peers_with_configuration.get call to include an `is_active` flag set to True as
the third element, ensuring the default matches the expected structure returned
by get_peers_with_configuration.

Comment on lines +549 to +639
async def test_sender_left_session_after_message_sent(
self,
mock_tracked_db: AsyncMock,
db_session: AsyncSession,
sample_data: tuple[Workspace, Peer],
):
"""Test that messages from senders who left the session still get processed with default config"""
mock_tracked_db.return_value.__aenter__.return_value = db_session

test_workspace, sender_peer = sample_data

# Create an observer peer
observer_peer = models.Peer(
workspace_name=test_workspace.name, name=str(generate_nanoid())
)
db_session.add(observer_peer)

# Create session with both peers
test_session = await crud.get_or_create_session(
db_session,
schemas.SessionCreate(
name=str(generate_nanoid()),
peers={
sender_peer.name: schemas.SessionPeerConfig(observe_me=True),
observer_peer.name: schemas.SessionPeerConfig(observe_others=True),
},
),
test_workspace.name,
)
await db_session.commit()

# Simulate sender leaving the session by setting left_at
from datetime import datetime, timezone

session_peer_result = await db_session.execute(
select(models.SessionPeer).where(
models.SessionPeer.session_name == test_session.name,
models.SessionPeer.peer_name == sender_peer.name,
models.SessionPeer.workspace_name == test_workspace.name,
)
)
session_peer = session_peer_result.scalar_one()
session_peer.left_at = datetime.now(timezone.utc)
await db_session.commit()

# Create message payload from the peer who left
payload = self.create_sample_payload(
workspace_name=test_workspace.name,
session_name=test_session.name,
peer_name=sender_peer.name,
)

initial_count = await self.count_queue_items(db_session)
await enqueue(payload)
final_count = await self.count_queue_items(db_session)

# Should create 2 queue items:
# 1 representation for sender (using default config since they left)
# 1 representation for observer (still in session and observing others)
assert final_count - initial_count == 2

result = await db_session.execute(
select(QueueItem).where(QueueItem.session_id == test_session.id)
)
queue_items = result.scalars().all()

expected_payloads = [
{
"sender_name": sender_peer.name,
"target_name": sender_peer.name,
"task_type": "representation",
},
{
"sender_name": sender_peer.name,
"target_name": observer_peer.name,
"task_type": "representation",
},
]
actual_payloads = [
{
"sender_name": item.payload.get("sender_name"),
"target_name": item.payload.get("target_name"),
"task_type": item.payload.get("task_type"),
}
for item in queue_items
]

assert len(actual_payloads) == len(expected_payloads)
for expected in expected_payloads:
assert expected in actual_payloads

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.

🧹 Nitpick (assertive)

Add return type annotations to test methods.

The test logic is comprehensive and correctly validates the race condition scenario. However, for consistency with Python type hints, consider adding return type annotations.

Add return type annotations to all test methods:

-    async def test_sender_left_session_after_message_sent(
+    async def test_sender_left_session_after_message_sent(
         self,
         mock_tracked_db: AsyncMock,
         db_session: AsyncSession,
         sample_data: tuple[Workspace, Peer],
-    ):
+    ) -> None:

This applies to all the new test methods in this file.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async def test_sender_left_session_after_message_sent(
self,
mock_tracked_db: AsyncMock,
db_session: AsyncSession,
sample_data: tuple[Workspace, Peer],
):
"""Test that messages from senders who left the session still get processed with default config"""
mock_tracked_db.return_value.__aenter__.return_value = db_session
test_workspace, sender_peer = sample_data
# Create an observer peer
observer_peer = models.Peer(
workspace_name=test_workspace.name, name=str(generate_nanoid())
)
db_session.add(observer_peer)
# Create session with both peers
test_session = await crud.get_or_create_session(
db_session,
schemas.SessionCreate(
name=str(generate_nanoid()),
peers={
sender_peer.name: schemas.SessionPeerConfig(observe_me=True),
observer_peer.name: schemas.SessionPeerConfig(observe_others=True),
},
),
test_workspace.name,
)
await db_session.commit()
# Simulate sender leaving the session by setting left_at
from datetime import datetime, timezone
session_peer_result = await db_session.execute(
select(models.SessionPeer).where(
models.SessionPeer.session_name == test_session.name,
models.SessionPeer.peer_name == sender_peer.name,
models.SessionPeer.workspace_name == test_workspace.name,
)
)
session_peer = session_peer_result.scalar_one()
session_peer.left_at = datetime.now(timezone.utc)
await db_session.commit()
# Create message payload from the peer who left
payload = self.create_sample_payload(
workspace_name=test_workspace.name,
session_name=test_session.name,
peer_name=sender_peer.name,
)
initial_count = await self.count_queue_items(db_session)
await enqueue(payload)
final_count = await self.count_queue_items(db_session)
# Should create 2 queue items:
# 1 representation for sender (using default config since they left)
# 1 representation for observer (still in session and observing others)
assert final_count - initial_count == 2
result = await db_session.execute(
select(QueueItem).where(QueueItem.session_id == test_session.id)
)
queue_items = result.scalars().all()
expected_payloads = [
{
"sender_name": sender_peer.name,
"target_name": sender_peer.name,
"task_type": "representation",
},
{
"sender_name": sender_peer.name,
"target_name": observer_peer.name,
"task_type": "representation",
},
]
actual_payloads = [
{
"sender_name": item.payload.get("sender_name"),
"target_name": item.payload.get("target_name"),
"task_type": item.payload.get("task_type"),
}
for item in queue_items
]
assert len(actual_payloads) == len(expected_payloads)
for expected in expected_payloads:
assert expected in actual_payloads
async def test_sender_left_session_after_message_sent(
self,
mock_tracked_db: AsyncMock,
db_session: AsyncSession,
sample_data: tuple[Workspace, Peer],
) -> None:
"""Test that messages from senders who left the session still get processed with default config"""
mock_tracked_db.return_value.__aenter__.return_value = db_session
test_workspace, sender_peer = sample_data
# Create an observer peer
observer_peer = models.Peer(
workspace_name=test_workspace.name, name=str(generate_nanoid())
)
db_session.add(observer_peer)
# Create session with both peers
test_session = await crud.get_or_create_session(
db_session,
schemas.SessionCreate(
name=str(generate_nanoid()),
peers={
sender_peer.name: schemas.SessionPeerConfig(observe_me=True),
observer_peer.name: schemas.SessionPeerConfig(observe_others=True),
},
),
test_workspace.name,
)
await db_session.commit()
# Simulate sender leaving the session by setting left_at
from datetime import datetime, timezone
session_peer_result = await db_session.execute(
select(models.SessionPeer).where(
models.SessionPeer.session_name == test_session.name,
models.SessionPeer.peer_name == sender_peer.name,
models.SessionPeer.workspace_name == test_workspace.name,
)
)
session_peer = session_peer_result.scalar_one()
session_peer.left_at = datetime.now(timezone.utc)
await db_session.commit()
# Create message payload from the peer who left
payload = self.create_sample_payload(
workspace_name=test_workspace.name,
session_name=test_session.name,
peer_name=sender_peer.name,
)
initial_count = await self.count_queue_items(db_session)
await enqueue(payload)
final_count = await self.count_queue_items(db_session)
# Should create 2 queue items:
# 1 representation for sender (using default config since they left)
# 1 representation for observer (still in session and observing others)
assert final_count - initial_count == 2
result = await db_session.execute(
select(QueueItem).where(QueueItem.session_id == test_session.id)
)
queue_items = result.scalars().all()
expected_payloads = [
{
"sender_name": sender_peer.name,
"target_name": sender_peer.name,
"task_type": "representation",
},
{
"sender_name": sender_peer.name,
"target_name": observer_peer.name,
"task_type": "representation",
},
]
actual_payloads = [
{
"sender_name": item.payload.get("sender_name"),
"target_name": item.payload.get("target_name"),
"task_type": item.payload.get("task_type"),
}
for item in queue_items
]
assert len(actual_payloads) == len(expected_payloads)
for expected in expected_payloads:
assert expected in actual_payloads
🧰 Tools
🪛 Ruff (0.12.2)

549-549: Missing return type annotation for public function test_sender_left_session_after_message_sent

Add return type annotation: None

(ANN201)


562-562: Trailing comma missing

Add trailing comma

(COM812)


588-588: Trailing comma missing

Add trailing comma

(COM812)


611-611: Trailing comma missing

Add trailing comma

(COM812)

🤖 Prompt for AI Agents
In tests/integration/test_enqueue.py around lines 549 to 639, the test method
test_sender_left_session_after_message_sent lacks a return type annotation. Add
a return type annotation of '-> None' to this async test method and any other
new test methods in this file to maintain consistency with Python type hinting
conventions.

Comment on lines +934 to +1041
class TestGetEffectiveObserveMeFunction:
"""Unit tests for the get_effective_observe_me function specifically testing race condition handling"""

def test_sender_missing_from_configuration_uses_default(self):
"""Test that missing sender uses default observe_me=True"""
from src.deriver.enqueue import get_effective_observe_me

# Empty peer configuration dict simulates sender who left after sending message
peers_with_configuration: dict[str, list[dict[str, Any]]] = {}

result = get_effective_observe_me("missing_sender", peers_with_configuration)

# Should use default PeerConfig() which has observe_me=True
assert result is True

def test_sender_with_empty_configurations_uses_default(self):
"""Test that sender with empty peer and session configs uses default"""
from src.deriver.enqueue import get_effective_observe_me

# Sender present but with empty configurations
peers_with_configuration: dict[str, list[dict[str, Any]]] = {
"sender": [{}, {}] # Empty peer config, empty session config
}

result = get_effective_observe_me("sender", peers_with_configuration)

# Should use default PeerConfig() which has observe_me=True
assert result is True

def test_sender_with_peer_config_observe_me_false(self):
"""Test that peer config observe_me=False is respected"""
from src.deriver.enqueue import get_effective_observe_me

peers_with_configuration = {
"sender": [{"observe_me": False}, {}] # Peer config with observe_me=False
}

result = get_effective_observe_me("sender", peers_with_configuration)

assert result is False

def test_session_config_overrides_peer_config(self):
"""Test that session peer config takes precedence over peer config"""
from src.deriver.enqueue import get_effective_observe_me

peers_with_configuration = {
"sender": [
{"observe_me": True}, # Peer config says True
{"observe_me": False}, # Session config says False - should win
]
}

result = get_effective_observe_me("sender", peers_with_configuration)

assert result is False

def test_session_config_none_falls_back_to_peer_config(self):
"""Test that session config with None observe_me falls back to peer config"""
from src.deriver.enqueue import get_effective_observe_me

peers_with_configuration = {
"sender": [
{"observe_me": False}, # Peer config says False
{"observe_me": None}, # Session config is None - should fall back
]
}

result = get_effective_observe_me("sender", peers_with_configuration)

assert result is False

def test_mixed_configurations_with_active_status(self):
"""Test various configuration combinations with active status"""
from src.deriver.enqueue import get_effective_observe_me

# Test cases: (peer_config, session_config, expected_result)
test_cases: list[tuple[dict[str, Any] | None, dict[str, Any] | None, bool]] = [
# Default case - missing sender
(None, None, True),
# Empty configs
({}, {}, True),
# Peer config only
({"observe_me": False}, {}, False),
({"observe_me": True}, {}, True),
# Session config overrides
({"observe_me": True}, {"observe_me": False}, False),
({"observe_me": False}, {"observe_me": True}, True),
# Session config None falls back to peer
({"observe_me": False}, {"observe_me": None}, False),
({"observe_me": True}, {"observe_me": None}, True),
]

for i, (peer_config, session_config, expected) in enumerate(test_cases):
if peer_config is None:
# Test missing sender
peers_with_configuration = {}
sender_name = "missing_sender"
else:
peers_with_configuration = {
f"sender_{i}": [peer_config or {}, session_config or {}]
}
sender_name = f"sender_{i}"

result = get_effective_observe_me(sender_name, peers_with_configuration)
assert result == expected, (
f"Test case {i} failed: peer_config={peer_config}, session_config={session_config}, expected={expected}, got={result}"
)

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.

🧹 Nitpick (assertive)

Comprehensive unit test coverage for configuration handling.

The test cases thoroughly validate all edge cases for get_effective_observe_me, including missing senders, empty configurations, and precedence rules. The parametric test approach in test_mixed_configurations_with_active_status is particularly well-structured.

Consider extracting the test cases list into a class-level constant for better readability:

class TestGetEffectiveObserveMeFunction:
    TEST_CASES = [
        # (peer_config, session_config, expected_result)
        (None, None, True),  # Default case - missing sender
        ({}, {}, True),      # Empty configs
        # ... rest of test cases
    ]
🧰 Tools
🪛 Ruff (0.12.2)

937-937: Missing return type annotation for public function test_sender_missing_from_configuration_uses_default

Add return type annotation: None

(ANN201)


949-949: Missing return type annotation for public function test_sender_with_empty_configurations_uses_default

Add return type annotation: None

(ANN201)


955-955: Trailing comma missing

Add trailing comma

(COM812)


963-963: Missing return type annotation for public function test_sender_with_peer_config_observe_me_false

Add return type annotation: None

(ANN201)


968-968: Trailing comma missing

Add trailing comma

(COM812)


975-975: Missing return type annotation for public function test_session_config_overrides_peer_config

Add return type annotation: None

(ANN201)


983-983: Trailing comma missing

Add trailing comma

(COM812)


990-990: Missing return type annotation for public function test_session_config_none_falls_back_to_peer_config

Add return type annotation: None

(ANN201)


998-998: Trailing comma missing

Add trailing comma

(COM812)


1005-1005: Missing return type annotation for public function test_mixed_configurations_with_active_status

Add return type annotation: None

(ANN201)


1033-1033: Trailing comma missing

Add trailing comma

(COM812)

🤖 Prompt for AI Agents
In tests/integration/test_enqueue.py around lines 934 to 1041, the test cases
list inside the test_mixed_configurations_with_active_status method should be
extracted to a class-level constant for improved readability and
maintainability. Define a class attribute named TEST_CASES containing the list
of tuples representing the test cases, then update the test method to iterate
over this constant instead of the local variable.

@VVoruganti VVoruganti merged commit 23557ce into main Aug 5, 2025
11 checks passed
@VVoruganti VVoruganti deleted the ben/fix-get-effective-observe-me-race-condition branch August 5, 2025 17:44
dr-frmr added a commit that referenced this pull request Aug 6, 2025
* expose core client from sdks

* align text

* fix: resolve get_effective_observe me race condition, default peer config (#176)

* fix: resolve get_effective_observe me race condition, default peer config

* fix: preserve custom config even after leaving

* chore: test cases, enqueue types

* Update sdks/typescript/package.json

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

---------

Co-authored-by: doria <93405247+dr-frmr@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
VVoruganti pushed a commit that referenced this pull request Aug 6, 2025
* chore: fill out missing metadata inputs in python sdk

* feat: add get_peer_config to python sdk, thoroughly document ts sdk and remove bad client usage

* feat: zod
chore: update tests
chore: bump version, changelog

* chore: python sdk version bump and changelog

* [WIP] feat: combine search methods and rework endpoint to include limit param

* chore: test new stainless config with library

* nits: coderabbit

* Merge branch 'ben/sdk-improvements' into ben/search-rrf

* chore: pre-commit hooks cleanup

* feat: thoroughly document observation config

* Update sdks/python/src/honcho/peer.py

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* chore: v1.3.0

* feat: update version to 2.2.0 and enhance search functionality with arbitrary filters

- Remove unused config variables
- Added arbitrary filters to all search endpoints.
- Pluralize `filters` everywhere in SDKs for consistency
- Updated documentation and changelog to reflect these changes.

* expose core client in TS and Python SDKs (#150)

* expose core client from sdks

* align text

* fix: resolve get_effective_observe me race condition, default peer config (#176)

* fix: resolve get_effective_observe me race condition, default peer config

* fix: preserve custom config even after leaving

* chore: test cases, enqueue types

* Update sdks/typescript/package.json

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

---------

Co-authored-by: doria <93405247+dr-frmr@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* chore: formatting

* chore: revert undesired changes to v1 spec, clean up docs, coderabbit

* feat: better search docs, fix worker.ts

* fix: correctly make ts params optional in cases, update docs

* chore: coderabbit

* chore: remove spurious package-lock

* fix: asyncify examples, use limit properly in search

* fix(tests): handle 4 return values in test_get_session_peer_configuration

---------

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: Rajat Ahuja <rahuja445@gmail.com>
@coderabbitai coderabbitai Bot mentioned this pull request Aug 12, 2025
VVoruganti pushed a commit that referenced this pull request Aug 12, 2025
* chore: fill out missing metadata inputs in python sdk

* feat: add get_peer_config to python sdk, thoroughly document ts sdk and remove bad client usage

* feat: zod
chore: update tests
chore: bump version, changelog

* chore: python sdk version bump and changelog

* [WIP] feat: combine search methods and rework endpoint to include limit param

* chore: test new stainless config with library

* nits: coderabbit

* Merge branch 'ben/sdk-improvements' into ben/search-rrf

* chore: pre-commit hooks cleanup

* feat: thoroughly document observation config

* Update sdks/python/src/honcho/peer.py

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* chore: v1.3.0

* feat: update version to 2.2.0 and enhance search functionality with arbitrary filters

- Remove unused config variables
- Added arbitrary filters to all search endpoints.
- Pluralize `filters` everywhere in SDKs for consistency
- Updated documentation and changelog to reflect these changes.

* expose core client in TS and Python SDKs (#150)

* expose core client from sdks

* align text

* fix: resolve get_effective_observe me race condition, default peer config (#176)

* fix: resolve get_effective_observe me race condition, default peer config

* fix: preserve custom config even after leaving

* chore: test cases, enqueue types

* Update sdks/typescript/package.json

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

---------

Co-authored-by: doria <93405247+dr-frmr@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* chore: formatting

* chore: revert undesired changes to v1 spec, clean up docs, coderabbit

* feat: better search docs, fix worker.ts

* fix: correctly make ts params optional in cases, update docs

* chore: coderabbit

* chore: remove spurious package-lock

* feat: [WIP] introduce peer cards

* chore: remove search.mdx

* chore: clean up deriver

* feat: peer cards working in deriver

* feat: add basic peer_card_bench

* feat: refine peer card prompt, add mini-benchmark, switch to gpt-5-nano

* refactor: update peer card handling in dialectic functions and improve error handling

- Enhanced `get_peer_card` function to handle `ResourceNotFoundException`.
- Updated `dialectic_call` and `dialectic_stream` to accept `peer_card` and `target_peer_card` parameters.
- Modified prompt generation to include peer card information.
- Cleaned up whitespace in several files for consistency.

* chore: update mirascope dependency version in configuration files

- Bumped mirascope version from 1.25.1 to 1.25.5 in pyproject.toml and uv.lock.
- Added a note in config.py regarding peer card output token handling.
- Removed unnecessary comments in clients.py for clarity.

* fix: [coderabbit] improve error handling in set_peer_card and enhance logging

- Added a check in `set_peer_card` to raise `ResourceNotFoundException` if the peer does not exist.
- Updated logging in `CertaintyReasoner` to capture exceptions with Sentry when enabled.
- Refined logging messages for clarity and consistency across various functions.
- Cleaned up whitespace and formatting in several files for improved readability.

* refactor: update working representation handling and improve metadata key usage

- Introduced constants for representation collection names to enhance clarity and maintainability.
- Updated function signatures in `get_working_representation` and `set_working_representation` to require `session_name`.
- Simplified metadata key determination logic by using constants instead of hardcoded strings.
- Removed legacy fallback logic for working representation data retrieval.
- Refactored `save_working_representation_to_peer` to utilize the new `set_working_representation` function for improved code reuse.

* chore: update configuration files and enhance working representation settings

- Added new peer card settings and context token limits to `.env.template`, `config.toml.example`, and documentation.
- Introduced `WORKING_REPRESENTATION_MAX_OBSERVATIONS` to `DeriverSettings` for better control over observation storage.
- Updated `set_working_representation` to merge new observations while respecting the maximum limit.
- Improved docstrings for clarity and consistency across functions.

* feat: introduce LLMError exception and enhance error handling in deriver

- Added LLMError exception to handle failures in LLM calls, normalizing inputs into a JSON-serializable format.
- Updated CertaintyReasoner to raise LLMError on exceptions during LLM function calls.
- Enhanced QueueManager to log LLMError occurrences and re-queue messages appropriately.
- Modified test runner to support asynchronous operations and improved output formatting for test results.
- Updated test cases to include session information for better context.

* feat: add __repr__ method to QueueItem for improved string representation

- Implemented a __repr__ method in the QueueItem class to provide a clear and informative string representation of its attributes.
- Updated timeout handling in TestRunner to default to 10000.0 seconds when timeout_seconds is not set, enhancing robustness in polling operations.

* refactor: update peer card data structure and improve handling in related functions

- Changed return type of `get_peer_card` and `set_peer_card` to use `list[str]` instead of `str | None`.
- Updated `peer_card_call` and related functions to accommodate the new list structure for peer cards.
- Introduced `PeerCardQuery` model to standardize responses from peer card queries.
- Adjusted prompt generation in `peer_card_prompt` to reflect the new data structure.
- Modified benchmark tests to align with the updated peer card handling.

* refactor: adjust peer card output token settings and update related functions

- Increased `PEER_CARD_MAX_OUTPUT_TOKENS` from 2000 to 4000 in `DeriverSettings`.
- Updated `critical_analysis_call` to use `json_mode` and removed unused parameters.
- Modified benchmark tests to utilize the new `PEER_CARD_MAX_OUTPUT_TOKENS` setting.
- Removed obsolete `add_dislike.json` test file.

* refactor: update peer card handling in critical analysis and dialectic prompts

- Changed `peer_card` parameter type from `str | None` to `list[str] | None` in `critical_analysis_call` and related functions.
- Simplified error handling in `process_representation_task` by removing redundant try-except block.
- Updated prompt generation in `critical_analysis_prompt` and `dialectic_prompt` to format `peer_card` as a string with newlines.
- Adjusted benchmark tests to reflect changes in peer card structure and output formatting.

* refactor: update peer card test cases to use list structure

- Modified test cases in `test_representation_crud.py` to reflect the change in `peer_card` parameter type from `str` to `list[str]`.
- Updated assertions to accommodate the new list format for setting and retrieving peer cards.
- Ensured that tests for missing peers correctly handle the list input format.

* fix: improve formatting of peer card output in prompts

- Updated `peer_card_prompt` to join `old_peer_card` list elements with newlines for better readability.
- Removed outdated comment in `dialectic_prompt` regarding handling of non-existent cards.

* chore: [coderabbit] enhance docstring and logging in prompts and queue manager

- Updated the docstring in `critical_analysis_prompt` to provide detailed type annotations for parameters.
- Improved logging in `chat` to differentiate between single and multiple retrieved peer cards.
- Adjusted logging format in `QueueManager` to use a more structured approach for shutdown messages.

---------

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: Rajat Ahuja <rahuja445@gmail.com>
ranc1 pushed a commit to ranc1/honcho that referenced this pull request Apr 16, 2026
…nfig (plastic-labs#176)

* fix: resolve get_effective_observe me race condition, default peer config

* fix: preserve custom config even after leaving

* chore: test cases, enqueue types
ranc1 pushed a commit to ranc1/honcho that referenced this pull request Apr 16, 2026
* chore: fill out missing metadata inputs in python sdk

* feat: add get_peer_config to python sdk, thoroughly document ts sdk and remove bad client usage

* feat: zod
chore: update tests
chore: bump version, changelog

* chore: python sdk version bump and changelog

* [WIP] feat: combine search methods and rework endpoint to include limit param

* chore: test new stainless config with library

* nits: coderabbit

* Merge branch 'ben/sdk-improvements' into ben/search-rrf

* chore: pre-commit hooks cleanup

* feat: thoroughly document observation config

* Update sdks/python/src/honcho/peer.py

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* chore: v1.3.0

* feat: update version to 2.2.0 and enhance search functionality with arbitrary filters

- Remove unused config variables
- Added arbitrary filters to all search endpoints.
- Pluralize `filters` everywhere in SDKs for consistency
- Updated documentation and changelog to reflect these changes.

* expose core client in TS and Python SDKs (plastic-labs#150)

* expose core client from sdks

* align text

* fix: resolve get_effective_observe me race condition, default peer config (plastic-labs#176)

* fix: resolve get_effective_observe me race condition, default peer config

* fix: preserve custom config even after leaving

* chore: test cases, enqueue types

* Update sdks/typescript/package.json

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

---------

Co-authored-by: doria <93405247+dr-frmr@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* chore: formatting

* chore: revert undesired changes to v1 spec, clean up docs, coderabbit

* feat: better search docs, fix worker.ts

* fix: correctly make ts params optional in cases, update docs

* chore: coderabbit

* chore: remove spurious package-lock

* fix: asyncify examples, use limit properly in search

* fix(tests): handle 4 return values in test_get_session_peer_configuration

---------

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: Rajat Ahuja <rahuja445@gmail.com>
ranc1 pushed a commit to ranc1/honcho that referenced this pull request Apr 16, 2026
* chore: fill out missing metadata inputs in python sdk

* feat: add get_peer_config to python sdk, thoroughly document ts sdk and remove bad client usage

* feat: zod
chore: update tests
chore: bump version, changelog

* chore: python sdk version bump and changelog

* [WIP] feat: combine search methods and rework endpoint to include limit param

* chore: test new stainless config with library

* nits: coderabbit

* Merge branch 'ben/sdk-improvements' into ben/search-rrf

* chore: pre-commit hooks cleanup

* feat: thoroughly document observation config

* Update sdks/python/src/honcho/peer.py

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* chore: v1.3.0

* feat: update version to 2.2.0 and enhance search functionality with arbitrary filters

- Remove unused config variables
- Added arbitrary filters to all search endpoints.
- Pluralize `filters` everywhere in SDKs for consistency
- Updated documentation and changelog to reflect these changes.

* expose core client in TS and Python SDKs (plastic-labs#150)

* expose core client from sdks

* align text

* fix: resolve get_effective_observe me race condition, default peer config (plastic-labs#176)

* fix: resolve get_effective_observe me race condition, default peer config

* fix: preserve custom config even after leaving

* chore: test cases, enqueue types

* Update sdks/typescript/package.json

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

---------

Co-authored-by: doria <93405247+dr-frmr@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* chore: formatting

* chore: revert undesired changes to v1 spec, clean up docs, coderabbit

* feat: better search docs, fix worker.ts

* fix: correctly make ts params optional in cases, update docs

* chore: coderabbit

* chore: remove spurious package-lock

* feat: [WIP] introduce peer cards

* chore: remove search.mdx

* chore: clean up deriver

* feat: peer cards working in deriver

* feat: add basic peer_card_bench

* feat: refine peer card prompt, add mini-benchmark, switch to gpt-5-nano

* refactor: update peer card handling in dialectic functions and improve error handling

- Enhanced `get_peer_card` function to handle `ResourceNotFoundException`.
- Updated `dialectic_call` and `dialectic_stream` to accept `peer_card` and `target_peer_card` parameters.
- Modified prompt generation to include peer card information.
- Cleaned up whitespace in several files for consistency.

* chore: update mirascope dependency version in configuration files

- Bumped mirascope version from 1.25.1 to 1.25.5 in pyproject.toml and uv.lock.
- Added a note in config.py regarding peer card output token handling.
- Removed unnecessary comments in clients.py for clarity.

* fix: [coderabbit] improve error handling in set_peer_card and enhance logging

- Added a check in `set_peer_card` to raise `ResourceNotFoundException` if the peer does not exist.
- Updated logging in `CertaintyReasoner` to capture exceptions with Sentry when enabled.
- Refined logging messages for clarity and consistency across various functions.
- Cleaned up whitespace and formatting in several files for improved readability.

* refactor: update working representation handling and improve metadata key usage

- Introduced constants for representation collection names to enhance clarity and maintainability.
- Updated function signatures in `get_working_representation` and `set_working_representation` to require `session_name`.
- Simplified metadata key determination logic by using constants instead of hardcoded strings.
- Removed legacy fallback logic for working representation data retrieval.
- Refactored `save_working_representation_to_peer` to utilize the new `set_working_representation` function for improved code reuse.

* chore: update configuration files and enhance working representation settings

- Added new peer card settings and context token limits to `.env.template`, `config.toml.example`, and documentation.
- Introduced `WORKING_REPRESENTATION_MAX_OBSERVATIONS` to `DeriverSettings` for better control over observation storage.
- Updated `set_working_representation` to merge new observations while respecting the maximum limit.
- Improved docstrings for clarity and consistency across functions.

* feat: introduce LLMError exception and enhance error handling in deriver

- Added LLMError exception to handle failures in LLM calls, normalizing inputs into a JSON-serializable format.
- Updated CertaintyReasoner to raise LLMError on exceptions during LLM function calls.
- Enhanced QueueManager to log LLMError occurrences and re-queue messages appropriately.
- Modified test runner to support asynchronous operations and improved output formatting for test results.
- Updated test cases to include session information for better context.

* feat: add __repr__ method to QueueItem for improved string representation

- Implemented a __repr__ method in the QueueItem class to provide a clear and informative string representation of its attributes.
- Updated timeout handling in TestRunner to default to 10000.0 seconds when timeout_seconds is not set, enhancing robustness in polling operations.

* refactor: update peer card data structure and improve handling in related functions

- Changed return type of `get_peer_card` and `set_peer_card` to use `list[str]` instead of `str | None`.
- Updated `peer_card_call` and related functions to accommodate the new list structure for peer cards.
- Introduced `PeerCardQuery` model to standardize responses from peer card queries.
- Adjusted prompt generation in `peer_card_prompt` to reflect the new data structure.
- Modified benchmark tests to align with the updated peer card handling.

* refactor: adjust peer card output token settings and update related functions

- Increased `PEER_CARD_MAX_OUTPUT_TOKENS` from 2000 to 4000 in `DeriverSettings`.
- Updated `critical_analysis_call` to use `json_mode` and removed unused parameters.
- Modified benchmark tests to utilize the new `PEER_CARD_MAX_OUTPUT_TOKENS` setting.
- Removed obsolete `add_dislike.json` test file.

* refactor: update peer card handling in critical analysis and dialectic prompts

- Changed `peer_card` parameter type from `str | None` to `list[str] | None` in `critical_analysis_call` and related functions.
- Simplified error handling in `process_representation_task` by removing redundant try-except block.
- Updated prompt generation in `critical_analysis_prompt` and `dialectic_prompt` to format `peer_card` as a string with newlines.
- Adjusted benchmark tests to reflect changes in peer card structure and output formatting.

* refactor: update peer card test cases to use list structure

- Modified test cases in `test_representation_crud.py` to reflect the change in `peer_card` parameter type from `str` to `list[str]`.
- Updated assertions to accommodate the new list format for setting and retrieving peer cards.
- Ensured that tests for missing peers correctly handle the list input format.

* fix: improve formatting of peer card output in prompts

- Updated `peer_card_prompt` to join `old_peer_card` list elements with newlines for better readability.
- Removed outdated comment in `dialectic_prompt` regarding handling of non-existent cards.

* chore: [coderabbit] enhance docstring and logging in prompts and queue manager

- Updated the docstring in `critical_analysis_prompt` to provide detailed type annotations for parameters.
- Improved logging in `chat` to differentiate between single and multiple retrieved peer cards.
- Adjusted logging format in `QueueManager` to use a more structured approach for shutdown messages.

---------

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: Rajat Ahuja <rahuja445@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants