diff --git a/.claude/skills/csharp-sync/CONVENTIONS.md b/.claude/skills/csharp-sync/CONVENTIONS.md index 6c7cc5a8..1cd609a0 100644 --- a/.claude/skills/csharp-sync/CONVENTIONS.md +++ b/.claude/skills/csharp-sync/CONVENTIONS.md @@ -1,5 +1,7 @@ # Naming & Code Conventions +See CLAUDE.md for base code style, JSON conventions, and common gotchas. This file covers only migration-specific conventions beyond the baseline. + ## Naming map | C# | Python | @@ -7,14 +9,27 @@ | `PascalCase` class | `PascalCase` class | | `PascalCase` method | `snake_case` method | | `PascalCase` property | `snake_case` attribute | -| JSON keys in `ToJson()` | `"PascalCase"` in `to_json()` | -| JSON keys in `FromJson()` | `"PascalCase"` in `from_json()` | | `IOperation` | `IOperation[T]` | | `IMaintenanceOperation` | `IMaintenanceOperation` | | `IVoidMaintenanceOperation` | `VoidMaintenanceOperation` | | `null` | `None` | | `TimeSpan.FromDays(n)` | `timedelta(days=n)` | -| `DateTime.UtcNow` | `datetime.datetime.now(datetime.timezone.utc)` — NEVER `utcnow()` | + +## C# → Python pattern map + +These C# patterns have no 1:1 equivalent — use the Python strategy shown: + +| C# Pattern | Python Strategy | +|-------------|----------------| +| Method overloading | Optional params with `None` defaults + `if` guards | +| `async/await` | **Synchronous** — this codebase is sync-only; use `concurrent.futures.ThreadPoolExecutor` where needed | +| `IDisposable` | `__enter__`/`__exit__` context managers | +| `event Action` | `List[Callable[[EventArgs], None]]` with `add_`/`remove_` registration methods | +| `lock(obj)` | `with self._lock:` using `threading.Lock`, `RLock`, or `Semaphore` | +| Fluent builder API | Methods returning `self` for chaining | +| `Span`, `ReadOnlySpan` | Skip — no Python equivalent | +| `Generic` with constraints | `TypeVar("T", bound=Base)` | +| Lazy initialization | Double-check locking with `threading.RLock` | ## Operation class pattern @@ -87,12 +102,6 @@ class MyEnum(enum.Enum): VALUE_TWO = "ValueTwo" ``` -## Import style - -- Absolute imports from `ravendb.*`. -- Group: stdlib → third-party → project, separated by blank lines. -- Use `TYPE_CHECKING` guard for circular-import-prone types. - ## Verifying against C# source When unsure about a field name, method signature, or serialization key, fetch the original C# file: diff --git a/.claude/skills/csharp-sync/SKILL.md b/.claude/skills/csharp-sync/SKILL.md index 8ad149a7..c0025459 100644 --- a/.claude/skills/csharp-sync/SKILL.md +++ b/.claude/skills/csharp-sync/SKILL.md @@ -1,6 +1,11 @@ --- name: csharp-sync description: Migrates RavenDB C# client features to the Python client. Use when porting C# diffs, patches, or test files to Python, batching sync work, or writing migration tests. +allowed-tools: "Read Edit Write Grep Glob Bash WebFetch Agent" +argument-hint: "" +effort: max +model: opus +disable-model-invocation: true --- # C# → Python Client Sync @@ -8,11 +13,22 @@ description: Migrates RavenDB C# client features to the Python client. Use when Migrate features from the RavenDB C# client into this Python client codebase. Input is a raw `.patch` / `.diff` from the C# repo. Output is working Python code with tests. +## Current state +- Branch: !`git branch --show-current` +- Recent syncs: !`git log --oneline -5 --grep="RDBC"` + +## Input + +If arguments were provided (`$ARGUMENTS`), use them as the starting point: +- If it's a file path, read it as the diff/patch input +- If it's a feature name, fetch the relevant C# source to begin triage + ## Pipeline 1. **Triage** — read the diff, list every changed file, group into batches -2. **Implement** — port each batch following [CONVENTIONS.md](CONVENTIONS.md) -3. **Test** — migrate or write tests following [TESTING.md](TESTING.md) +2. **Plan** — present the batch plan to the user and wait for approval before writing any code +3. **Implement** — port each batch following [CONVENTIONS.md](CONVENTIONS.md) +4. **Test** — migrate or write tests following [TESTING.md](TESTING.md) ## Reference sources @@ -67,31 +83,82 @@ Use docs to determine whether a C# change is: 2. For unfamiliar changes, **fetch the full C# source** from GitHub to understand context. 3. **Check RavenDB docs** to confirm the feature is documented and understand its scope. 4. **Filter out** changes that are C#-specific or relate to features not yet in the Python client. -5. Group remaining changes into **batches** by feature area. Good boundaries: +5. **Search for existing Python equivalents** — for each changed C# file, Grep for the class name, operation name, or endpoint path in the Python codebase: + - If a Python version exists, compare it with the C# version to identify only the delta to port. + - If the Python version has intentional divergences (different architecture, Pythonic patterns), preserve them. +6. Group remaining changes into **batches** by feature area. Good boundaries: - New operation class (e.g. `ConfigureRemoteAttachmentsOperation`) - New model / DTO cluster (e.g. settings + configuration classes) - New exception type + dispatcher wiring - Test migration for an already-implemented feature -6. Create a **checklist** — one `- [ ]` per batch, ordered by dependency (models → operations → tests). +7. Create a **checklist** — one `- [ ]` per batch, ordered by dependency (models → operations → tests). + Use TodoWrite to create tasks for each batch. Mark each as completed before moving to the next. ### Batch sizing - Target **≤ 300 lines of Python** per batch. - If a single C# file maps to > 300 lines, split by class or logical section. - Tests are their own batch, listed after the implementation batch they cover. +- If a batch modifies existing models/DTOs that have downstream consumers, include the consumer updates in the same batch — even if it exceeds the 300-line target. A broken intermediate state is worse than a large batch. + +## Step 2 — Present the plan for approval + +Before writing any code, present the full migration plan to the user. The plan should include: -## Step 2 — Implement each batch +1. **Summary** — what the diff covers and what will be ported vs. skipped. +2. **Batch list** — numbered batches with: + - Batch name / feature area + - Which C# files map to which Python files (new or existing) + - Estimated scope (new file, modify existing, add fields, etc.) + - Any risks or ambiguities flagged during triage +3. **Skipped items** — C# changes filtered out and why (C#-specific, dependency not ported, etc.) +4. **Open questions** — anything that needs the user's decision before implementation. + +**Wait for the user to approve the plan before proceeding to Step 3.** The user may reorder batches, split/merge them, or ask to skip certain items. Adjust the TodoWrite tasks accordingly. + +## Step 3 — Implement each batch + +Only proceed after user approval from Step 2. Follow the naming rules and patterns in [CONVENTIONS.md](CONVENTIONS.md). +### Porting principle + +Port the *behavior*, not the *syntax*. If the Python client already handles something in a Pythonic way (context managers, generators, keyword arguments), preserve that pattern even if the C# implementation looks different. The goal is feature parity, not code parity. + When porting a C# class, **always fetch the full source from GitHub** — diffs alone often lack constructor signatures, base classes, or `ToJson`/`FromJson` methods needed for a correct port. For every edit: -1. Use `codebase-retrieval` to find ALL downstream callers, implementations, and tests. +1. Use Grep and Glob to find ALL downstream callers, implementations, and tests. 2. Update every affected file — missing a downstream change is a critical failure. 3. After editing, verify imports resolve and no existing tests are broken. 4. If unsure about a field or method, check the **RavenDB docs** for the canonical behavior. +### When unsure + +1. If the C# pattern has no direct Python equivalent → ask the user. +2. If the feature partially exists → check `git log` for prior sync commits to understand how similar cases were handled. +3. If a dependency isn't ported yet → skip and note it as a prerequisite in the batch checklist. + +### Session changes require extra caution + +The session subsystem (`ravendb/documents/session/`) spans 25+ modules and ~13K lines. Before porting session-related C# changes: +1. Read the target Python module thoroughly — session modules are tightly coupled. +2. Trace the full call chain from session API → command generation → request execution. +3. Session batches should be smaller (≤150 lines) due to higher coupling risk. +4. Always run the full session test suite after changes, not just new tests. + +### Streaming operations + +Streaming operations (bulk insert, subscriptions, query streaming) use generators and `concurrent.futures` — NOT the standard operation pattern. Fetch the full Python source for the existing streaming infrastructure before porting. + +### Verification gate + +After completing all batches, run: +1. `black --check .` — fix any formatting issues. +2. `python -m unittest -v` — all new tests must pass. +3. `python -m unittest discover` on affected test directories — confirm no regressions. + ### Key files to touch per feature | What | Where | @@ -106,8 +173,12 @@ For every edit: | Bulk insert changes | `ravendb/documents/bulk_insert_operation.py` | | Module exports | `ravendb/__init__.py` | -## Step 3 — Write tests +## Step 4 — Write tests Follow the infrastructure and patterns in [TESTING.md](TESTING.md). +## If a batch fails +- If tests fail due to a bug in the port → fix in the same batch. +- If the feature can't be ported (missing dependency, server version mismatch) → revert the batch, add a `# todo: requires ` comment, and continue with remaining batches. +- Never leave broken code committed — each batch must be independently valid. diff --git a/.claude/skills/csharp-sync/TESTING.md b/.claude/skills/csharp-sync/TESTING.md index 62651b30..b9925566 100644 --- a/.claude/skills/csharp-sync/TESTING.md +++ b/.claude/skills/csharp-sync/TESTING.md @@ -1,10 +1,12 @@ # Testing Conventions +See CLAUDE.md for base testing infrastructure and common gotchas. This file covers only migration-specific testing patterns. + ## Infrastructure - All tests extend `TestBase` from `ravendb/tests/test_base.py`. - `self.store` is pre-configured with a fresh database per test — no manual setup needed. -- Run tests: `.venv1\Scripts\python.exe -m unittest -v` +- Run tests: `python -m unittest -v` ## Assertion helpers @@ -60,11 +62,7 @@ class _IndexResult: Then query with `select_fields(_IndexResult, "Id", "Errors")`. -## Common gotchas +## Migration-specific testing notes -- **Datetime**: always `datetime.datetime.now(datetime.timezone.utc)`, never `utcnow()`. -- **Empty vs None**: server may return `[]` instead of `null` for empty collections — use `assertFalse`/`assertTrue` not `assertIsNone`/`assertIsNotNone`. -- **Exception messages**: the dispatcher wraps the full server error (message + stack trace) into the exception string — always use `assertRaisesWithMessageContaining` with a meaningful substring. -- **`RavenException.__init__`**: stores message as plain string in `args[0]`, not `(message, cause)` tuple. - **Async operations**: use `store.maintenance.send_async(op)` → `op.wait_for_completion()` → `op.fetch_operations_status()["Result"]`. diff --git a/ravendb/documents/ai/ai_conversation.py b/ravendb/documents/ai/ai_conversation.py index 210754e6..247fe7a9 100644 --- a/ravendb/documents/ai/ai_conversation.py +++ b/ravendb/documents/ai/ai_conversation.py @@ -12,6 +12,7 @@ AiAgentActionResponse, AiConversationCreationOptions, ) +from ravendb.exceptions.exceptions import InvalidOperationException from ravendb.documents.operations.ai.agents.run_conversation_operation import AiAgentArtificialActionResponse if TYPE_CHECKING: @@ -56,7 +57,7 @@ def __init__( self._change_vector = change_vector self._prompt_parts: List[ContentPart] = [] - self._action_responses: List[AiAgentActionResponse] = [] + self._action_responses: Dict[str, AiAgentActionResponse] = {} self._artificial_actions: List[AiAgentArtificialActionResponse] = [] self._action_requests: Optional[List[AiAgentActionRequest]] = None @@ -114,15 +115,25 @@ def add_action_response(self, action_id: str, action_response: str) -> None: Args: action_id: The ID of the action to respond to action_response: The response content + + Raises: + InvalidOperationException: If a response for the given tool-id was already added """ from ravendb.documents.operations.ai.agents import AiAgentActionResponse + if action_id in self._action_responses: + raise InvalidOperationException( + f"An action response for tool-id '{action_id}' was already added. " + f"Each tool call must have exactly one response. " + f"If you're using handle, return the value from the handler (don't call add_action_response manually)." + ) + response = AiAgentActionResponse(tool_id=action_id) if isinstance(action_response, str): response.content = action_response - self._action_responses.append(response) + self._action_responses[action_id] = response def add_artificial_action_with_response(self, tool_id: str, action_response) -> None: """ @@ -211,7 +222,7 @@ def _run_internal( agent_id=self._agent_id, conversation_id=self._conversation_id, prompt_parts=self._prompt_parts, # Always send list, even if empty - action_responses=self._action_responses, # Always send list, even if empty + action_responses=list(self._action_responses.values()), # Always send list, even if empty artificial_actions=self._artificial_actions, # Always send list, even if empty options=self._options, change_vector=self._change_vector, diff --git a/ravendb/documents/operations/ai/gen_ai_configuration.py b/ravendb/documents/operations/ai/gen_ai_configuration.py index bff5dac7..7e602334 100644 --- a/ravendb/documents/operations/ai/gen_ai_configuration.py +++ b/ravendb/documents/operations/ai/gen_ai_configuration.py @@ -33,6 +33,7 @@ def __init__( queries: List[AiAgentToolQuery] = None, enable_tracing: bool = False, expiration_in_sec: int = None, + version: int = None, disabled: bool = False, mentor_node: str = None, pin_to_mentor_node: bool = False, @@ -60,6 +61,7 @@ def __init__( self.queries: List[AiAgentToolQuery] = queries or [] self.enable_tracing = enable_tracing self.expiration_in_sec: Optional[int] = expiration_in_sec + self.version: Optional[int] = version self._transforms: Optional[List[Transformation]] = None @@ -179,6 +181,8 @@ def to_json(self) -> Dict[str, Any]: "ExpirationInSec": self.expiration_in_sec, } ) + if self.version is not None: + result["Version"] = self.version return result @classmethod @@ -201,6 +205,7 @@ def from_json(cls, json_dict: Dict[str, Any]) -> "GenAiConfiguration": queries=[AiAgentToolQuery.from_json(q) for q in queries_data] if queries_data else None, enable_tracing=json_dict.get("EnableTracing", False), expiration_in_sec=json_dict.get("ExpirationInSec"), + version=json_dict.get("Version"), disabled=json_dict.get("Disabled", False), mentor_node=json_dict.get("MentorNode"), pin_to_mentor_node=json_dict.get("PinToMentorNode", False), diff --git a/ravendb/tests/ai_agent_tests/test_ai_agents_conversation_mock.py b/ravendb/tests/ai_agent_tests/test_ai_agents_conversation_mock.py index 13466c8e..d15fc869 100644 --- a/ravendb/tests/ai_agent_tests/test_ai_agents_conversation_mock.py +++ b/ravendb/tests/ai_agent_tests/test_ai_agents_conversation_mock.py @@ -438,6 +438,47 @@ def test_duplicate_action_handler_raises(self): with self.assertRaises(ValueError): chat.handle("store-result", lambda args: None, AiHandleErrorStrategy.SEND_ERRORS_TO_MODEL) + def test_action_responses_cleared_between_turns(self): + """After a server round-trip, action_responses are cleared so the same tool-id can be reused.""" + responses = iter( + [ + _action_result("store-result", "tool-x", {"result": "a"}), + _action_result("store-result", "tool-x", {"result": "b"}), + _done_result(response={"answer": "ok"}), + ] + ) + + with patch.object(self.store.maintenance, "send", side_effect=self._patched_send(lambda: next(responses))): + chat = self.store.ai.conversation(AGENT_ID, "conversations/") + chat.set_user_prompt("Multi-turn with same tool-id") + chat.handle("store-result", lambda args: "handled", AiHandleErrorStrategy.SEND_ERRORS_TO_MODEL) + result = chat.run() + + self.assertEqual(AiConversationStatus.DONE, result.status) + + +class TestAiConversationActionResponseValidation(unittest.TestCase): + """Pure client-side tests — no server or license required.""" + + def test_duplicate_action_response_raises(self): + """Adding two action responses for the same tool-id must raise.""" + from ravendb.documents.ai.ai_conversation import AiConversation + from ravendb.exceptions.exceptions import InvalidOperationException + + chat = AiConversation(store=None, agent_id="dummy") + chat.add_action_response("tool-1", "first response") + with self.assertRaises(InvalidOperationException): + chat.add_action_response("tool-1", "second response") + + def test_different_tool_ids_allowed(self): + """Different tool-ids should not conflict.""" + from ravendb.documents.ai.ai_conversation import AiConversation + + chat = AiConversation(store=None, agent_id="dummy") + chat.add_action_response("tool-1", "response-1") + chat.add_action_response("tool-2", "response-2") + self.assertEqual(2, len(chat._action_responses)) + if __name__ == "__main__": unittest.main() diff --git a/ravendb/tests/gen_ai_tests/test_gen_ai_operations.py b/ravendb/tests/gen_ai_tests/test_gen_ai_operations.py index fcea7c06..0d478f5c 100644 --- a/ravendb/tests/gen_ai_tests/test_gen_ai_operations.py +++ b/ravendb/tests/gen_ai_tests/test_gen_ai_operations.py @@ -253,6 +253,70 @@ def test_gen_ai_configuration_round_trip(self): self.assertEqual(original.max_concurrency, restored.max_concurrency) self.assertEqual(original.enable_tracing, restored.enable_tracing) + def test_version_not_in_json_when_none(self): + """Tests that Version key is omitted from JSON when version is None.""" + config = GenAiConfiguration( + name="TestGenAi", + identifier="test-gen-ai-1", + collection="Documents", + connection_string_name="my-connection", + prompt="Test prompt", + gen_ai_transformation=GenAiTransformation(script="ai.genContext(ctx);"), + update_script="this.Summary = $result;", + sample_object='{"summary": "test"}', + ) + json_data = config.to_json() + self.assertNotIn("Version", json_data) + + def test_version_in_json_when_set(self): + """Tests that Version key is included in JSON when version has a value.""" + config = GenAiConfiguration( + name="TestGenAi", + identifier="test-gen-ai-1", + collection="Documents", + connection_string_name="my-connection", + prompt="Test prompt", + gen_ai_transformation=GenAiTransformation(script="ai.genContext(ctx);"), + update_script="this.Summary = $result;", + sample_object='{"summary": "test"}', + version=1, + ) + json_data = config.to_json() + self.assertEqual(1, json_data["Version"]) + + def test_version_round_trip(self): + """Tests that version survives serialization -> deserialization.""" + config = GenAiConfiguration( + name="TestGenAi", + identifier="test-gen-ai-1", + collection="Documents", + connection_string_name="my-connection", + prompt="Test prompt", + gen_ai_transformation=GenAiTransformation(script="ai.genContext(ctx);"), + update_script="this.Summary = $result;", + sample_object='{"summary": "test"}', + version=1, + ) + json_data = config.to_json() + restored = GenAiConfiguration.from_json(json_data) + self.assertEqual(1, restored.version) + + def test_version_none_round_trip(self): + """Tests that None version survives serialization -> deserialization.""" + config = GenAiConfiguration( + name="TestGenAi", + identifier="test-gen-ai-1", + collection="Documents", + connection_string_name="my-connection", + prompt="Test prompt", + gen_ai_transformation=GenAiTransformation(script="ai.genContext(ctx);"), + update_script="this.Summary = $result;", + sample_object='{"summary": "test"}', + ) + json_data = config.to_json() + restored = GenAiConfiguration.from_json(json_data) + self.assertIsNone(restored.version) + class TestGenAiTransformation(unittest.TestCase): """Tests for GenAiTransformation. No server required."""