Improved API - Memory manipulation

[DrEverr]

Added values for memory manipulation that could clearly indicate success or error.

Summary by CodeRabbit

• New Features

  • Debug API now signals faults and missing interpreter state via return values instead of exceptions.

• Breaking Changes

  • getMemory(address, length) returns null on failure (missing interpreter or page fault) rather than throwing or returning empty.
  • setMemory(address, data) returns boolean indicating success (true) or failure (false) instead of void.

• Documentation

  • Minor inline docs added for getMemory and setMemory.

[coderabbitai]

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

📥 Commits

Reviewing files that changed from the base of the PR and between 0445a71 and 6467c05.

📒 Files selected for processing (1)

• assembly/api-debugger.ts (1 hunks)

 ________________________________
< GPU-accelerated bug detection. >
 --------------------------------
  \
   \   \
        \ /\
        ( )
      .( o ).

Tip

CodeRabbit can suggest fixes for GitHub Check annotations.

Configure reviews.tools.github-checks in your project's settings in CodeRabbit to adjust the time to wait for GitHub Checks to complete.

Walkthrough

The getMemory and setMemory functions in the debugger API now use explicit return values for fault/error signaling: getMemory returns Uint8Array | null and setMemory returns boolean, replacing prior exception/empty-array behavior.

Changes

Cohort / File(s)	Summary
Debugger API return type updates assembly/api-debugger.ts	getMemory(address, length) return type changed from Uint8Array → Uint8Array | null; returns null on missing interpreter or page fault. setMemory(address, data) return type changed from void → boolean; returns false on missing interpreter or page fault, true on success. Doc comments added and fault propagation moved to return values.

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant DebuggerAPI
  participant Interpreter

  Note over DebuggerAPI,Interpreter: getMemory flow (new)
  Caller->>DebuggerAPI: getMemory(address, length)
  DebuggerAPI->>Interpreter: readMemory(address, length)
  alt success
    Interpreter-->>DebuggerAPI: bytes
    DebuggerAPI-->>Caller: Uint8Array (bytes)
  else fault or no interpreter
    Interpreter-->>DebuggerAPI: fault / no-response
    DebuggerAPI-->>Caller: null
  end

  Note over DebuggerAPI,Interpreter: setMemory flow (new)
  Caller->>DebuggerAPI: setMemory(address, data)
  DebuggerAPI->>Interpreter: writeMemory(address, data)
  alt success
    Interpreter-->>DebuggerAPI: ack
    DebuggerAPI-->>Caller: true
  else fault or no interpreter
    Interpreter-->>DebuggerAPI: fault / no-response
    DebuggerAPI-->>Caller: false
  end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Pay attention to fault handling branches and documentation in assembly/api-debugger.ts.
• Verify callers of getMemory/setMemory handle null/false appropriately.

Possibly related PRs

• Added getMemory to api-debugger #92: Earlier change that introduced getMemory behavior; closely related to the updated return-type and fault semantics here.

Suggested reviewers

• tomusdrw

Poem

🐰 With quiet hops I test each byte,
If faults appear I show the light—
Nulls and falses, honest and clear,
No hidden empties, none to fear.
Debugger tuned, I leap with cheer! 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title "Improved API - Memory manipulation" is partially related to the changeset. It correctly identifies that the changes involve memory manipulation functions, specifically the getMemory and setMemory functions in the API. However, the title lacks specificity about the main improvement—that these functions now return values (null and boolean respectively) to clearly indicate success or error instead of throwing exceptions or returning empty arrays. While the title accurately points to the relevant feature area, it doesn't fully convey the nature of the improvement, which means someone quickly scanning the commit history would understand what area was changed but not understand the key improvement or why it matters.

---

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.}

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a0facae and 69b3264.

📒 Files selected for processing (1)

• assembly/api-debugger.ts (1 hunks)

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-10-16T15:17:10.841Z

Learnt from: tomusdrw
PR: tomusdrw/anan-as#92
File: assembly/api-debugger.ts:170-182
Timestamp: 2025-10-16T15:17:10.841Z
Learning: In assembly/memory.ts, the `bytesRead` method signature is: `bytesRead(faultRes: MaybePageFault, address: u32, destination: Uint8Array, destinationOffset: u32): void`. The last parameter is the offset within the destination buffer to start writing at, not the number of bytes to read. The method reads until the destination buffer is filled from destinationOffset to destination.length.

Applied to files:

• assembly/api-debugger.ts

🧬 Code graph analysis (1)

assembly/api-debugger.ts (3)

assembly/interpreter.ts (1)

• Interpreter (21-200)

assembly/memory.ts (1)

• MaybePageFault (15-19)

assembly/memory-page.ts (1)

• data (42-47)

🔇 Additional comments (1)

assembly/api-debugger.ts (1)

170-182: Breaking API change confirmed, but no internal callers need updating.

The return type changes are intentional breaking changes to the public API:

• getMemory: now returns null on errors (interpreter absent or page fault) instead of empty Uint8Array
• setMemory: now returns boolean to indicate success/failure

Verification shows zero internal callers of these functions in the codebase—they are public API exports for external WebAssembly consumers. The code changes are correct and the error-signaling improvements are sound. External API consumers will need to adapt to these breaking changes, but that is outside the scope of this repository.

[DrEverr]

hmm, why i have so many commits :c

[tomusdrw]

hmm, why i have so many commits :c

Seems that you've added some commits on a branch that was already merged. Please cherry-pick the commits that add new functionality onto a new branch and force push to rename this one or just start a new pr.

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 69b3264 and 0445a71.

📒 Files selected for processing (1)

• assembly/api-debugger.ts (1 hunks)

🧰 Additional context used

🧠 Learnings (2)

📚 Learning: 2025-10-16T15:17:10.864Z

Learnt from: tomusdrw
PR: tomusdrw/anan-as#92
File: assembly/api-debugger.ts:170-182
Timestamp: 2025-10-16T15:17:10.864Z
Learning: In assembly/memory.ts, the `bytesRead` method signature is: `bytesRead(faultRes: MaybePageFault, address: u32, destination: Uint8Array, destinationOffset: u32): void`. The last parameter is the offset within the destination buffer to start writing at, not the number of bytes to read. The method reads until the destination buffer is filled from destinationOffset to destination.length.

Applied to files:

• assembly/api-debugger.ts

📚 Learning: 2025-04-22T15:58:24.284Z

Learnt from: tomusdrw
PR: tomusdrw/anan-as#49
File: assembly/memory.ts:335-335
Timestamp: 2025-04-22T15:58:24.284Z
Learning: In the bytesWrite method, memory writes should be atomic. If a page fault occurs during a multi-page write, no memory should be altered (current implementation allows partial writes).

Applied to files:

• assembly/api-debugger.ts

🧬 Code graph analysis (1)

assembly/api-debugger.ts (3)

assembly/interpreter.ts (1)

• Interpreter (21-200)

assembly/memory.ts (1)

• MaybePageFault (15-19)

assembly/memory-page.ts (1)

• data (42-47)

🪛 GitHub Actions: Node.js CI

assembly/api-debugger.ts

[error] 168-176: Prettier formatting check failed. File content differs from formatting output.

🔇 Additional comments (2)

assembly/api-debugger.ts (2)

170-187: LGTM! Excellent improvement to error signaling.

The changes properly implement explicit null return for fault conditions, making the API clearer and more robust. The JSDoc comment accurately describes the behavior, and the logic correctly handles both missing interpreter and page fault scenarios.

---

189-208: LGTM! Boolean return improves error signaling.

The changes successfully implement explicit boolean return values for fault conditions, making the API clearer. The JSDoc comment accurately describes the behavior, and the function correctly propagates failure states to callers.

Note: The partial write atomicity concern raised in previous reviews remains but is already tracked in issue #51.