fix(stubs): handle None return from void @remote functions

[deanq]

Summary

• handle_response raised ValueError when response.result was None, blocking all void @remote functions (fire-and-forget tasks, setup functions)
• Now falls back through: cloudpickle result -> json_result -> None return

Closes AE-2374

Test plan

• test_handle_response_void_function_returns_none — verifies void functions return None
• test_handle_response_json_result_dict — verifies json_result dict path when result is absent
• test_handle_response_json_result_scalar — verifies json_result scalar returned without deserialization
• test_handle_response_result_takes_priority_over_json_result — verifies cloudpickle result wins when both set
• All 2383 tests passing, 85.48% coverage, live_serverless.py at 100%

[Copilot]

Pull request overview

This PR fixes LiveServerlessStub.handle_response to correctly handle successful remote executions that return no value (void @remote functions), avoiding an erroneous ValueError when result is None.

Changes:

• Update handle_response to return cloudpickle-decoded result when present, otherwise fall back to json_result, otherwise return None.
• Replace the prior test that asserted the buggy ValueError with new tests covering void responses, json_result handling, and precedence when both fields are set.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
src/runpod_flash/stubs/live_serverless.py	Adjusts success-path response decoding to support void functions and json_result fallback.
tests/unit/test_stub_live_serverless.py	Updates unit tests to cover void returns, json_result behavior, and precedence rules.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

[runpod-Henrik]

1. The fix — correct

The three-branch fallback is clean and the logic is right:

# Before
if response.result is None:
    raise ValueError("Response result is None")
return cloudpickle.loads(base64.b64decode(response.result))

# After
if response.result is not None:
    return cloudpickle.loads(base64.b64decode(response.result))
if response.json_result is not None:
    return response.json_result
return None

No objections to the implementation.

---

2. Question: what actually triggers result=None today?

The generic_handler always serializes the return value — including None:

# generic_handler.py:194–198
serialized_result = serialize_result(result)   # cloudpickle.dumps(None) → valid b64
return {"success": True, "result": serialized_result}

serialize_result(None) produces a valid base64 string, so FunctionResponse.result is never None for void functions going through the Flash QB handler. The old raise ValueError("Response result is None") would only fire if job.output arrives without a result key at all — e.g. a raw RunPod worker returning {"success": True}, or an older worker image with a different response format.

Worth a sentence in the commit message or PR body: what concrete scenario produces result=None? If it's a compatibility scenario with non-Flash workers or old images, that should be stated — otherwise the bug description ("void @Remote functions") doesn't match how the server side actually behaves.

---

3. Issue: json_result fallback path is dead code

json_result is defined in FunctionResponse as Optional[Any] = None with the note "used when serialization_format='json'". But grep across all of src/ shows it is never set by any server-side code — not in generic_handler.py, not in lb_handler.py, not in production_wrapper.py. The new fallback branch can never be reached in production.

The tests validate the logic correctly in isolation, but the E2E path doesn't exist yet. If json_result support is planned for a future server-side change, that's fine — but the PR description doesn't mention it, and a reader will wonder why the branch is there. A comment would close this:

if response.json_result is not None:
    # json_result is populated by workers using serialization_format='json'
    return response.json_result

---

4. Test gap: missing the actual production trigger

The four new tests construct FunctionResponse directly. None of them test the path that actually fires in production: FunctionResponse(**job.output) where job.output has no result key. A test like:

def test_handle_response_from_raw_job_output_no_result_key(self, stub):
    """Simulates job.output dict arriving without 'result' key (non-Flash worker)."""
    response = FunctionResponse(**{"success": True})  # result defaults to None
    assert stub.handle_response(response) is None

...would document the actual trigger scenario and prevent regression if FunctionResponse defaults change.

---

Nit

test_handle_response_json_result_scalar passes json_result=42. Worth also testing json_result=0 or json_result="" to confirm falsy-but-not-None scalars return correctly — 0 and "" are not None so they'd pass already, but an explicit test documents the intent.

---

Verdict

PASS with two asks: (1) clarify what production scenario actually produces result=None in the PR description, and (2) add a one-line comment on the json_result branch explaining it's for a future serialization_format='json' server path. The fix itself is correct. Issue 3 is a documentation/readability gap, not a bug.

🤖 Reviewed by Henrik's AI-Powered Bug Finder

[runpod-Henrik]

Follow-up on prior review

Good additions — the scalar (json_result=42) and priority tests close the coverage gaps flagged earlier.

Two items from the prior review still open:

json_result branch comment not added. The branch is dead code in production (no server-side code sets json_result). Without a comment, future readers will wonder why it's there. One line is enough:

if response.json_result is not None:
    # populated by workers using serialization_format='json' (planned, not yet shipped)
    return response.json_result

Test for the actual production trigger still missing. The tests construct FunctionResponse with explicit result=None. The real scenario is a job.output dict arriving without a result key at all:

def test_handle_response_from_raw_job_output_no_result_key(self, stub):
    response = FunctionResponse(**{"success": True})  # result defaults to None
    assert stub.handle_response(response) is None

This documents what actually fires in production and guards against FunctionResponse default changes.

---

Verdict: PASS WITH NITS — fix is correct, neither item above is a blocker.

🤖 Reviewed by Henrik's AI-Powered Bug Finder