Conversation
📝 WalkthroughWalkthroughNew CouchDB/Cloudant connector: DSN normalization, Kivik client creation/teardown with ping, namespace enumeration and partition planning, paginated listing, batched writes with conflict resolution, continuous change and LSN streaming, CLI flags, unit and integration tests, and a Kivik dependency. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
connectors/couchdb/conn_test.go (1)
14-77: Add a mixed-case URI case to this table.These cases only cover lowercase schemes, so they won't catch the current case-sensitive prefix-trimming bug in
convertUriToDSN. ACouchDB://...orCloudant://...case would pin the intended behavior down.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@connectors/couchdb/conn_test.go` around lines 14 - 77, Add a test case with mixed-case schemes to TestConvertUriToDSN to catch case-sensitive trimming in convertUriToDSN: extend the tests slice with entries like name "mixed-case couchdb" and uri "CouchDB://localhost:5984" expecting "http://localhost:5984", and "mixed-case cloudant" with "Cloudant://user:pass@account.cloudant.com" expecting "https://user:pass@account.cloudant.com"; run the table-driven loop unchanged so convertUriToDSN is exercised for mixed-case prefixes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@connectors/couchdb/conn.go`:
- Around line 391-403: WriteData currently logs non-409 per-document bulk errors
inside the results loop (the for i, result := range results block) but does not
propagate them, so partial failures can be reported as success; change the logic
in the BulkDocs result handling to collect any non-409 document errors (and any
retry errors treated the same) into a returned error instead of only slog.Error.
Specifically, in the loop that manipulates conflictIDs and conflictDocs, add a
nonConflictErrors slice or aggregate error variable, append a descriptive entry
for result.Error (including result.ID and the underlying error) whenever
kivik.HTTPStatus(result.Error) != 409, and after processing results return that
aggregated error from WriteData (or wrap it) so the RPC surface reflects
per-document failures; ensure the same change is applied to the mirrored block
around the retry handling (the 434-443 area) so retry failures are also
propagated rather than only logged.
- Around line 531-535: The WriteUpdates implementation currently skips malformed
JSON (json.Unmarshal failure) and only logs per-document BulkDocs failures,
which allows WriteUpdates to return success while silently dropping updates;
change it so that WriteUpdates returns an error when any update is not applied:
in the loop that unmarshals update.GetData() (inside WriteUpdates) replace the
continue on json.Unmarshal error with early return of an error (including the
update ID and JSON error), and for the BulkDocs call(s) collect per-document
failures (the BulkDocs response entries) into an aggregated error instead of
just logging them (return the aggregated error if any entry failed); ensure the
function signature and callers handle and propagate the returned error
accordingly so failures are not silently ignored.
- Around line 163-170: The current logic computes numPartitions and
docsPerPartition but calls db.AllDocs once with "skip" set to docsPerPartition
and "limit" set to numPartitions-1, which returns a contiguous window instead of
individual partition boundaries; fix by querying for each boundary index i
(1..numPartitions-1) and call db.AllDocs (or equivalent) with "skip" = i *
docsPerPartition and "limit" = 1 (include_docs=false) to fetch a single row per
boundary, collecting those keys/rows into the partition list (adjusting the code
around numPartitions, docsPerPartition, and the rows/db.AllDocs usage).
- Around line 302-316: When pageCursor (from r.Msg.GetCursor()) is present we
currently set params["startkey"] and params["skip"] but drop the partition's end
bound; to fix, still decode the partition cursor via
decodeCursor(partition.GetCursor()) even when pageCursor != "" and, if endKey is
non-empty, set params["endkey"] so the partition upper bound is preserved;
update the logic around pageCursor handling in the block that sets
params["startkey"], params["skip"] to also compute endKey and set
params["endkey"] when applicable.
- Around line 527-542: The handler currently trusts the incoming JSON's _id
instead of the authoritative Update.Id (docID), which can cause writes to the
wrong document; after unmarshalling update.GetData() into doc in the
insert/update branch (the block that uses json.Unmarshal, docID, revMap, and
upsertDocs), explicitly set doc["_id"] = docID (overriding or filling in any
payload value) so the CouchDB upsert uses the Update.Id, and keep the rev
handling using revMap[docID] as-is; this ensures the persisted document _id
always matches update.GetId().
- Around line 66-80: The convertUriToDSN function incorrectly uses TrimPrefix on
the original case-sensitive uri after matching against a lowercased copy; change
it to compute the scheme by inspecting the lowercased string (as already done)
but then extract the suffix using the "://"-separator position from the original
uri (e.g. find idx := strings.Index(uri, "://"); suffix := uri[idx+3:]) and
build the DSN as "http://"+suffix or "https://"+suffix for the matched schemes
(handle "couchdb://", "couchdbs://", "cloudant://", or existing http/https) so
inputs like "CouchDB://..." produce the correct URL in convertUriToDSN.
In `@internal/app/options/connectorflags.go`:
- Around line 522-535: IsConnector currently only recognizes "couchdb://",
"couchdbs://", and "cloudant://", but convertUriToDSN also accepts "http://" and
"https://", so update the IsConnector func to also detect "http://" and
"https://" (e.g., add strings.HasPrefix(lower, "http://") ||
strings.HasPrefix(lower, "https://")) so CLI-parsed CouchDB URIs are reachable;
ensure the Create block (Create and couchdb.ConnectorSettings{Uri: args[0]} /
couchdb.NewConn) continues to accept the same URI forms without other changes.
---
Nitpick comments:
In `@connectors/couchdb/conn_test.go`:
- Around line 14-77: Add a test case with mixed-case schemes to
TestConvertUriToDSN to catch case-sensitive trimming in convertUriToDSN: extend
the tests slice with entries like name "mixed-case couchdb" and uri
"CouchDB://localhost:5984" expecting "http://localhost:5984", and "mixed-case
cloudant" with "Cloudant://user:pass@account.cloudant.com" expecting
"https://user:pass@account.cloudant.com"; run the table-driven loop unchanged so
convertUriToDSN is exercised for mixed-case prefixes.
🪄 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: 873e5a94-7f0b-42f3-a4e7-d302dd961580
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (6)
connectors/couchdb/conn.goconnectors/couchdb/conn_test.goconnectors/couchdb/integration_test.goconnectors/null/connector.gogo.modinternal/app/options/connectorflags.go
Comment on lines
+66
to
+80
| func convertUriToDSN(uri string) (string, error) { | ||
| lower := strings.ToLower(uri) | ||
| if strings.HasPrefix(lower, "couchdb://") { | ||
| return "http://" + strings.TrimPrefix(uri, "couchdb://"), nil | ||
| } | ||
| if strings.HasPrefix(lower, "couchdbs://") { | ||
| return "https://" + strings.TrimPrefix(uri, "couchdbs://"), nil | ||
| } | ||
| if strings.HasPrefix(lower, "cloudant://") { | ||
| return "https://" + strings.TrimPrefix(uri, "cloudant://"), nil | ||
| } | ||
| if strings.HasPrefix(lower, "http://") || strings.HasPrefix(lower, "https://") { | ||
| return uri, nil | ||
| } | ||
| return "", fmt.Errorf("unsupported URI scheme: %s", uri) |
There was a problem hiding this comment.
Normalize the scheme from the parsed URI, not with case-sensitive TrimPrefix.
Lines 68-75 test a lowercased copy, but the actual TrimPrefix calls use the original string. Inputs like CouchDB://host:5984 or Cloudant://... therefore build malformed DSNs instead of valid HTTP(S) URLs.
Proposed fix
func convertUriToDSN(uri string) (string, error) {
- lower := strings.ToLower(uri)
- if strings.HasPrefix(lower, "couchdb://") {
- return "http://" + strings.TrimPrefix(uri, "couchdb://"), nil
- }
- if strings.HasPrefix(lower, "couchdbs://") {
- return "https://" + strings.TrimPrefix(uri, "couchdbs://"), nil
- }
- if strings.HasPrefix(lower, "cloudant://") {
- return "https://" + strings.TrimPrefix(uri, "cloudant://"), nil
- }
- if strings.HasPrefix(lower, "http://") || strings.HasPrefix(lower, "https://") {
- return uri, nil
- }
- return "", fmt.Errorf("unsupported URI scheme: %s", uri)
+ parsed, err := url.Parse(uri)
+ if err != nil {
+ return "", fmt.Errorf("invalid URI: %w", err)
+ }
+
+ switch strings.ToLower(parsed.Scheme) {
+ case "couchdb":
+ parsed.Scheme = "http"
+ case "couchdbs", "cloudant":
+ parsed.Scheme = "https"
+ case "http", "https":
+ // passthrough
+ default:
+ return "", fmt.Errorf("unsupported URI scheme: %s", uri)
+ }
+
+ return parsed.String(), nil
}📝 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
| func convertUriToDSN(uri string) (string, error) { | |
| lower := strings.ToLower(uri) | |
| if strings.HasPrefix(lower, "couchdb://") { | |
| return "http://" + strings.TrimPrefix(uri, "couchdb://"), nil | |
| } | |
| if strings.HasPrefix(lower, "couchdbs://") { | |
| return "https://" + strings.TrimPrefix(uri, "couchdbs://"), nil | |
| } | |
| if strings.HasPrefix(lower, "cloudant://") { | |
| return "https://" + strings.TrimPrefix(uri, "cloudant://"), nil | |
| } | |
| if strings.HasPrefix(lower, "http://") || strings.HasPrefix(lower, "https://") { | |
| return uri, nil | |
| } | |
| return "", fmt.Errorf("unsupported URI scheme: %s", uri) | |
| func convertUriToDSN(uri string) (string, error) { | |
| parsed, err := url.Parse(uri) | |
| if err != nil { | |
| return "", fmt.Errorf("invalid URI: %w", err) | |
| } | |
| switch strings.ToLower(parsed.Scheme) { | |
| case "couchdb": | |
| parsed.Scheme = "http" | |
| case "couchdbs", "cloudant": | |
| parsed.Scheme = "https" | |
| case "http", "https": | |
| // passthrough | |
| default: | |
| return "", fmt.Errorf("unsupported URI scheme: %s", uri) | |
| } | |
| return parsed.String(), nil | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@connectors/couchdb/conn.go` around lines 66 - 80, The convertUriToDSN
function incorrectly uses TrimPrefix on the original case-sensitive uri after
matching against a lowercased copy; change it to compute the scheme by
inspecting the lowercased string (as already done) but then extract the suffix
using the "://"-separator position from the original uri (e.g. find idx :=
strings.Index(uri, "://"); suffix := uri[idx+3:]) and build the DSN as
"http://"+suffix or "https://"+suffix for the matched schemes (handle
"couchdb://", "couchdbs://", "cloudant://", or existing http/https) so inputs
like "CouchDB://..." produce the correct URL in convertUriToDSN.
Comment on lines
+391
to
+403
| for i, result := range results { | ||
| if result.Error != nil { | ||
| if kivik.HTTPStatus(result.Error) == 409 { | ||
| docID, ok := docs[i]["_id"].(string) | ||
| if ok { | ||
| conflictIDs = append(conflictIDs, docID) | ||
| conflictDocs[docID] = docs[i] | ||
| } | ||
| } else { | ||
| slog.Error(fmt.Sprintf("Failed to insert document %s: %v", result.ID, result.Error)) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Propagate document-level BulkDocs failures from WriteData.
Non-409 errors on the first attempt and any retry errors are only logged here. The RPC can return success after CouchDB rejected part of the batch, which is unsafe for a sync sink.
Also applies to: 434-443
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@connectors/couchdb/conn.go` around lines 391 - 403, WriteData currently logs
non-409 per-document bulk errors inside the results loop (the for i, result :=
range results block) but does not propagate them, so partial failures can be
reported as success; change the logic in the BulkDocs result handling to collect
any non-409 document errors (and any retry errors treated the same) into a
returned error instead of only slog.Error. Specifically, in the loop that
manipulates conflictIDs and conflictDocs, add a nonConflictErrors slice or
aggregate error variable, append a descriptive entry for result.Error (including
result.ID and the underlying error) whenever kivik.HTTPStatus(result.Error) !=
409, and after processing results return that aggregated error from WriteData
(or wrap it) so the RPC surface reflects per-document failures; ensure the same
change is applied to the mirrored block around the retry handling (the 434-443
area) so retry failures are also propagated rather than only logged.
Comment on lines
+527
to
+542
| docID := string(update.GetId()[0].GetData()) | ||
|
|
||
| switch update.GetType() { | ||
| case adiomv1.UpdateType_UPDATE_TYPE_INSERT, adiomv1.UpdateType_UPDATE_TYPE_UPDATE: | ||
| var doc map[string]interface{} | ||
| if err := json.Unmarshal(update.GetData(), &doc); err != nil { | ||
| slog.Error(fmt.Sprintf("Failed to unmarshal update data: %v", err)) | ||
| continue | ||
| } | ||
|
|
||
| if rev, exists := revMap[docID]; exists { | ||
| doc["_rev"] = rev | ||
| } else { | ||
| delete(doc, "_rev") | ||
| } | ||
| upsertDocs = append(upsertDocs, doc) |
There was a problem hiding this comment.
Use Update.Id as the authoritative document _id.
Line 527 derives docID from Update.Id, but Lines 531-542 still trust whatever _id is inside update.Data. If the payload omits _id or disagrees with the key, this handler writes the wrong document.
Proposed fix
case adiomv1.UpdateType_UPDATE_TYPE_INSERT, adiomv1.UpdateType_UPDATE_TYPE_UPDATE:
var doc map[string]interface{}
if err := json.Unmarshal(update.GetData(), &doc); err != nil {
slog.Error(fmt.Sprintf("Failed to unmarshal update data: %v", err))
continue
}
+ doc["_id"] = docID
if rev, exists := revMap[docID]; exists {
doc["_rev"] = rev
} else {
delete(doc, "_rev")📝 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
| docID := string(update.GetId()[0].GetData()) | |
| switch update.GetType() { | |
| case adiomv1.UpdateType_UPDATE_TYPE_INSERT, adiomv1.UpdateType_UPDATE_TYPE_UPDATE: | |
| var doc map[string]interface{} | |
| if err := json.Unmarshal(update.GetData(), &doc); err != nil { | |
| slog.Error(fmt.Sprintf("Failed to unmarshal update data: %v", err)) | |
| continue | |
| } | |
| if rev, exists := revMap[docID]; exists { | |
| doc["_rev"] = rev | |
| } else { | |
| delete(doc, "_rev") | |
| } | |
| upsertDocs = append(upsertDocs, doc) | |
| docID := string(update.GetId()[0].GetData()) | |
| switch update.GetType() { | |
| case adiomv1.UpdateType_UPDATE_TYPE_INSERT, adiomv1.UpdateType_UPDATE_TYPE_UPDATE: | |
| var doc map[string]interface{} | |
| if err := json.Unmarshal(update.GetData(), &doc); err != nil { | |
| slog.Error(fmt.Sprintf("Failed to unmarshal update data: %v", err)) | |
| continue | |
| } | |
| doc["_id"] = docID | |
| if rev, exists := revMap[docID]; exists { | |
| doc["_rev"] = rev | |
| } else { | |
| delete(doc, "_rev") | |
| } | |
| upsertDocs = append(upsertDocs, doc) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@connectors/couchdb/conn.go` around lines 527 - 542, The handler currently
trusts the incoming JSON's _id instead of the authoritative Update.Id (docID),
which can cause writes to the wrong document; after unmarshalling
update.GetData() into doc in the insert/update branch (the block that uses
json.Unmarshal, docID, revMap, and upsertDocs), explicitly set doc["_id"] =
docID (overriding or filling in any payload value) so the CouchDB upsert uses
the Update.Id, and keep the rev handling using revMap[docID] as-is; this ensures
the persisted document _id always matches update.GetId().
Comment on lines
+531
to
+535
| var doc map[string]interface{} | ||
| if err := json.Unmarshal(update.GetData(), &doc); err != nil { | ||
| slog.Error(fmt.Sprintf("Failed to unmarshal update data: %v", err)) | ||
| continue | ||
| } |
There was a problem hiding this comment.
Do not acknowledge updates that were never applied.
Line 532 logs and skips malformed JSON, and Lines 562-565 only log per-document BulkDocs failures. WriteUpdates can therefore return success after silently dropping some updates.
Also applies to: 557-565
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@connectors/couchdb/conn.go` around lines 531 - 535, The WriteUpdates
implementation currently skips malformed JSON (json.Unmarshal failure) and only
logs per-document BulkDocs failures, which allows WriteUpdates to return success
while silently dropping updates; change it so that WriteUpdates returns an error
when any update is not applied: in the loop that unmarshals update.GetData()
(inside WriteUpdates) replace the continue on json.Unmarshal error with early
return of an error (including the update ID and JSON error), and for the
BulkDocs call(s) collect per-document failures (the BulkDocs response entries)
into an aggregated error instead of just logging them (return the aggregated
error if any entry failed); ensure the function signature and callers handle and
propagate the returned error accordingly so failures are not silently ignored.
Comment on lines
+522
to
+535
| { | ||
| Name: "CouchDB", | ||
| IsConnector: func(s string) bool { | ||
| lower := strings.ToLower(s) | ||
| return strings.HasPrefix(lower, "couchdb://") || | ||
| strings.HasPrefix(lower, "couchdbs://") || | ||
| strings.HasPrefix(lower, "cloudant://") | ||
| }, | ||
| Create: func(args []string, as AdditionalSettings) (adiomv1connect.ConnectorServiceHandler, []string, error) { | ||
| settings := couchdb.ConnectorSettings{Uri: args[0]} | ||
| return CreateHelper("CouchDB", "couchdb://user:pass@host:port OR cloudant://user:pass@account.cloudant.com [options]", CouchDBFlags(&settings), func(_ *cli.Context, _ []string, _ AdditionalSettings) (adiomv1connect.ConnectorServiceHandler, error) { | ||
| return couchdb.NewConn(settings) | ||
| })(args, as) | ||
| }, |
There was a problem hiding this comment.
Allow the HTTP(S) URI forms that the connector already supports.
convertUriToDSN accepts http:// and https://, but IsConnector never matches those prefixes here. That makes valid CouchDB connection strings unreachable from the CLI.
Proposed fix
{
Name: "CouchDB",
IsConnector: func(s string) bool {
lower := strings.ToLower(s)
return strings.HasPrefix(lower, "couchdb://") ||
strings.HasPrefix(lower, "couchdbs://") ||
- strings.HasPrefix(lower, "cloudant://")
+ strings.HasPrefix(lower, "cloudant://") ||
+ strings.HasPrefix(lower, "http://") ||
+ strings.HasPrefix(lower, "https://")
},📝 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
| { | |
| Name: "CouchDB", | |
| IsConnector: func(s string) bool { | |
| lower := strings.ToLower(s) | |
| return strings.HasPrefix(lower, "couchdb://") || | |
| strings.HasPrefix(lower, "couchdbs://") || | |
| strings.HasPrefix(lower, "cloudant://") | |
| }, | |
| Create: func(args []string, as AdditionalSettings) (adiomv1connect.ConnectorServiceHandler, []string, error) { | |
| settings := couchdb.ConnectorSettings{Uri: args[0]} | |
| return CreateHelper("CouchDB", "couchdb://user:pass@host:port OR cloudant://user:pass@account.cloudant.com [options]", CouchDBFlags(&settings), func(_ *cli.Context, _ []string, _ AdditionalSettings) (adiomv1connect.ConnectorServiceHandler, error) { | |
| return couchdb.NewConn(settings) | |
| })(args, as) | |
| }, | |
| { | |
| Name: "CouchDB", | |
| IsConnector: func(s string) bool { | |
| lower := strings.ToLower(s) | |
| return strings.HasPrefix(lower, "couchdb://") || | |
| strings.HasPrefix(lower, "couchdbs://") || | |
| strings.HasPrefix(lower, "cloudant://") || | |
| strings.HasPrefix(lower, "http://") || | |
| strings.HasPrefix(lower, "https://") | |
| }, | |
| Create: func(args []string, as AdditionalSettings) (adiomv1connect.ConnectorServiceHandler, []string, error) { | |
| settings := couchdb.ConnectorSettings{Uri: args[0]} | |
| return CreateHelper("CouchDB", "couchdb://user:pass@host:port OR cloudant://user:pass@account.cloudant.com [options]", CouchDBFlags(&settings), func(_ *cli.Context, _ []string, _ AdditionalSettings) (adiomv1connect.ConnectorServiceHandler, error) { | |
| return couchdb.NewConn(settings) | |
| })(args, as) | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/app/options/connectorflags.go` around lines 522 - 535, IsConnector
currently only recognizes "couchdb://", "couchdbs://", and "cloudant://", but
convertUriToDSN also accepts "http://" and "https://", so update the IsConnector
func to also detect "http://" and "https://" (e.g., add strings.HasPrefix(lower,
"http://") || strings.HasPrefix(lower, "https://")) so CLI-parsed CouchDB URIs
are reachable; ensure the Create block (Create and
couchdb.ConnectorSettings{Uri: args[0]} / couchdb.NewConn) continues to accept
the same URI forms without other changes.
There was a problem hiding this comment.
♻️ Duplicate comments (6)
connectors/couchdb/conn.go (6)
527-542:⚠️ Potential issue | 🟠 MajorUse
Update.Idas authoritative_idinWriteUpdates.Line 527 derives
docID, but the insert/update branch still trusts payload_id. If missing/mismatched, writes can target the wrong document.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@connectors/couchdb/conn.go` around lines 527 - 542, The insert/update branch in WriteUpdates trusts the incoming payload's _id instead of using the authoritative Update.Id (docID), which can cause writes to target the wrong document; update the branch handling in WriteUpdates to explicitly set doc["_id"] = docID (overriding any payload _id) and then proceed to apply _rev from revMap or delete _rev as currently done before appending to upsertDocs, referencing symbols update.GetId()/docID, revMap, doc["_id"], and upsertDocs to locate the change.
391-403:⚠️ Potential issue | 🔴 Critical
writeDataBatchshould fail on non-conflict per-documentBulkDocserrors.Line 400 and Line 441 only log failures. The RPC can return success after partial batch rejection, which is unsafe for a sink.
Also applies to: 434-443
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@connectors/couchdb/conn.go` around lines 391 - 403, In writeDataBatch, the per-document BulkDocs result loop currently only logs non-409 errors; change the logic in the results iteration (the loop handling results from BulkDocs in writeDataBatch) so that any result.Error that is not a 409 causes writeDataBatch to return an error (including result.ID and result.Error) instead of just slog.Error, while preserving the existing conflict handling that collects conflictIDs and conflictDocs for 409s; ensure the function propagates that error back to the caller so the RPC does not report success on partial non-conflict failures.
66-75:⚠️ Potential issue | 🟠 MajorNormalize DSN scheme replacement without case-sensitive
TrimPrefix.Line 69, Line 72, and Line 75 trim lowercase prefixes from the original URI, so mixed-case schemes (e.g.
CouchDB://...) generate malformed DSNs.Proposed fix
func convertUriToDSN(uri string) (string, error) { - lower := strings.ToLower(uri) - if strings.HasPrefix(lower, "couchdb://") { - return "http://" + strings.TrimPrefix(uri, "couchdb://"), nil - } - if strings.HasPrefix(lower, "couchdbs://") { - return "https://" + strings.TrimPrefix(uri, "couchdbs://"), nil - } - if strings.HasPrefix(lower, "cloudant://") { - return "https://" + strings.TrimPrefix(uri, "cloudant://"), nil - } - if strings.HasPrefix(lower, "http://") || strings.HasPrefix(lower, "https://") { - return uri, nil - } - return "", fmt.Errorf("unsupported URI scheme: %s", uri) + parsed, err := url.Parse(uri) + if err != nil { + return "", fmt.Errorf("invalid URI: %w", err) + } + + switch strings.ToLower(parsed.Scheme) { + case "couchdb": + parsed.Scheme = "http" + case "couchdbs", "cloudant": + parsed.Scheme = "https" + case "http", "https": + default: + return "", fmt.Errorf("unsupported URI scheme: %s", uri) + } + return parsed.String(), nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@connectors/couchdb/conn.go` around lines 66 - 75, convertUriToDSN currently checks prefixes against a lower-cased URI but uses TrimPrefix on the original uri, producing malformed DSNs for mixed-case schemes; fix by, after detecting a matched lowercase prefix (e.g. "couchdb://", "couchdbs://", "cloudant://") using the lower variable, remove the prefix from the original uri by slicing off len(prefix) (e.g. rest := uri[len(prefix):]) and then prepend the correct scheme ("http://" or "https://") to that rest; apply this change inside convertUriToDSN for the "couchdb://", "couchdbs://", and "cloudant://" branches so trimming is case-insensitive but preserves the original URI remainder.
163-170:⚠️ Potential issue | 🟠 MajorPartition boundaries are computed from one contiguous window, not per partition stride.
Using one
_all_docscall withskip=docsPerPartitionandlimit=numPartitions-1returns adjacent rows, not boundaries ati*docsPerPartition. For 3+ partitions this skews partition sizing badly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@connectors/couchdb/conn.go` around lines 163 - 170, The current code computes partition boundaries from a single AllDocs call using skip=docsPerPartition and limit=numPartitions-1 which returns a contiguous window rather than rows at offsets i*docsPerPartition; change the logic in the partitioning block around numPartitions, docsPerPartition and db.AllDocs to issue one query per boundary instead of one windowed query: for each boundary index i from 1 to numPartitions-1 call db.AllDocs with params {"include_docs": false, "limit": 1, "skip": i * docsPerPartition} (or the equivalent API to fetch the single row at that offset) and collect those keys/rows as partition boundaries, then build partitions from those collected offsets. This ensures boundaries are taken at multiples of docsPerPartition rather than adjacent rows.
302-317:⚠️ Potential issue | 🔴 CriticalPreserve partition
endkeywhen paginating withr.Msg.GetCursor().When page cursor is present,
decodeCursor(partition.GetCursor())is skipped, so Line 315 never runs and the partition upper bound is dropped. Paging can spill into the next partition.Proposed fix
- pageCursor := r.Msg.GetCursor() + startKey, endKey := decodeCursor(partition.GetCursor()) + pageCursor := r.Msg.GetCursor() if len(pageCursor) > 0 { // Page cursor from previous call - start after this key params["startkey"] = string(pageCursor) params["skip"] = 1 // skip the document we already returned } else { // Initial call - use partition cursor if present - startKey, endKey := decodeCursor(partition.GetCursor()) if startKey != "" { params["startkey"] = startKey params["skip"] = 1 } - if endKey != "" { - params["endkey"] = endKey - } + } + if endKey != "" { + params["endkey"] = endKey }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@connectors/couchdb/conn.go` around lines 302 - 317, When a page cursor (r.Msg.GetCursor()) is present you currently set params["startkey"] and params["skip"] but drop the partition upper bound because decodeCursor(partition.GetCursor()) is skipped; preserve the partition end key by calling decodeCursor(partition.GetCursor()) even when pageCursor is non-empty and, if the returned endKey != "", set params["endkey"] accordingly. Update the cursor-handling block that references r.Msg.GetCursor(), partition.GetCursor(), decodeCursor, and params["endkey"] so pagination starts after pageCursor while still enforcing the partition end key.
531-535:⚠️ Potential issue | 🔴 CriticalDo not acknowledge updates that fail to parse or fail in
BulkDocs.Malformed JSON (Line 532) and per-doc bulk errors (Line 563) are logged and skipped, then success is returned. This silently drops updates.
Also applies to: 557-565
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@connectors/couchdb/conn.go` around lines 531 - 535, The code currently logs and continues when json.Unmarshal(update.GetData(), &doc) fails and when per-document errors are returned from BulkDocs, which silently drops updates; instead, for the unmarshalling failure (the block using json.Unmarshal and variable doc/update) and for the BulkDocs response handling, do not simply continue—capture the failing update(s), mark or return them as failed (aggregate their IDs/errors) and return a non-nil error or a per-update failure result so callers will not acknowledge them; specifically modify the code around json.Unmarshal(update.GetData(), &doc) to record the parse error for that update and halt acknowledgement, and change the BulkDocs handling to inspect each item in the BulkDocs response, map any per-doc error back to the corresponding update and include it in the aggregated error/failed list so only truly succeeded updates are acknowledged.
🧹 Nitpick comments (1)
connectors/couchdb/conn.go (1)
721-724:ServerConnectTimeoutis exposed but currently unused.The setting is defaulted and configurable, but no code path applies it to connection setup. This is misleading operationally; either wire it to client transport/dial timeout or remove the setting.
Also applies to: 733-743
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@connectors/couchdb/conn.go` around lines 721 - 724, The ServerConnectTimeout setting is currently defaulted (settings.ServerConnectTimeout) but not applied; either remove it or wire it into the HTTP client/transport used to talk to CouchDB: when building the HTTP client or transport (where you already use settings.PingTimeout), set DialContext timeouts and/or http.Client.Timeout from settings.ServerConnectTimeout and use it for connection dialing/round-trip timeouts and any initial connection attempts (including the ping path); ensure the same value is honored for both initial connect and any client-level timeouts so the default is effective.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@connectors/couchdb/conn.go`:
- Around line 527-542: The insert/update branch in WriteUpdates trusts the
incoming payload's _id instead of using the authoritative Update.Id (docID),
which can cause writes to target the wrong document; update the branch handling
in WriteUpdates to explicitly set doc["_id"] = docID (overriding any payload
_id) and then proceed to apply _rev from revMap or delete _rev as currently done
before appending to upsertDocs, referencing symbols update.GetId()/docID,
revMap, doc["_id"], and upsertDocs to locate the change.
- Around line 391-403: In writeDataBatch, the per-document BulkDocs result loop
currently only logs non-409 errors; change the logic in the results iteration
(the loop handling results from BulkDocs in writeDataBatch) so that any
result.Error that is not a 409 causes writeDataBatch to return an error
(including result.ID and result.Error) instead of just slog.Error, while
preserving the existing conflict handling that collects conflictIDs and
conflictDocs for 409s; ensure the function propagates that error back to the
caller so the RPC does not report success on partial non-conflict failures.
- Around line 66-75: convertUriToDSN currently checks prefixes against a
lower-cased URI but uses TrimPrefix on the original uri, producing malformed
DSNs for mixed-case schemes; fix by, after detecting a matched lowercase prefix
(e.g. "couchdb://", "couchdbs://", "cloudant://") using the lower variable,
remove the prefix from the original uri by slicing off len(prefix) (e.g. rest :=
uri[len(prefix):]) and then prepend the correct scheme ("http://" or "https://")
to that rest; apply this change inside convertUriToDSN for the "couchdb://",
"couchdbs://", and "cloudant://" branches so trimming is case-insensitive but
preserves the original URI remainder.
- Around line 163-170: The current code computes partition boundaries from a
single AllDocs call using skip=docsPerPartition and limit=numPartitions-1 which
returns a contiguous window rather than rows at offsets i*docsPerPartition;
change the logic in the partitioning block around numPartitions,
docsPerPartition and db.AllDocs to issue one query per boundary instead of one
windowed query: for each boundary index i from 1 to numPartitions-1 call
db.AllDocs with params {"include_docs": false, "limit": 1, "skip": i *
docsPerPartition} (or the equivalent API to fetch the single row at that offset)
and collect those keys/rows as partition boundaries, then build partitions from
those collected offsets. This ensures boundaries are taken at multiples of
docsPerPartition rather than adjacent rows.
- Around line 302-317: When a page cursor (r.Msg.GetCursor()) is present you
currently set params["startkey"] and params["skip"] but drop the partition upper
bound because decodeCursor(partition.GetCursor()) is skipped; preserve the
partition end key by calling decodeCursor(partition.GetCursor()) even when
pageCursor is non-empty and, if the returned endKey != "", set params["endkey"]
accordingly. Update the cursor-handling block that references r.Msg.GetCursor(),
partition.GetCursor(), decodeCursor, and params["endkey"] so pagination starts
after pageCursor while still enforcing the partition end key.
- Around line 531-535: The code currently logs and continues when
json.Unmarshal(update.GetData(), &doc) fails and when per-document errors are
returned from BulkDocs, which silently drops updates; instead, for the
unmarshalling failure (the block using json.Unmarshal and variable doc/update)
and for the BulkDocs response handling, do not simply continue—capture the
failing update(s), mark or return them as failed (aggregate their IDs/errors)
and return a non-nil error or a per-update failure result so callers will not
acknowledge them; specifically modify the code around
json.Unmarshal(update.GetData(), &doc) to record the parse error for that update
and halt acknowledgement, and change the BulkDocs handling to inspect each item
in the BulkDocs response, map any per-doc error back to the corresponding update
and include it in the aggregated error/failed list so only truly succeeded
updates are acknowledged.
---
Nitpick comments:
In `@connectors/couchdb/conn.go`:
- Around line 721-724: The ServerConnectTimeout setting is currently defaulted
(settings.ServerConnectTimeout) but not applied; either remove it or wire it
into the HTTP client/transport used to talk to CouchDB: when building the HTTP
client or transport (where you already use settings.PingTimeout), set
DialContext timeouts and/or http.Client.Timeout from
settings.ServerConnectTimeout and use it for connection dialing/round-trip
timeouts and any initial connection attempts (including the ping path); ensure
the same value is honored for both initial connect and any client-level timeouts
so the default is effective.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a3fab24c-840c-43f2-b91c-947b3b8c2aad
📒 Files selected for processing (1)
connectors/couchdb/conn.go
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
connectors/couchdb/conn.go (2)
35-43:ServerConnectTimeoutsetting is defined but never used.The
ServerConnectTimeoutfield inConnectorSettingshas a default set (line 734) but is never applied anywhere in the code. OnlyPingTimeoutis used for the ping context.Either remove the unused setting or apply it appropriately (e.g., as the HTTP client timeout for the Kivik client).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@connectors/couchdb/conn.go` around lines 35 - 43, ConnectorSettings.ServerConnectTimeout is defined but never used; update the CouchDB client initialization to apply this timeout (instead of only using PingTimeout for the ping context) by wiring ServerConnectTimeout into the HTTP client or Kivik client creation (where the Kivik client is constructed and PingTimeout is used), e.g., set the underlying http.Client.Timeout or transport deadlines when creating the client so ConnectorSettings.ServerConnectTimeout controls overall connection/HTTP timeouts for functions like the Kivik client init and any dialing logic; alternatively, if you prefer to remove it, delete ServerConnectTimeout from ConnectorSettings and remove any default initialization to avoid dead config.
149-153: Silently skipping namespaces on stats failure may cause incomplete sync plans.When
db.Statsfails, the namespace is silently skipped with only a warning log. This could lead to incomplete sync plans without the caller being aware that certain namespaces were excluded.Consider either:
- Returning an error to fail fast
- Including the namespace with a default partition (count=0) so it's still processed
- Collecting failures and returning them in a structured way
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@connectors/couchdb/conn.go` around lines 149 - 153, The code currently calls db.Stats(ctx) and on error only logs via slog.Warn and continues, which silently skips namespaces (ns) and leads to incomplete sync plans; update the error handling in the namespace stats loop that calls db.Stats(ctx) so it either (A) returns an error from the enclosing function (wrap with context including ns and err) instead of calling slog.Warn, or (B) if you prefer to continue, replace the continue path by inserting a default partition entry for that ns (e.g., create a stats/partition entry with count=0) into the same collection used for successful stats so the namespace is still processed; ensure you reference db.Stats, the stats variable, ns, and replace slog.Warn with the chosen behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@connectors/couchdb/conn.go`:
- Around line 629-641: The current loop silently skips change events when
changes.ScanDoc(&doc) or json.Marshal(doc) fails (see uses of changes.ScanDoc
and json.Marshal on the local variable doc), which can drop CDC updates; update
the error handling to record/emit an observable metric and/or increment an error
counter whenever ScanDoc or Marshal fails (e.g., call a provided metrics.Inc or
emit to an error channel) and include contextual details in the log (change id
or sequence) so operators can detect dropped updates; ensure the function
handling CouchDB changes (the loop using changes.ScanDoc) does not just continue
silently but also updates the metric/error-state before continuing.
- Around line 346-358: The loop currently swallows failures from
rows.ScanDoc(&doc) and json.Marshal(doc) (after deleting "_rev"), which can
cause silent data loss; update the logic in the connector (the loop using
rows.ScanDoc, delete(doc, "_rev"), and json.Marshal) to surface failures instead
of silently continuing — either return an error immediately on the first ScanDoc
or Marshal failure, or accumulate failures in a structured error/metadata object
and include that metadata in the response so the caller can detect skipped
documents; ensure any change references the same symbols (rows.ScanDoc, doc,
delete(doc, "_rev"), json.Marshal) and updates the function's return signature
to convey the error/metadata as appropriate.
- Around line 758-760: The call to url.Parse(dsn) ignores its error and can
leave parsedURL nil, causing a panic when building baseURL and calling
slog.Info; update the code around parsedURL/url.Parse to capture the returned
error, check parsedURL for nil (or err != nil) before accessing parsedURL.Scheme
and parsedURL.Host, and if parsing fails use a safe fallback (e.g., use the
original dsn or a placeholder) so baseURL is always a valid string; then call
slog.Info with that safe baseURL.
---
Nitpick comments:
In `@connectors/couchdb/conn.go`:
- Around line 35-43: ConnectorSettings.ServerConnectTimeout is defined but never
used; update the CouchDB client initialization to apply this timeout (instead of
only using PingTimeout for the ping context) by wiring ServerConnectTimeout into
the HTTP client or Kivik client creation (where the Kivik client is constructed
and PingTimeout is used), e.g., set the underlying http.Client.Timeout or
transport deadlines when creating the client so
ConnectorSettings.ServerConnectTimeout controls overall connection/HTTP timeouts
for functions like the Kivik client init and any dialing logic; alternatively,
if you prefer to remove it, delete ServerConnectTimeout from ConnectorSettings
and remove any default initialization to avoid dead config.
- Around line 149-153: The code currently calls db.Stats(ctx) and on error only
logs via slog.Warn and continues, which silently skips namespaces (ns) and leads
to incomplete sync plans; update the error handling in the namespace stats loop
that calls db.Stats(ctx) so it either (A) returns an error from the enclosing
function (wrap with context including ns and err) instead of calling slog.Warn,
or (B) if you prefer to continue, replace the continue path by inserting a
default partition entry for that ns (e.g., create a stats/partition entry with
count=0) into the same collection used for successful stats so the namespace is
still processed; ensure you reference db.Stats, the stats variable, ns, and
replace slog.Warn with the chosen behavior.
🪄 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: 83ac644f-4527-4560-bba9-62c406c36d97
📒 Files selected for processing (1)
connectors/couchdb/conn.go
Comment on lines
+346
to
+358
| var doc map[string]interface{} | ||
| if err := rows.ScanDoc(&doc); err != nil { | ||
| slog.Warn(fmt.Sprintf("Failed to scan document: %v", err)) | ||
| continue | ||
| } | ||
|
|
||
| delete(doc, "_rev") | ||
|
|
||
| jsonBytes, err := json.Marshal(doc) | ||
| if err != nil { | ||
| slog.Warn(fmt.Sprintf("Failed to marshal document: %v", err)) | ||
| continue | ||
| } |
There was a problem hiding this comment.
Silently skipping documents with scan/marshal errors may cause undetected data loss.
Documents that fail ScanDoc or json.Marshal are logged and skipped. The response will report success with partial data, and the caller cannot distinguish between "all documents retrieved" and "some documents were silently dropped."
Consider either:
- Returning an error on the first failure
- Including metadata in the response about skipped documents
- Using a structured error that collects all failures
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@connectors/couchdb/conn.go` around lines 346 - 358, The loop currently
swallows failures from rows.ScanDoc(&doc) and json.Marshal(doc) (after deleting
"_rev"), which can cause silent data loss; update the logic in the connector
(the loop using rows.ScanDoc, delete(doc, "_rev"), and json.Marshal) to surface
failures instead of silently continuing — either return an error immediately on
the first ScanDoc or Marshal failure, or accumulate failures in a structured
error/metadata object and include that metadata in the response so the caller
can detect skipped documents; ensure any change references the same symbols
(rows.ScanDoc, doc, delete(doc, "_rev"), json.Marshal) and updates the
function's return signature to convey the error/metadata as appropriate.
Comment on lines
+629
to
+641
| var doc map[string]interface{} | ||
| if err := changes.ScanDoc(&doc); err != nil { | ||
| slog.Error(fmt.Sprintf("Failed to scan change doc: %v", err)) | ||
| continue | ||
| } | ||
|
|
||
| delete(doc, "_rev") | ||
|
|
||
| jsonBytes, err := json.Marshal(doc) | ||
| if err != nil { | ||
| slog.Error(fmt.Sprintf("Failed to marshal change doc: %v", err)) | ||
| continue | ||
| } |
There was a problem hiding this comment.
Silently skipping change events on scan/marshal errors may lose updates.
When ScanDoc or json.Marshal fails for a change event, it's logged and skipped. The stream continues without notifying the consumer that an update was dropped. For CDC (Change Data Capture) use cases, this could cause data inconsistency between source and sink.
Consider at minimum incrementing an error counter or emitting a metric so operators can detect when updates are being dropped.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@connectors/couchdb/conn.go` around lines 629 - 641, The current loop silently
skips change events when changes.ScanDoc(&doc) or json.Marshal(doc) fails (see
uses of changes.ScanDoc and json.Marshal on the local variable doc), which can
drop CDC updates; update the error handling to record/emit an observable metric
and/or increment an error counter whenever ScanDoc or Marshal fails (e.g., call
a provided metrics.Inc or emit to an error channel) and include contextual
details in the log (change id or sequence) so operators can detect dropped
updates; ensure the function handling CouchDB changes (the loop using
changes.ScanDoc) does not just continue silently but also updates the
metric/error-state before continuing.
Comment on lines
+758
to
+760
| parsedURL, _ := url.Parse(dsn) | ||
| baseURL := fmt.Sprintf("%s://%s", parsedURL.Scheme, parsedURL.Host) | ||
| slog.Info(fmt.Sprintf("Connected to CouchDB at %s", baseURL)) |
There was a problem hiding this comment.
Ignoring url.Parse error could cause nil pointer panic.
If url.Parse(dsn) fails, parsedURL will be nil, and accessing parsedURL.Scheme or parsedURL.Host on line 759 will panic.
Although the DSN was already used successfully to create the client, defensive error handling would prevent potential edge cases.
Proposed fix
- parsedURL, _ := url.Parse(dsn)
- baseURL := fmt.Sprintf("%s://%s", parsedURL.Scheme, parsedURL.Host)
- slog.Info(fmt.Sprintf("Connected to CouchDB at %s", baseURL))
+ parsedURL, err := url.Parse(dsn)
+ if err == nil {
+ baseURL := fmt.Sprintf("%s://%s", parsedURL.Scheme, parsedURL.Host)
+ slog.Info(fmt.Sprintf("Connected to CouchDB at %s", baseURL))
+ } else {
+ slog.Info("Connected to CouchDB")
+ }📝 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
| parsedURL, _ := url.Parse(dsn) | |
| baseURL := fmt.Sprintf("%s://%s", parsedURL.Scheme, parsedURL.Host) | |
| slog.Info(fmt.Sprintf("Connected to CouchDB at %s", baseURL)) | |
| parsedURL, err := url.Parse(dsn) | |
| if err == nil { | |
| baseURL := fmt.Sprintf("%s://%s", parsedURL.Scheme, parsedURL.Host) | |
| slog.Info(fmt.Sprintf("Connected to CouchDB at %s", baseURL)) | |
| } else { | |
| slog.Info("Connected to CouchDB") | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@connectors/couchdb/conn.go` around lines 758 - 760, The call to
url.Parse(dsn) ignores its error and can leave parsedURL nil, causing a panic
when building baseURL and calling slog.Info; update the code around
parsedURL/url.Parse to capture the returned error, check parsedURL for nil (or
err != nil) before accessing parsedURL.Scheme and parsedURL.Host, and if parsing
fails use a safe fallback (e.g., use the original dsn or a placeholder) so
baseURL is always a valid string; then call slog.Info with that safe baseURL.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Improvements
Tests
Chores