feat(sdk): add semantic search by file helper

[you06]

SDK: search file by file

Summary by CodeRabbit

• New Features

  • Added enriched file metadata functionality across all SDKs (JavaScript, Kotlin, Python, Rust, Swift) through new statMetadata / stat_metadata method, returning comprehensive file information including semantic classification, tags, content type, and standard metadata fields.

• Documentation

  • Updated README files for all SDKs documenting the new enriched metadata operation.

[coderabbitai]

📝 Walkthrough

Walkthrough

Five language SDKs (JavaScript, Kotlin, Python, Rust, Swift) gain a new statMetadata(path) method that fetches enriched file metadata via GET /v1/fs/{path}?stat=1. Each SDK defines a StatMetadataResult type, implements the client method to parse and normalize the response, exports the type, and documents the feature in its README.

Changes

Enriched File Metadata API Across SDKs

Layer / File(s)	Summary
StatMetadataResult type definitions clients/drive9-js/src/models.ts, clients/drive9-kotlin/lib/src/main/kotlin/com/drive9/mobile/Drive9.kt, clients/drive9-py/drive9/models.py, clients/drive9-rs/src/models.rs, clients/drive9-swift/Sources/Drive9Mobile/Drive9.swift	New StatMetadataResult interface/struct/dataclass added consistently across all five SDKs with fields for size, directory flag, resource ID, revision, modification time, content type, semantic_text, and tags.
Client method implementations clients/drive9-js/src/client.ts, clients/drive9-kotlin/lib/src/main/kotlin/com/drive9/mobile/Drive9.kt, clients/drive9-py/drive9/client.py, clients/drive9-rs/src/client.rs, clients/drive9-swift/Sources/Drive9Mobile/Drive9.swift	statMetadata(path) methods added to all five clients. Each method issues GET /v1/fs/{path}?stat=1, applies language-specific error handling, parses the JSON response, normalizes missing fields with defaults, and returns the typed result.
Type exports and re-exports clients/drive9-js/src/index.ts, clients/drive9-py/drive9/__init__.py	StatMetadataResult added to public module exports for JavaScript and Python SDKs.
README documentation clients/drive9-js/README.md, clients/drive9-kotlin/README.md, clients/drive9-py/README.md, clients/drive9-rs/README.md, clients/drive9-swift/README.md	Consistent updates to all README files adding statMetadata to supported operations tables and documenting the enriched metadata fields and underlying GET /v1/fs/{path}?stat=1 endpoint.
Test cleanup clients/drive9-js/tests/client.test.ts	Whitespace normalization inserted within test suite.

---

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly Related PRs

• mem9-ai/drive9#352: Adds resource_id field to the server-side enriched stat response; this PR implements client-side parsing and export of that enriched metadata across all SDK languages.

Suggested Reviewers

• qiffang
• JaySon-Huang

---

🐰 Five SDKs, one feature, enriched so bright,
StatMetadata hops in with semantic delight!
Types, clients, and docs all aligned,
A multi-language hop, perfectly designed! 🐇✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 30.43% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'add semantic search by file helper' is partially related to the changeset. The PR primarily adds a statMetadata() method across multiple SDK clients (JS, Kotlin, Python, Rust, Swift), but the title focuses on 'semantic search by file' which is only one aspect of the enriched metadata being returned (the semanticText field).	Consider a more descriptive title that captures the main change, such as 'feat(sdk): add statMetadata() for enriched file metadata' or 'feat(sdk): add enriched stat operation across SDKs'.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d4a416984f

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

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

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

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Preserve context for the final grep request

When SearchByFileCtx reaches the grep step after semantic text is available, it calls c.Grep, which constructs its request without the caller's ctx; the client also has no default HTTP timeout. In contexts with a deadline/cancellation, a slow or hung grep request can therefore block past the caller's cancellation instead of returning promptly, defeating the purpose of the Ctx variant. Use a context-aware grep request here or add a GrepCtx helper.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Use multipart-capable upload for query files

For local query files larger than the server's inline threshold, this simple PUT path fails before semantic extraction: handleWrite rejects large-file PUTs that do not include multipart checksums with missing X-Dat9-Part-Checksums. That makes SearchByFile unusable for common audio/image query files (for example the documented query.wav) unless they happen to be below the inline cutoff; use the existing stream/multipart upload path with expectedRevision=0 instead of WriteCtxConditional here.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Cap polling sleep at the remaining timeout

When callers pass a pollInterval larger than the remaining timeout, this sleeps for the full interval before checking the deadline again, so SearchByFileCtx can block well past the requested timeout (for example timeout=1s, pollInterval=1m waits about a minute). The helper advertises timeout as the bound for waiting on semantic_text; use a timer for the smaller of the poll interval and remaining time.

Useful? React with 👍 / 👎.

[coderabbitai]

Actionable comments posted: 7

🧹 Nitpick comments (1)

clients/drive9-py/drive9/models.py (1)

26-37: 💤 Low value

Consider aligning mtime handling with StatResult and improving tags type annotation.

Two minor observations:

1. StatResult.mtime is Optional[datetime] while StatMetadataResult.mtime is Optional[int] (Unix seconds). This inconsistency may confuse users switching between the two.

2. tags: Optional[dict] could be more precisely typed as Optional[Dict[str, str]] for better IDE support.

These are not blocking issues since the docstring clarifies the raw response format.

💡 Optional type refinement

+from typing import Dict, Optional
+
 `@dataclass`
 class StatMetadataResult:
     """Represents enriched file metadata from GET ?stat=1."""
     size: int
     is_dir: bool
     resource_id: str
     revision: int
     mtime: Optional[int] = None
     content_type: str = ""
     semantic_text: str = ""
-    tags: Optional[dict] = None
+    tags: Optional[Dict[str, str]] = None

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@clients/drive9-py/drive9/models.py` around lines 26 - 37,
StatMetadataResult's mtime and tags types should be aligned with StatResult for
consistency: change StatMetadataResult.mtime from Optional[int] to
Optional[datetime] (or normalize both to the same representation) and tighten
tags from Optional[dict] to Optional[Dict[str, str]]; update imports to include
typing.Dict and datetime as needed and ensure any serialization/deserialization
in methods that populate StatMetadataResult convert the raw Unix-second mtime to
a datetime if you choose datetime.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@clients/drive9-js/README.md`:
- Around line 70-75: Update the README entry for client.searchByFile to include
an options parameter and document timeout/poll controls: mention that
searchByFile(localPath, tmpPrefix, searchPrefix, limit?, options?) accepts an
optional options object (e.g., { timeoutMs, pollIntervalMs, maxPollAttempts,
abortSignal }) which controls how long to wait for semantic_text from
statMetadata() and how frequently to poll; list default values, describe
behavior on timeout (throws or returns empty), and update the table row and the
following paragraph to reference these option fields and their interaction with
semantic_text polling and temporary file cleanup.

In `@clients/drive9-py/tests/test_client.py`:
- Around line 201-205: The test registers DELETE mocks via
responses.add_callback (using the regex
f"{BASE_URL}/v1/fs/tmp/query/[0-9a-f]+\\.txt") but never asserts those mocks
were invoked; add an assertion after the operation that should trigger cleanup
to verify the DELETE occurred. Locate the responses.add_callback registration(s)
and then assert either responses.assert_call_count for DELETE (or check
responses.calls) contains at least one request whose method is DELETE and whose
URL matches the same regex (or the BASE_URL/.../tmp/query/[0-9a-f]+\.txt
pattern); do the same for the second registration referenced at 219-222 so the
test fails if remote temp-file cleanup wasn't performed.

In `@clients/drive9-rs/src/client.rs`:
- Around line 434-440: The current unique_id() uses SystemTime nanoseconds which
can collide; replace it to generate a UUIDv4 instead: add the uuid crate to
Cargo.toml (uuid = { version = "1", features = ["v4"] }) and update the
unique_id() function to return Uuid::new_v4().to_string() (remove the
SystemTime/duration logic). Ensure you import uuid::Uuid at the top of the
module and remove unused SystemTime/UNIX_EPOCH imports if no longer used.

In `@clients/drive9-rs/src/models.rs`:
- Around line 21-32: The struct StatMetadataResult can fail deserialization when
the server omits certain fields; update the struct by adding #[serde(default)]
on the potentially-missing fields (e.g., resource_id, content_type,
semantic_text, tags) so they fall back to their Rust Default values during
Deserialize; locate the StatMetadataResult definition and add #[serde(default)]
above each of those fields (tags will default to an empty HashMap, strings to
empty String) to match the behavior of the other SDKs.

In `@pkg/client/client.go`:
- Around line 1189-1201: Replace the manual string manipulations in joinRemote
with the canonical path utilities: import pkg/pathutil, use its join/normalize
functions (e.g., pathutil.Join + pathutil.Normalize or the equivalent
pathutil.JoinRemote API) to combine prefix and name, then validate/enforce
drive9 constraints (no "..", no backslashes, NFC, leading slash) via the
pathutil normalization/validation calls and return that normalized path; remove
the hand-rolled TrimRight/HasPrefix logic in the joinRemote function and rely on
pathutil to produce a safe, normalized remote path.
- Line 1163: The cleanup call currently reuses the caller's ctx for
c.DeleteCtx(ctx, tmpPath) so if the caller cancels the deletion is skipped;
change this to use a dedicated cleanup context (e.g., ctxCleanup :=
context.Background() or context.WithTimeout(context.Background(), 5*time.Second)
with defer cancel()) and call c.DeleteCtx(ctxCleanup, tmpPath) to ensure
best-effort removal of tmpPath even when the original ctx is canceled; update
any surrounding code to create and cancel the cleanup context properly.
- Line 1137: SearchByFileCtx is calling the non-cancellable c.Grep(semanticText,
searchPrefix, limit); update the call chain to propagate ctx by either adding a
context-aware Grep method (e.g., Client.GrepCtx(ctx, semanticText, searchPrefix,
limit)) or by changing the existing Grep signature to accept ctx, and then
replace the call in SearchByFileCtx to use the ctx-aware variant; inside the
new/updated Grep/GrepCtx implementation, build the HTTP request with
http.NewRequestWithContext(ctx, ...) (instead of http.NewRequest) so
cancellations/deadlines are honored during the grep HTTP request.

---

Nitpick comments:
In `@clients/drive9-py/drive9/models.py`:
- Around line 26-37: StatMetadataResult's mtime and tags types should be aligned
with StatResult for consistency: change StatMetadataResult.mtime from
Optional[int] to Optional[datetime] (or normalize both to the same
representation) and tighten tags from Optional[dict] to Optional[Dict[str,
str]]; update imports to include typing.Dict and datetime as needed and ensure
any serialization/deserialization in methods that populate StatMetadataResult
convert the raw Unix-second mtime to a datetime if you choose datetime.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6a8ee730-cf22-48aa-93e3-908574928574

📥 Commits

Reviewing files that changed from the base of the PR and between 69a20e6 and d4a4169.

📒 Files selected for processing (19)

• clients/drive9-js/README.md
• clients/drive9-js/src/client.ts
• clients/drive9-js/src/index.ts
• clients/drive9-js/src/models.ts
• clients/drive9-js/tests/client.test.ts
• clients/drive9-kotlin/README.md
• clients/drive9-kotlin/lib/src/main/kotlin/com/drive9/mobile/Drive9.kt
• clients/drive9-py/README.md
• clients/drive9-py/drive9/__init__.py
• clients/drive9-py/drive9/client.py
• clients/drive9-py/drive9/models.py
• clients/drive9-py/tests/test_client.py
• clients/drive9-rs/README.md
• clients/drive9-rs/src/client.rs
• clients/drive9-rs/src/models.rs
• clients/drive9-rs/tests/test_client.rs
• clients/drive9-swift/README.md
• clients/drive9-swift/Sources/Drive9Mobile/Drive9.swift
• pkg/client/client.go

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Document timeout/poll options in the searchByFile method signature.

The table omits optional timeout controls while the paragraph discusses timeout behavior, which makes the API harder to discover.

Suggested README signature update

-| Search by local file | `client.searchByFile(localPath, tmpPrefix, searchPrefix, limit?)` |
+| Search by local file | `client.searchByFile(localPath, tmpPrefix, searchPrefix, limit?, timeout?, pollInterval?)` |

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@clients/drive9-js/README.md` around lines 70 - 75, Update the README entry
for client.searchByFile to include an options parameter and document
timeout/poll controls: mention that searchByFile(localPath, tmpPrefix,
searchPrefix, limit?, options?) accepts an optional options object (e.g., {
timeoutMs, pollIntervalMs, maxPollAttempts, abortSignal }) which controls how
long to wait for semantic_text from statMetadata() and how frequently to poll;
list default values, describe behavior on timeout (throws or returns empty), and
update the table row and the following paragraph to reference these option
fields and their interaction with semantic_text polling and temporary file
cleanup.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Assert that temporary remote cleanup actually happened.

The test registers a DELETE mock but never asserts it was called, so cleanup regressions could pass undetected.

Proposed test assertion update

 def test_search_by_file(client):
     calls = []
     tmp_paths = []
+    deleted = False
@@
-    responses.add_callback(
-        responses.DELETE,
-        re.compile(f"{BASE_URL}/v1/fs/tmp/query/[0-9a-f]+\\.txt"),
-        callback=lambda req: (200, {}, ""),
-    )
+    def delete_callback(req):
+        nonlocal deleted
+        deleted = True
+        return (200, {}, "")
+
+    responses.add_callback(
+        responses.DELETE,
+        re.compile(f"{BASE_URL}/v1/fs/tmp/query/[0-9a-f]+\\.txt"),
+        callback=delete_callback,
+    )
@@
     assert len(results) == 1
     assert results[0].path == "/data/hit.txt"
     assert calls
+    assert deleted

Also applies to: 219-222

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@clients/drive9-py/tests/test_client.py` around lines 201 - 205, The test
registers DELETE mocks via responses.add_callback (using the regex
f"{BASE_URL}/v1/fs/tmp/query/[0-9a-f]+\\.txt") but never asserts those mocks
were invoked; add an assertion after the operation that should trigger cleanup
to verify the DELETE occurred. Locate the responses.add_callback registration(s)
and then assert either responses.assert_call_count for DELETE (or check
responses.calls) contains at least one request whose method is DELETE and whose
URL matches the same regex (or the BASE_URL/.../tmp/query/[0-9a-f]+\.txt
pattern); do the same for the second registration referenced at 219-222 so the
test fails if remote temp-file cleanup wasn't performed.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if uuid crate is already a dependency
rg -l "^uuid" --glob "Cargo.toml" clients/drive9-rs/
cat clients/drive9-rs/Cargo.toml | grep -A5 "\[dependencies\]"

Repository: mem9-ai/drive9

Length of output: 274

---

Replace timestamp-based unique_id() with a random UUID to avoid collisions

clients/drive9-rs/src/client.rs’s unique_id() derives IDs from SystemTime nanoseconds (format!("{:x}", nanos)), which can collide if multiple calls happen within the same clock resolution/tick. Switching to UUID v4 removes this collision risk. Also, clients/drive9-rs/Cargo.toml currently has no uuid dependency.

Proposed fix using uuid crate

uuid = { version = "1", features = ["v4"] }

-fn unique_id() -> String {
-    let nanos = SystemTime::now()
-        .duration_since(UNIX_EPOCH)
-        .map(|d| d.as_nanos())
-        .unwrap_or_default();
-    format!("{:x}", nanos)
+fn unique_id() -> String {
+    uuid::Uuid::new_v4().simple().to_string()
 }

📝 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

	fn unique_id() -> String {
	let nanos = SystemTime::now()
	.duration_since(UNIX_EPOCH)
	.map(|d| d.as_nanos())
	.unwrap_or_default();
	format!("{:x}", nanos)
	}
	fn unique_id() -> String {
	uuid::Uuid::new_v4().simple().to_string()
	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@clients/drive9-rs/src/client.rs` around lines 434 - 440, The current
unique_id() uses SystemTime nanoseconds which can collide; replace it to
generate a UUIDv4 instead: add the uuid crate to Cargo.toml (uuid = { version =
"1", features = ["v4"] }) and update the unique_id() function to return
Uuid::new_v4().to_string() (remove the SystemTime/duration logic). Ensure you
import uuid::Uuid at the top of the module and remove unused
SystemTime/UNIX_EPOCH imports if no longer used.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add #[serde(default)] to handle potentially missing fields gracefully.

If the server omits fields like resource_id, content_type, semantic_text, or tags, deserialization will fail. Other SDKs (JS, Kotlin, Swift) provide defaults for missing fields. Adding #[serde(default)] ensures consistent behavior.

🛡️ Proposed fix to add default handling

-#[derive(Debug, Clone, Deserialize)]
+#[derive(Debug, Clone, Deserialize, Default)]
 pub struct StatMetadataResult {
+    #[serde(default)]
     pub size: i64,
     #[serde(rename = "isdir")]
+    #[serde(default)]
     pub is_dir: bool,
+    #[serde(default)]
     pub resource_id: String,
+    #[serde(default)]
     pub revision: i64,
     pub mtime: Option<i64>,
+    #[serde(default)]
     pub content_type: String,
+    #[serde(default)]
     pub semantic_text: String,
+    #[serde(default)]
     pub tags: std::collections::HashMap<String, String>,
 }

📝 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

	#[derive(Debug, Clone, Deserialize)]
	pub struct StatMetadataResult {
	pub size: i64,
	#[serde(rename = "isdir")]
	pub is_dir: bool,
	pub resource_id: String,
	pub revision: i64,
	pub mtime: Option<i64>,
	pub content_type: String,
	pub semantic_text: String,
	pub tags: std::collections::HashMap<String, String>,
	}
	#[derive(Debug, Clone, Deserialize, Default)]
	pub struct StatMetadataResult {
	#[serde(default)]
	pub size: i64,
	#[serde(rename = "isdir")]
	#[serde(default)]
	pub is_dir: bool,
	#[serde(default)]
	pub resource_id: String,
	#[serde(default)]
	pub revision: i64,
	pub mtime: Option<i64>,
	#[serde(default)]
	pub content_type: String,
	#[serde(default)]
	pub semantic_text: String,
	#[serde(default)]
	pub tags: std::collections::HashMap<String, String>,
	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@clients/drive9-rs/src/models.rs` around lines 21 - 32, The struct
StatMetadataResult can fail deserialization when the server omits certain
fields; update the struct by adding #[serde(default)] on the potentially-missing
fields (e.g., resource_id, content_type, semantic_text, tags) so they fall back
to their Rust Default values during Deserialize; locate the StatMetadataResult
definition and add #[serde(default)] above each of those fields (tags will
default to an empty HashMap, strings to empty String) to match the behavior of
the other SDKs.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify whether a context-aware Grep variant exists and where Grep is called.
rg -nP --type=go -C2 '\bfunc\s*\(c \*Client\)\s*GrepCtx\s*\('
rg -nP --type=go -C2 '\bfunc\s*\(c \*Client\)\s*Grep\s*\('
rg -nP --type=go -C2 '\bSearchByFileCtx\s*\(|\bGrep\s*\('

Repository: mem9-ai/drive9

Length of output: 5244

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Show Grep and SearchByFileCtx implementations around the reported line
sed -n '1060,1160p' pkg/client/client.go | cat -n

# Show whether any GrepCtx exists anywhere else
rg -n --type=go '\bGrepCtx\b' pkg/client || true
rg -n --type=go '\bfunc\s*\(c \*Client\)\s*GrepCtx\s*\(' -S . || true

# Show Grep callsites where SearchByFileCtx is used (context propagation)
rg -n --type=go 'SearchByFileCtx\(' -S pkg cmd . || true

Repository: mem9-ai/drive9

Length of output: 4390

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find Client.do implementation
rg -n --type=go 'func\s*\(c \*Client\)\s*do\s*\(' pkg/client/client.go
rg -n --type=go '\.do\(req\)' pkg/client/client.go

# Print a focused window around Client.do and any related request creation helpers
sed -n '1,260p' pkg/client/client.go | cat -n
# Narrow further around the do() function once we know its location
# (fallback: print around likely area where do is defined)
sed -n '260,520p' pkg/client/client.go | cat -n

Repository: mem9-ai/drive9

Length of output: 23791

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Repo-wide search for any context-aware grep variant
rg -n --type=go '\bGrepCtx\b' . || true
rg -n --type=go 'func\s*\(c\s*\*Client\)\s*Grep\w*Ctx\(' . || true

# Also confirm the grep request in Grep() uses http.NewRequest (no context)
rg -n --type=go 'func\s*\(c \*Client\)\s*Grep\(' pkg/client/client.go
rg -n --type=go 'http\.NewRequest\(' pkg/client/client.go

Repository: mem9-ai/drive9

Length of output: 435

---

Propagate ctx into the grep request path.

SearchByFileCtx is context-aware, but it calls c.Grep(...) (non-ctx) at pkg/client/client.go:1137, so cancellation/deadlines won’t apply to the grep HTTP request. Client.Grep also constructs the request with http.NewRequest (no context), making it inherently non-cancellable.

result, resultErr = c.Grep(semanticText, searchPrefix, limit)

Add a context-aware grep method (e.g., GrepCtx) and use it from SearchByFileCtx, or update the existing Grep to accept/use ctx (per “Pass context through call chains”).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/client/client.go` at line 1137, SearchByFileCtx is calling the
non-cancellable c.Grep(semanticText, searchPrefix, limit); update the call chain
to propagate ctx by either adding a context-aware Grep method (e.g.,
Client.GrepCtx(ctx, semanticText, searchPrefix, limit)) or by changing the
existing Grep signature to accept ctx, and then replace the call in
SearchByFileCtx to use the ctx-aware variant; inside the new/updated
Grep/GrepCtx implementation, build the HTTP request with
http.NewRequestWithContext(ctx, ...) (instead of http.NewRequest) so
cancellations/deadlines are honored during the grep HTTP request.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use a cleanup context independent of caller cancellation.

Line 1163 reuses ctx for delete. If caller cancels/times out, cleanup is skipped and temp files are left behind; this undermines the helper’s best-effort cleanup contract.

Proposed fix

-	_ = c.DeleteCtx(ctx, tmpPath)
+	cleanupCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+	_ = c.DeleteCtx(cleanupCtx, tmpPath)

📝 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

	_ = c.DeleteCtx(ctx, tmpPath)
	cleanupCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
	defer cancel()
	_ = c.DeleteCtx(cleanupCtx, tmpPath)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/client/client.go` at line 1163, The cleanup call currently reuses the
caller's ctx for c.DeleteCtx(ctx, tmpPath) so if the caller cancels the deletion
is skipped; change this to use a dedicated cleanup context (e.g., ctxCleanup :=
context.Background() or context.WithTimeout(context.Background(), 5*time.Second)
with defer cancel()) and call c.DeleteCtx(ctxCleanup, tmpPath) to ensure
best-effort removal of tmpPath even when the original ctx is canceled; update
any surrounding code to create and cancel the cleanup context properly.

[coderabbitai]

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Replace raw path string manipulation with pkg/pathutil.

joinRemote manually builds remote paths and does not enforce normalization constraints (.., backslashes, NFC), which is explicitly prohibited for drive9 paths.

As per coding guidelines, "Use pkg/pathutil for all path normalization; never manipulate raw path strings directly."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/client/client.go` around lines 1189 - 1201, Replace the manual string
manipulations in joinRemote with the canonical path utilities: import
pkg/pathutil, use its join/normalize functions (e.g., pathutil.Join +
pathutil.Normalize or the equivalent pathutil.JoinRemote API) to combine prefix
and name, then validate/enforce drive9 constraints (no "..", no backslashes,
NFC, leading slash) via the pathutil normalization/validation calls and return
that normalized path; remove the hand-rolled TrimRight/HasPrefix logic in the
joinRemote function and rely on pathutil to produce a safe, normalized remote
path.

[coderabbitai]

🧹 Nitpick comments (1)

clients/drive9-py/drive9/client.py (1)

198-212: ⚡ Quick win

Consider converting mtime to datetime for consistency with stat().

The existing stat() method (lines 181-196) converts the mtime header to a datetime object. For consistency, stat_metadata() should also convert the epoch-seconds mtime field to datetime:

from datetime import timezone

mtime = None
mt = data.get("mtime")
if mt is not None:
    try:
        mtime = datetime.fromtimestamp(int(mt), tz=timezone.utc)
    except (ValueError, OSError):
        pass

This would align with the TypeScript SDK (line 163 in client.ts converts to Date) and ensure a consistent API surface across methods in the Python client.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@clients/drive9-py/drive9/client.py` around lines 198 - 212, The stat_metadata
method currently returns mtime raw from the response; change it to mirror stat()
by converting the epoch-seconds mtime to a timezone-aware datetime in UTC (use
datetime.fromtimestamp(..., tz=timezone.utc)), handling missing or invalid
values by leaving mtime as None and catching ValueError/OSError; update imports
to include datetime and timezone and set the StatMetadataResult mtime field to
the converted datetime (or None) in stat_metadata.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@clients/drive9-py/drive9/client.py`:
- Around line 198-212: The stat_metadata method currently returns mtime raw from
the response; change it to mirror stat() by converting the epoch-seconds mtime
to a timezone-aware datetime in UTC (use datetime.fromtimestamp(...,
tz=timezone.utc)), handling missing or invalid values by leaving mtime as None
and catching ValueError/OSError; update imports to include datetime and timezone
and set the StatMetadataResult mtime field to the converted datetime (or None)
in stat_metadata.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 84482b33-a7e2-467a-96d6-ca22365a1d3e

📥 Commits

Reviewing files that changed from the base of the PR and between d4a4169 and 6d2e8e8.

📒 Files selected for processing (16)

• clients/drive9-js/README.md
• clients/drive9-js/src/client.ts
• clients/drive9-js/src/index.ts
• clients/drive9-js/src/models.ts
• clients/drive9-js/tests/client.test.ts
• clients/drive9-kotlin/README.md
• clients/drive9-kotlin/lib/src/main/kotlin/com/drive9/mobile/Drive9.kt
• clients/drive9-py/README.md
• clients/drive9-py/drive9/__init__.py
• clients/drive9-py/drive9/client.py
• clients/drive9-py/drive9/models.py
• clients/drive9-rs/README.md
• clients/drive9-rs/src/client.rs
• clients/drive9-rs/src/models.rs
• clients/drive9-swift/README.md
• clients/drive9-swift/Sources/Drive9Mobile/Drive9.swift

✅ Files skipped from review due to trivial changes (7)

• clients/drive9-js/src/index.ts
• clients/drive9-swift/README.md
• clients/drive9-rs/README.md
• clients/drive9-kotlin/README.md
• clients/drive9-py/README.md
• clients/drive9-py/drive9/init.py
• clients/drive9-js/tests/client.test.ts

🚧 Files skipped from review as they are similar to previous changes (4)

• clients/drive9-js/src/models.ts
• clients/drive9-rs/src/models.rs
• clients/drive9-py/drive9/models.py
• clients/drive9-kotlin/lib/src/main/kotlin/com/drive9/mobile/Drive9.kt