fix(dynamodb): paren-wrapped KeyCondition clauses and nested-path SET targets

[Bowbaq]

Heads up — this PR was written by an AI agent (Claude Code) under my direction. I don't write Rust day-to-day, so I can't speak to the idiom-level quality of the diff off the top of my head. The test coverage and reasoning should stand on their own; happy to revise anything that doesn't fit the project's style.

Summary

Two silent-failure bugs in the DynamoDB expression evaluator, both triggered by output shapes the aws-sdk-go-v2 expression builder emits. Same class as #257, #261, #368 — hand-rolled string scanning that misses a grammar context.

1. Parenthesised clauses in KeyConditionExpression silently return zero rows

Repro against 0.9.2:

cli := dynamodb.NewFromConfig(fakecloud.AWSConfig())
// Table: store_id (HASH) + order_id (RANGE). 3 items seeded with store_id="s".

works, _ := cli.Query(ctx, &dynamodb.QueryInput{
    TableName:              aws.String(tbl),
    KeyConditionExpression: aws.String("store_id = :s AND order_id > :a"),
    ExpressionAttributeValues: map[string]types.AttributeValue{
        ":s": &types.AttributeValueMemberS{Value: "s"},
        ":a": &types.AttributeValueMemberS{Value: "aaa"},
    },
})
fails, _ := cli.Query(ctx, &dynamodb.QueryInput{
    TableName:              aws.String(tbl),
    KeyConditionExpression: aws.String("(store_id = :s) AND (order_id > :a)"), // parens
    ExpressionAttributeValues: /* ...same... */,
})
// len(works.Items) == 3   ✅
// len(fails.Items) == 0   ❌ (real DynamoDB: 3)

The shape that reliably hits this is what the aws-sdk-go-v2 key-condition builder emits for a compound: (#0 = :0) AND (#1 > :1). So every caller composing KeyConditions through the official SDK saw silent empty results.

Root cause: split_on_and is paren-aware and correctly leaves each clause's outer parens intact, but evaluate_single_key_condition (the leaf evaluator) never called strip_outer_parens before searching for a comparison operator. part.find('=') on "(store_id = :s)" returns position 11, slicing left = "(store_id", which as an attribute name is absent from the item — the comparison silently evaluates to false.

Fix: restructure evaluate_key_condition to mirror evaluate_filter_expression (already correctly paren-handling): recursive, with top-level AND split and outer-paren stripping at each frame.

2. SET #a.#b = :v silently creates a top-level attribute named "#a.#b"

Repro against 0.9.2:

cli.UpdateItem(ctx, &dynamodb.UpdateItemInput{
    TableName: &tbl,
    Key: /* ... */,
    UpdateExpression: aws.String("SET #web.#tab_id = :tab"),
    ExpressionAttributeNames: map[string]string{
        "#web":    "web",
        "#tab_id": "tab_id",
    },
    ExpressionAttributeValues: map[string]types.AttributeValue{
        ":tab": &types.AttributeValueMemberS{Value: "tab-a"},
    },
})
// item.web.tab_id  → unchanged ❌
// item["#web.#tab_id"] → "tab-a" (new bogus top-level attribute) ❌

Root cause: apply_set_assignment called resolve_attr_name("#web.#tab_id", ...) on the whole LHS. resolve_attr_name only handles a single #name lookup — it saw the dotted path as one token, missed the match, and returned the literal string. item.insert("#web.#tab_id", value) then created a top-level attribute with that name instead of updating the nested map. No error, no side-effect visible to the caller.

Fix: detect dotted LHS paths in apply_set_assignment (is_dotted_path helper) and route them through a new assign_nested_path helper that resolves each #-prefixed segment and walks the M structure. Only plain-value RHS is supported in this first pass (no if_not_exists / list_append / arithmetic into a nested path — those shapes aren't common and can be added when needed). Writing through a missing parent returns ValidationException, matching real DynamoDB.

3. Corpus-driven test file (new)

Added crates/fakecloud-dynamodb/src/service/expression_corpus_tests.rs — 28 table-driven tests, one per dimension of the grammar (operators, paren shapes, SDK-builder output, functions with/without space before paren, IN, BETWEEN, nested paths, compound updates, etc.). Cases are seeded from aws-sdk-go-v2/feature/dynamodb/expression and moto/dynamodb/parsing/.

While building the corpus I also surfaced 5 pre-existing grammar gaps that aren't fixed in this PR — they're marked #[ignore = "known gap: ..."] with a description of the root cause. CI stays green; each ignored test is a work item:

• BETWEEN inside a compound expression (split_on_and grabs BETWEEN's inner AND) — affects both KeyCondition and Filter.
• size(X) without a space before the paren.
• Tab-separated tokens.
• Filter comparison LHS does not resolve dot-notation paths.

These can be tackled incrementally without blocking this fix.

Test plan

• 7 new unit tests in the corpus covering the two bug shapes (paren-wrapped KeyCondition clauses, nested-path SET targets, and sibling-preservation on nested SETs).
• Full regression net in expression_corpus_tests.rs — 28 tests.
• cargo test -p fakecloud-dynamodb — 204 pass, 5 known-gap ignored.
• cargo clippy -p fakecloud-dynamodb --all-targets -- -D warnings — clean.
• cargo fmt --check — clean.
• End-to-end verification: ran the downstream service's integration suite against a locally-built binary from this branch. The WSConnectionsService.SetTabContext flow (which hits both bugs in sequence) now works; previously it was writing "#web.#tab_id" as a top-level attribute and then failing to read it back.
• Not run: real-AWS behavioural parity. No credentials configured. Both fixes match the AWS documented grammar — paren grouping is universal, and SET a.b = :v writes into a's map per the UpdateExpression SET docs.

---

Summary by cubic

Fixes two DynamoDB expression bugs that could return empty Query results and write wrong updates when using aws-sdk-go-v2-generated expressions. Adds a corpus and end-to-end tests, and now rejects unsupported nested-path RHS instead of silently succeeding.

• Bug Fixes

  • KeyCondition: parenthesized clauses are parsed recursively. Matches SDK shapes like (#0 = :0) AND (#1 > :1) and returns correct rows.
  • UpdateExpression SET: dotted targets (e.g., SET #web.#tab_id = :tab) resolve each # segment and update the nested map. Missing parents raise ValidationException. Unresolvable/complex RHS into nested paths now raise ValidationException instead of silently doing nothing.
  • Go e2e: restored scheduler imports in sdks/go/e2e after merge to keep tests green.

• New Features

  • Added a corpus-driven test suite in fakecloud-dynamodb (33 tests) covering operators, paren shapes, SDK output, IN/BETWEEN, functions, and nested paths (5 known gaps are ignored).
  • Added end-to-end guards in fakecloud-e2e and sdks/go/e2e for paren-wrapped KeyCondition and nested-path SET to ensure SDK-shaped requests behave correctly.

^{Written for commit d9b6cac. Summary will update on new commits.}

[cubic-dev-ai]

1 issue found across 2 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="crates/fakecloud-dynamodb/src/service/mod.rs">

<violation number="1" location="crates/fakecloud-dynamodb/src/service/mod.rs:1732">
P2: Dotted-path SET handling can return success without applying any update when RHS is not directly resolvable, silently dropping assignments.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[codecov]

Codecov Report

❌ Patch coverage is 87.93103% with 7 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
crates/fakecloud-dynamodb/src/service/mod.rs	87.71%	7 Missing ⚠️

📢 Thoughts on this report? Let us know!

[vieiralucas]

Pushed two follow-up commits directly (maintainerCanModify: true) rather than ask you to revise — holler if you'd rather I revert and you take them over.

55a72b5 — fix + dead-arg drop

• apply_set_assignment: the nested-path branch returned Ok(()) when resolve_value couldn't handle the RHS (if_not_exists, list_append, arithmetic). That's the same silent-failure class this PR fixes elsewhere — switched to ValidationException so callers see the error instead of a dropped write.
• Dropped hash_key_name / range_key_name from evaluate_key_condition and _hash_key_name from evaluate_single_key_condition. All three were threaded through the recursion but never read at the leaf — the #[allow(clippy::only_used_in_recursion)] was covering dead params.
• Corpus: update_set_nested_path_complex_rhs_errors_cleanly locks in the new error behavior; update_set_nested_path_complex_rhs is #[ignore] and documents the underlying gap (complex RHS into a nested path).

733bd60 — e2e guards

• crates/fakecloud-e2e/tests/dynamodb.rs: dynamodb_query_paren_wrapped_key_condition + dynamodb_update_nested_set_path exercise the exact wire shapes through the HTTP stack via aws_sdk_dynamodb.
• sdks/go/e2e/e2e_test.go: TestE2EDynamoDBParenKeyCondition + TestE2EDynamoDBNestedSetPath do the same via aws-sdk-go-v2, matching the repros in the PR description.

Tests green locally: cargo test -p fakecloud-dynamodb (206 pass / 6 ignored), both new Rust e2e tests pass, both new Go e2e tests pass, cargo clippy --all-targets -- -D warnings clean, cargo fmt --check clean.