fix(codec): await nested Arrow decode in torch

[bbopen]

Fixes #21

What

• Ensure decodeValueAsync awaits nested envelope decodes when decoding torch.tensor values (so TorchTensor.data is never a Promise).

Tests

• Added a regression test for a torch tensor containing an Arrow-encoded ndarray.

[coderabbitai]

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

Added asynchronous envelope decoding internals and an async decode path so nested Promise-based decodes (e.g., Arrow-encoded ndarrays inside torch.tensor) are awaited; decodeValueAsync now uses the async envelope flow while synchronous decode paths remain unchanged.

Changes

Cohort / File(s)	Summary
Codec core & async envelope src/utils/codec.ts	Added MaybePromise and isPromiseLike; implemented decodeEnvelopeCore, decodeEnvelope (sync wrapper), and decodeEnvelopeAsync (async). Reworked envelope decoding to support both sync and async flows.
decodeValueAsync & Torch handling src/utils/codec.ts (torch branch)	decodeValueAsync now delegates to decodeEnvelopeAsync. Torch tensor branch uses async recursion so nested value fields (e.g., Arrow ndarrays) are awaited and returned resolved with preserved metadata (shape, dtype, device).
Tests: nested Arrow ndarray scenarios test/runtime_codec.test.ts	Added tests for nested Arrow ndarray decoding inside torch.tensor: async success (data resolves to Uint8Array), missing-arrow-decoder failure, nested-arrow-decoder error propagation, and a synchronous decode case.

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant Codec as codec (decodeValueAsync / decodeEnvelopeAsync)
  participant Arrow as Arrow decoder
  participant Result

  Caller->>Codec: decodeValueAsync(envelope)
  Codec->>Codec: inspect envelope.type (e.g., torch.tensor)
  alt nested value uses Arrow encoding
    Codec->>Arrow: decodeArrow(bytes) (async)
    Arrow-->>Codec: Promise<decoded ndarray>
    Codec->>Codec: await decoded ndarray
  end
  Codec->>Result: construct resolved object (no Promises in fields)
  Result-->>Caller: return resolved value

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• fix(codec): await nested Arrow decode in torch #33: Adds an async envelope decoder and updates decodeValueAsync to await nested Arrow decodes for torch.tensor, closely matching these changes.

Poem

🐇
I nibbled bytes and chased a dangling Promise,
Burrowed through Arrows until each array was honest.
Torch nests unwrapped, data tidy and bright,
No more waiting — decoded through the night. 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: fixing the codec to await nested Arrow decodes in torch tensor handling.
Description check	✅ Passed	The description clearly relates to the changeset, explaining the fix for ensuring decodeValueAsync awaits nested envelope decodes for torch.tensor values.
Linked Issues check	✅ Passed	The PR successfully addresses issue #21 by implementing async envelope decoding to await nested Arrow decodes in torch tensor handling, with appropriate test coverage.
Out of Scope Changes check	✅ Passed	All changes are focused on fixing the nested Arrow decode issue in torch tensor handling and adding related test coverage, with no extraneous modifications.

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

✨ Finishing touches

• 📝 Generate docstrings

---

---

📜 Recent review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a135598 and d54aacb.

📒 Files selected for processing (2)

• src/utils/codec.ts
• test/runtime_codec.test.ts

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

---

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

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In `@src/utils/codec.ts`:
- Around line 261-370: Duplicate envelope-parsing/validation logic exists
between decodeEnvelope and decodeEnvelopeAsync; extract the shared
marker/validation and per-marker extraction into a single helper (e.g.,
parseEnvelope or extractEnvelopePayload) that returns a normalized envelope
descriptor (marker, encoding, and raw fields) and have both decodeEnvelope and
decodeEnvelopeAsync call that helper, with decodeEnvelopeAsync awaiting nested
decodeArrow calls and decodeEnvelope either wrapping a sync decodeArrow or
calling the same helper plus synchronous decoders; update usages where
torch.tensor and nested value decoding occur so the async path awaits
decodeEnvelopeAsync but reuses the same extracted envelope data.

In `@test/runtime_codec.test.ts`:
- Around line 395-419: Add an explicit assertion that the decoded `data` is not
a Promise to prevent regressions where `data` was unresolved (issue `#21`): after
calling `decodeValueAsync(envelope)` in the test, add a check such as
`expect(result.data).not.toBeInstanceOf(Promise)` (or assert `typeof
result.data.then !== 'function'`) in addition to the existing
`expect.any(Uint8Array)` check so the `decodeValueAsync` /
`registerArrowDecoder` path is validated to return a concrete Uint8Array rather
than a Promise.

📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a135598 and ddebbee.

📒 Files selected for processing (2)

• src/utils/codec.ts
• test/runtime_codec.test.ts

🧰 Additional context used

🧬 Code graph analysis (2)

test/runtime_codec.test.ts (1)

src/utils/codec.ts (3)

• registerArrowDecoder (102-106)
• CodecEnvelope (43-90)
• decodeValueAsync (375-377)

src/utils/codec.ts (1)

src/index.ts (1)

• decodeValueAsync (71-71)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: os (windows-latest)

🔇 Additional comments (3)

src/utils/codec.ts (2)

339-347: Core fix is correct—nested Arrow decoding is now properly awaited.

The key change at line 340 (await decodeEnvelopeAsync(torchValue.value, decodeArrow)) correctly addresses issue #21 by ensuring that when a torch tensor contains a nested Arrow-encoded ndarray, the Promise is resolved before assignment to TorchTensor.data.

---

375-377: LGTM—decodeValueAsync now correctly delegates to the async helper.

The updated implementation properly chains to decodeEnvelopeAsync with tryDecodeArrowTable, ensuring all nested async decodes are awaited.

test/runtime_codec.test.ts (1)

396-396: Decoder is synchronous but decodeValueAsync wraps it in tryDecodeArrowTable.

The registered decoder (bytes => bytes) is synchronous, but tryDecodeArrowTable in codec.ts wraps it in an async function. This test still validates the fix because decodeEnvelopeAsync awaits the result regardless. The test coverage is adequate for the regression.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

[bbopen]

Addressed CodeRabbit feedback:

• Deduped sync/async envelope decoding via decodeEnvelopeCore
• Added explicit data-is-not-Promise assertion in the torch/Arrow regression test

@CodeRabbit full review

[coderabbitai]

@bbopen: Thank you for addressing the feedback! I'll proceed with a full review of the PR. The changes you've made include:

• Deduplicating sync/async envelope decoding logic via decodeEnvelopeCore
• Adding an explicit assertion that data is not a Promise in the torch/Arrow regression test

I'll review all the changes in the PR now.

✅ Actions performed

Full review triggered.

[bbopen]

@CodeRabbit full review

[coderabbitai]

✅ Actions performed

Full review triggered.