fix: 5 critical bugs — TEE randomization, HTTP 402 hint, int dtype, inference None crash, gas estimation - #201

Open
amathxbt wants to merge 27 commits into
OpenGradient:mainfrom
amathxbt:fix/critical-5-bugs-tee-402-dtype-inference-gas
Open

fix: 5 critical bugs — TEE randomization, HTTP 402 hint, int dtype, inference None crash, gas estimation#201
amathxbt wants to merge 27 commits into
OpenGradient:mainfrom
amathxbt:fix/critical-5-bugs-tee-402-dtype-inference-gas

Conversation

@amathxbt

Copy link
Copy Markdown
Contributor

Summary

This PR fixes 5 critical bugs identified from open issues and code audit, spanning TEE selection, LLM error handling, type conversions, inference safety, and gas estimation.


Bug 1 — TEE always picks the same node (closes#200)

File:src/opengradient/client/tee_registry.py

Root cause:get_llm_tee() always returned tees[0], routing 100% of traffic to a single TEE with zero load distribution or failover.

Fix: Replace tees[0] with random.choice(tees) so each LLM() construction independently selects from the full pool of active, registry-verified TEEs.

# Beforereturntees[0]
# Afterselected=random.choice(tees)
logger.debug("Selected TEE %s from %d active LLM proxy TEE(s)", selected.tee_id, len(tees))
returnselected

Bug 2 — HTTP 402 swallowed as cryptic RuntimeError (closes#188)

File:src/opengradient/client/llm.py

Root cause: All HTTP errors (including 402 Payment Required) were caught by except Exception and re-raised as a generic RuntimeError("TEE LLM chat failed: ..."). Users had no idea they needed to call ensure_opg_approval() first.

Fix: Add import httpx and intercept httpx.HTTPStatusErrorbefore the generic handler. When status_code == 402, raise a RuntimeError with an explicit, actionable message pointing to ensure_opg_approval(). Applied to _chat_request(), completion(), and _parse_sse_response() (streaming path).

# New constant_402_HINT= (
"Payment required (HTTP 402): your wallet may have insufficient OPG token allowance. ""Call llm.ensure_opg_approval(opg_amount=<amount>) to approve Permit2 spending ""before making requests. Minimum amount is 0.05 OPG."
)
# In _chat_request / completion / _parse_sse_response:excepthttpx.HTTPStatusErrorase:
ife.response.status_code==402:
raiseRuntimeError(_402_HINT) fromeraiseRuntimeError(f"TEE LLM chat failed: {e}") frome

Bug 3 — Fixed-point int returns np.float32 instead of int (partially closes#103)

File:src/opengradient/client/_conversions.py

Root cause:convert_to_float32(value, decimals) always returned np.float32, even when decimals == 0 (i.e., the original value was an integer). Users expecting integer outputs received np.float32 and had to cast manually.

Fix: Add convert_fixed_point_to_python(value, decimals) that returns int when decimals == 0 and np.float32 otherwise. np.array() then infers the correct dtype (int64 vs float32) automatically. The old convert_to_float32 is kept as a deprecated alias for backward compatibility.

defconvert_fixed_point_to_python(value: int, decimals: int) ->Union[int, np.float32]:
ifdecimals==0:
returnint(value) # ← integer tensor stays integerreturnnp.float32(Decimal(value) / (10**Decimal(decimals)))

Bug 4 — infer() crashes with AttributeError when inference node returns None

File:src/opengradient/client/alpha.py

Root cause:_get_inference_result_from_node() returns None when the node has no result yet. This None was passed directly to convert_to_model_output(), which calls event_data.get("output", {})AttributeError: NoneType object has no attribute get. Additionally, if ModelInferenceEvent logs were empty, parsed_logs[0] caused an IndexError.

Fix:

# Guard 1: empty precompile logsifnotprecompile_logs:
raiseRuntimeError("ModelInferenceEvent not found in transaction logs.")
# Guard 2: None from inference nodeifinference_resultisNone:
raiseRuntimeError(
f"Inference node returned no result for inference ID {inference_id!r}. ""The result may not be available yet — retry after a short delay."
)

Bug 5 — run_workflow() uses hardcoded 30M gas

File:src/opengradient/client/alpha.py

Root cause:run_workflow() hardcoded gas=30000000, unlike infer() which correctly uses estimate_gas(). This is both wasteful (users overpay) and fragile on networks with lower block gas limits.

Fix: Call estimate_gas() first and multiply by 3 for headroom. Fall back to 30M only if estimation itself fails.

try:
estimated_gas=run_function.estimate_gas({"from": self._wallet_account.address})
gas_limit=int(estimated_gas*3)
exceptException:
gas_limit=30000000# fallback

Also replaces the hardcoded timeout=60 in new_workflow() with the INFERENCE_TX_TIMEOUT constant.


Files Changed

FileBugs Fixed
src/opengradient/client/tee_registry.pyBug 1 (TEE randomization)
src/opengradient/client/llm.pyBug 2 (HTTP 402 hint)
src/opengradient/client/_conversions.pyBug 3 (int dtype)
src/opengradient/client/alpha.pyBugs 4 & 5 (None guard + gas)

Related Issues

Closes#200
Closes#188
Partially closes#103

Previously get_llm_tee() always returned tees[0], the first TEE in the
registry list. This caused all clients to hit the same TEE, providing no
load distribution and no resilience when that TEE starts failing.
Fix: use random.choice(tees) so each LLM() construction independently
picks from all currently active TEEs. Successive retries or re-initializations
will therefore naturally spread across the healthy pool.
ClosesOpenGradient#200
Previously any HTTP error from the TEE (including 402 Payment Required)
was silently wrapped into a generic RuntimeError("TEE LLM chat failed: ...").
This caused the confusing traceback seen in issue OpenGradient#188 — the real cause
(insufficient OPG allowance) was buried inside the exception message.
Fix:
- Add `import httpx` and intercept httpx.HTTPStatusError before the
generic `except Exception` handler in both _chat_request() and
completion().
- When status == 402, raise a RuntimeError with a clear, actionable hint
telling the user to call llm.ensure_opg_approval(opg_amount=<amount>).
- Also handle 402 in _parse_sse_response() for the streaming path.
- All other HTTP errors continue to surface as before.
ClosesOpenGradient#188
Previously convert_to_float32() always returned np.float32 regardless of
the decimals field, forcing callers to manually cast integer results
(issue OpenGradient#103 — add proper type conversions from Solidity contract to Python).
Fix:
- Add convert_fixed_point_to_python(value, decimals) that returns int when
decimals == 0 and np.float32 otherwise. np.array() automatically picks
the correct dtype (int64 vs float32) based on the element types.
- Update both convert_to_model_output() and convert_array_to_model_output()
to call the new function.
- Keep convert_to_float32() as a deprecated backward-compatible alias so
any external code that imports it directly continues to work.
Partially closesOpenGradient#103
… in run_workflow
Bug 4 — None crash in infer():
_get_inference_result_from_node() can legitimately return None when the
inference node has no result yet. Previously this None was passed
directly to convert_to_model_output(), which calls event_data.get(),
causing an AttributeError: 'NoneType' object has no attribute 'get'.
Fix: after calling _get_inference_result_from_node(), check for None
and raise a clear RuntimeError with a human-readable message and the
inference_id so callers know what to retry.
Also guard the precompile log fetch: if parsed_logs is empty, raise
RuntimeError instead of crashing with an IndexError on parsed_logs[0].
Bug 5 — hardcoded 30M gas in run_workflow():
run_workflow() built the transaction with gas=30000000 (30 million)
unconditionally. This is wasteful (users overpay for gas) and can
fail on networks with a lower block gas limit.
Fix: call estimate_gas() first (consistent with infer()), then multiply
by 3 for headroom. Fall back to 30M only if estimation itself fails.
Also fixes new_workflow() deploy to use INFERENCE_TX_TIMEOUT constant
instead of the previous hardcoded 60 seconds.
@amathxbt

Copy link
Copy Markdown
ContributorAuthor

Hey @adambalogh 👋 — this PR addresses 5 critical bugs found during a code audit, 3 of which directly close or partially close open issues you and the team have filed:

#BugIssue
1get_llm_tee() always returned tees[0] — no randomization or load distributionCloses #200
2HTTP 402 from TEE was swallowed as a cryptic RuntimeError with no guidanceCloses #188
3Fixed-point values with decimals=0 returned np.float32 instead of intPartially closes #103
4infer() crashed with AttributeError when the inference node returned NoneCode audit
5run_workflow() hardcoded gas=30000000 instead of using estimate_gas()Code audit

All changes are backward-compatible. Happy to adjust anything before review! 🙏

@adambalogh

Copy link
Copy Markdown
Collaborator

Hey @amathxbt thanks for your contributions, I added it to my list to review, will get back to you tomorrow!

@amathxbt

Copy link
Copy Markdown
ContributorAuthor

Hey @amathxbt thanks for your contributions, I added it to my list to review, will get back to you tomorrow!

Ok @adambalogh thank you see you tomorrow

"""LLM chat and completion via TEE-verified execution with x402 payments."""

import json
import json as _json

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the reason for this?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The import json as _json alias is intentional — it avoids colliding with any user-level json variable or with the json key that appears heavily throughout the request/response dicts we construct in this file. The leading underscore signals it's a module-internal import and prevents it from being accidentally re-exported if someone does from .llm import *. No behaviour change; purely a namespace-safety convention.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses several correctness and UX issues in the OpenGradient Python SDK across TEE selection, LLM error reporting, fixed-point conversions, inference robustness, and workflow gas handling.

Changes:

  • Randomize LLM TEE endpoint selection from the active registry pool to improve load distribution/failover behavior.
  • Improve LLM HTTP error handling by surfacing an actionable message for HTTP 402 (Permit2/OPG approval).
  • Add guards in Alpha.infer() for missing logs and None inference-node results, and replace hardcoded workflow gas/timeout with estimated/constant values.
  • Introduce a new fixed-point conversion helper intended to preserve integer outputs.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

FileDescription
src/opengradient/client/tee_registry.pyRandomizes get_llm_tee() selection and adds debug logging.
src/opengradient/client/llm.pyAdds explicit HTTP 402 hint handling for completion/chat/streaming.
src/opengradient/client/_conversions.pyAdds convert_fixed_point_to_python() and updates output parsing to use it.
src/opengradient/client/alpha.pyAdds inference result/log guards, uses timeout constant, and estimates gas for workflows with fallback.

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

Comment on lines +131 to +133
selected = random.choice(tees)
logger.debug(
"Selected TEE %s (endpoint=%s) from %d active LLM proxy TEE(s)",

CopilotAIMar 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change makes get_llm_tee() non-deterministic, which will break/flakify the existing unit test TestGetLlmTee.test_returns_first_active_tee (it asserts the first endpoint). Update the tests to patch random.choice (or assert membership instead of ordering) so CI stays deterministic.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to fix this

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed ✅ — committed in tests/tee_registry_test.py. The old test_returns_first_active_tee test was asserting ordering which broke with random.choice. Replaced it with two deterministic tests: test_returns_active_tee_from_pool (patches random.choice via unittest.mock.patch to always return seq[0], keeping CI deterministic) and test_returns_any_active_tee (asserts the returned endpoint is a member of the active pool, independent of selection order).

Comment on lines +39 to +56
def convert_fixed_point_to_python(value: int, decimals: int) -> Union[int, np.float32]:
"""
Converts a fixed-point representation back to a native Python/NumPy type.

Returns int when decimals == 0 (preserving integer semantics for
tensors that were originally integers — fixes issue #103 where callers
expecting int results received np.float32 and had to cast manually).
Returns np.float32 for all other cases.

Args:
value: The integer significand stored on-chain.
decimals: The scale factor exponent (value / 10**decimals).

Returns:
int if decimals == 0, np.float32 otherwise.
"""
if decimals == 0:
return int(value)

CopilotAIMar 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Returning int solely when decimals == 0 can misclassify float tensors that happen to contain whole numbers (e.g., 1.0 normalizes to decimals==0). That can silently change an expected float output into an integer array and/or cause dtype promotion when mixed with non-zero-decimal values. Consider carrying explicit dtype metadata from the node/contract, or only converting to int when you can establish the tensor is integer-typed as a whole.

Suggested change
defconvert_fixed_point_to_python(value: int, decimals: int) ->Union[int, np.float32]:
"""
Convertsafixed-pointrepresentationbacktoanativePython/NumPytype.
Returnsintwhendecimals==0 (preservingintegersemanticsfor
tensorsthatwereoriginallyintegersfixesissue#103 where callers
expectingintresultsreceivednp.float32andhadtocastmanually).
Returnsnp.float32forallothercases.
Args:
value: Theintegersignificandstoredon-chain.
decimals: Thescalefactorexponent (value/10**decimals).
Returns:
intifdecimals==0, np.float32otherwise.
"""
ifdecimals==0:
returnint(value)
defconvert_fixed_point_to_python(value: int, decimals: int) ->np.float32:
"""
Convertsafixed-pointrepresentationbacktoaNumPyfloat32.
Thisfunctionisintentionallytype-stableandalwaysreturnsnp.float32,
regardlessofthevalueof`decimals`. Callersthatrequireinteger
semanticsshouldperformanexplicitcast (e.g., int(...)) basedon
theirowndtypemetadataorapplicationlogic.
Args:
value: Theintegersignificandstoredon-chain.
decimals: Thescalefactorexponent (value/10**decimals).
Returns:
np.float32correspondingto`value / 10**decimals`.
"""

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there are reason we are mixing up float and int? might make the handling of results more difficult

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed ✅ — committed in src/opengradient/client/_conversions.py. convert_fixed_point_to_python is now type-stable: it always returns np.float32 regardless of decimals, matching Copilot's suggestion. The int-when-decimals==0 branch has been removed. The return type annotation is updated from Union[int, np.float32]np.float32. Callers that genuinely need integer semantics can do an explicit int(...) cast based on their own dtype metadata.

Comment on lines +87 to +91
logging.debug(f" Converting {tensor_data} to np array")
tensor_data = np.array(tensor_data)

if isinstance(tensor_data, (str, int, float)):
logging.debug(f"\tConverting single entry {tensor_data} to a list")
logging.debug(f" Converting single entry {tensor_data} to a list")

CopilotAIMar 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This debug string includes a literal tab character inside the f-string. Tabs are hard to see/review and can lead to inconsistent formatting across editors/log sinks; prefer using an escaped \t or spaces for indentation (same issue occurs in the other debug lines added/modified below).

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please fix

@amathxbtamathxbtMar 26, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed ✅ committed in src/opengradient/client/_conversions.py. Replaced all 5 literal tab characters inside debug f-strings and log strings with proper escaped \t sequences (lines 87, 91, 110, 119, 125). No more invisible tabs in the source.

Comment threadsrc/opengradient/client/alpha.py Outdated
Comment threadsrc/opengradient/client/llm.py
@adambalogh

Copy link
Copy Markdown
Collaborator

@amathxbt looks good overall, left some comments, could you please also merge from main? i think there are changes in the llm.py that need to be resolved

@amathxbt

Copy link
Copy Markdown
ContributorAuthor

Done @adambalogh ✅ — merged main into llm.py (commit 8896eac).

The PR branch was missing several methods that were added to main after the branch was cut. I preserved all of them and kept the PR's bug fixes layered on top:

Restored from main:

  • _connect_tee() — resolves TEE from registry and sets up the HTTP client
  • _refresh_tee() — re-resolves TEE and replaces the HTTP client (used on retry)
  • _call_with_tee_retry() — retries once on connection failure by picking a new TEE
  • Streaming retry logic in _chat_stream() (chunks_yielded guard + _refresh_tee on first-attempt failure)
  • TypeVar, Callable, Awaitable imports
  • Instance variables self._rpc_url, self._tee_registry_address, self._llm_server_url

PR fixes kept on top:

  • import json as _json + all json.loads / json.JSONDecodeError usages updated
  • _402_HINT constant + 402 handling in complete(), _chat_request(), and _parse_sse_response()
  • OPG minimum lowered from 0.10.05

@amathxbt

Copy link
Copy Markdown
ContributorAuthor

Merged upstream main into the branch and resolved all conflicts:

  • src/opengradient/client/llm.py (SHA af38386): rebased on the latest main (which added TEE retry/refresh logic, _call_with_tee_retry, _connect_tee, _refresh_tee, new SSL/cert-rotation handling) while retaining all PR-specific changes — import json as _json, _402_HINT constant, HTTP 402 error handling in complete(), chat(), and _parse_sse_response(), and the 0.05 OPG minimum.

  • tests/llm_test.py: already contained the new TestTeeRetry*, TestRefreshTeeAndReset, and TestTeeCertRotation classes from main. The PR's 402-hint tests (test_http_402_raises_hint × 2, test_stream_402_raises_hint) and the FakeHTTPClient status-code fix were already in place.

The PR is now conflict-free (mergeable: true).

@amathxbt

Copy link
Copy Markdown
ContributorAuthor

@adambalogh please can you done this it's been long time no more comments

@amathxbt
amathxbt requested a review from adambaloghApril 4, 2026 22:53
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants

@amathxbt@adambalogh
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all \u003cpre\u003e\u003ccode\u003e blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks"); } } catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); } })(); (function(){ try { var __m = "github.com"; var __re = new RegExp('^' + "github\\.com" + '
Skip to content

fix: 5 critical bugs — TEE randomization, HTTP 402 hint, int dtype, inference None crash, gas estimation - #201

Open
amathxbt wants to merge 27 commits into
OpenGradient:mainfrom
amathxbt:fix/critical-5-bugs-tee-402-dtype-inference-gas
Open

fix: 5 critical bugs — TEE randomization, HTTP 402 hint, int dtype, inference None crash, gas estimation#201
amathxbt wants to merge 27 commits into
OpenGradient:mainfrom
amathxbt:fix/critical-5-bugs-tee-402-dtype-inference-gas

Conversation

@amathxbt

Copy link
Copy Markdown
Contributor

Summary

This PR fixes 5 critical bugs identified from open issues and code audit, spanning TEE selection, LLM error handling, type conversions, inference safety, and gas estimation.


Bug 1 — TEE always picks the same node (closes#200)

File:src/opengradient/client/tee_registry.py

Root cause:get_llm_tee() always returned tees[0], routing 100% of traffic to a single TEE with zero load distribution or failover.

Fix: Replace tees[0] with random.choice(tees) so each LLM() construction independently selects from the full pool of active, registry-verified TEEs.

# Beforereturntees[0]
# Afterselected=random.choice(tees)
logger.debug("Selected TEE %s from %d active LLM proxy TEE(s)", selected.tee_id, len(tees))
returnselected

Bug 2 — HTTP 402 swallowed as cryptic RuntimeError (closes#188)

File:src/opengradient/client/llm.py

Root cause: All HTTP errors (including 402 Payment Required) were caught by except Exception and re-raised as a generic RuntimeError("TEE LLM chat failed: ..."). Users had no idea they needed to call ensure_opg_approval() first.

Fix: Add import httpx and intercept httpx.HTTPStatusErrorbefore the generic handler. When status_code == 402, raise a RuntimeError with an explicit, actionable message pointing to ensure_opg_approval(). Applied to _chat_request(), completion(), and _parse_sse_response() (streaming path).

# New constant_402_HINT= (
"Payment required (HTTP 402): your wallet may have insufficient OPG token allowance. ""Call llm.ensure_opg_approval(opg_amount=<amount>) to approve Permit2 spending ""before making requests. Minimum amount is 0.05 OPG."
)
# In _chat_request / completion / _parse_sse_response:excepthttpx.HTTPStatusErrorase:
ife.response.status_code==402:
raiseRuntimeError(_402_HINT) fromeraiseRuntimeError(f"TEE LLM chat failed: {e}") frome

Bug 3 — Fixed-point int returns np.float32 instead of int (partially closes#103)

File:src/opengradient/client/_conversions.py

Root cause:convert_to_float32(value, decimals) always returned np.float32, even when decimals == 0 (i.e., the original value was an integer). Users expecting integer outputs received np.float32 and had to cast manually.

Fix: Add convert_fixed_point_to_python(value, decimals) that returns int when decimals == 0 and np.float32 otherwise. np.array() then infers the correct dtype (int64 vs float32) automatically. The old convert_to_float32 is kept as a deprecated alias for backward compatibility.

defconvert_fixed_point_to_python(value: int, decimals: int) ->Union[int, np.float32]:
ifdecimals==0:
returnint(value) # ← integer tensor stays integerreturnnp.float32(Decimal(value) / (10**Decimal(decimals)))

Bug 4 — infer() crashes with AttributeError when inference node returns None

File:src/opengradient/client/alpha.py

Root cause:_get_inference_result_from_node() returns None when the node has no result yet. This None was passed directly to convert_to_model_output(), which calls event_data.get("output", {})AttributeError: NoneType object has no attribute get. Additionally, if ModelInferenceEvent logs were empty, parsed_logs[0] caused an IndexError.

Fix:

# Guard 1: empty precompile logsifnotprecompile_logs:
raiseRuntimeError("ModelInferenceEvent not found in transaction logs.")
# Guard 2: None from inference nodeifinference_resultisNone:
raiseRuntimeError(
f"Inference node returned no result for inference ID {inference_id!r}. ""The result may not be available yet — retry after a short delay."
)

Bug 5 — run_workflow() uses hardcoded 30M gas

File:src/opengradient/client/alpha.py

Root cause:run_workflow() hardcoded gas=30000000, unlike infer() which correctly uses estimate_gas(). This is both wasteful (users overpay) and fragile on networks with lower block gas limits.

Fix: Call estimate_gas() first and multiply by 3 for headroom. Fall back to 30M only if estimation itself fails.

try:
estimated_gas=run_function.estimate_gas({"from": self._wallet_account.address})
gas_limit=int(estimated_gas*3)
exceptException:
gas_limit=30000000# fallback

Also replaces the hardcoded timeout=60 in new_workflow() with the INFERENCE_TX_TIMEOUT constant.


Files Changed

FileBugs Fixed
src/opengradient/client/tee_registry.pyBug 1 (TEE randomization)
src/opengradient/client/llm.pyBug 2 (HTTP 402 hint)
src/opengradient/client/_conversions.pyBug 3 (int dtype)
src/opengradient/client/alpha.pyBugs 4 & 5 (None guard + gas)

Related Issues

Closes#200
Closes#188
Partially closes#103

Previously get_llm_tee() always returned tees[0], the first TEE in the
registry list. This caused all clients to hit the same TEE, providing no
load distribution and no resilience when that TEE starts failing.
Fix: use random.choice(tees) so each LLM() construction independently
picks from all currently active TEEs. Successive retries or re-initializations
will therefore naturally spread across the healthy pool.
ClosesOpenGradient#200
Previously any HTTP error from the TEE (including 402 Payment Required)
was silently wrapped into a generic RuntimeError("TEE LLM chat failed: ...").
This caused the confusing traceback seen in issue OpenGradient#188 — the real cause
(insufficient OPG allowance) was buried inside the exception message.
Fix:
- Add `import httpx` and intercept httpx.HTTPStatusError before the
generic `except Exception` handler in both _chat_request() and
completion().
- When status == 402, raise a RuntimeError with a clear, actionable hint
telling the user to call llm.ensure_opg_approval(opg_amount=<amount>).
- Also handle 402 in _parse_sse_response() for the streaming path.
- All other HTTP errors continue to surface as before.
ClosesOpenGradient#188
Previously convert_to_float32() always returned np.float32 regardless of
the decimals field, forcing callers to manually cast integer results
(issue OpenGradient#103 — add proper type conversions from Solidity contract to Python).
Fix:
- Add convert_fixed_point_to_python(value, decimals) that returns int when
decimals == 0 and np.float32 otherwise. np.array() automatically picks
the correct dtype (int64 vs float32) based on the element types.
- Update both convert_to_model_output() and convert_array_to_model_output()
to call the new function.
- Keep convert_to_float32() as a deprecated backward-compatible alias so
any external code that imports it directly continues to work.
Partially closesOpenGradient#103
… in run_workflow
Bug 4 — None crash in infer():
_get_inference_result_from_node() can legitimately return None when the
inference node has no result yet. Previously this None was passed
directly to convert_to_model_output(), which calls event_data.get(),
causing an AttributeError: 'NoneType' object has no attribute 'get'.
Fix: after calling _get_inference_result_from_node(), check for None
and raise a clear RuntimeError with a human-readable message and the
inference_id so callers know what to retry.
Also guard the precompile log fetch: if parsed_logs is empty, raise
RuntimeError instead of crashing with an IndexError on parsed_logs[0].
Bug 5 — hardcoded 30M gas in run_workflow():
run_workflow() built the transaction with gas=30000000 (30 million)
unconditionally. This is wasteful (users overpay for gas) and can
fail on networks with a lower block gas limit.
Fix: call estimate_gas() first (consistent with infer()), then multiply
by 3 for headroom. Fall back to 30M only if estimation itself fails.
Also fixes new_workflow() deploy to use INFERENCE_TX_TIMEOUT constant
instead of the previous hardcoded 60 seconds.
@amathxbt

Copy link
Copy Markdown
ContributorAuthor

Hey @adambalogh 👋 — this PR addresses 5 critical bugs found during a code audit, 3 of which directly close or partially close open issues you and the team have filed:

#BugIssue
1get_llm_tee() always returned tees[0] — no randomization or load distributionCloses #200
2HTTP 402 from TEE was swallowed as a cryptic RuntimeError with no guidanceCloses #188
3Fixed-point values with decimals=0 returned np.float32 instead of intPartially closes #103
4infer() crashed with AttributeError when the inference node returned NoneCode audit
5run_workflow() hardcoded gas=30000000 instead of using estimate_gas()Code audit

All changes are backward-compatible. Happy to adjust anything before review! 🙏

@adambalogh

Copy link
Copy Markdown
Collaborator

Hey @amathxbt thanks for your contributions, I added it to my list to review, will get back to you tomorrow!

@amathxbt

Copy link
Copy Markdown
ContributorAuthor

Hey @amathxbt thanks for your contributions, I added it to my list to review, will get back to you tomorrow!

Ok @adambalogh thank you see you tomorrow

"""LLM chat and completion via TEE-verified execution with x402 payments."""

import json
import json as _json

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the reason for this?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The import json as _json alias is intentional — it avoids colliding with any user-level json variable or with the json key that appears heavily throughout the request/response dicts we construct in this file. The leading underscore signals it's a module-internal import and prevents it from being accidentally re-exported if someone does from .llm import *. No behaviour change; purely a namespace-safety convention.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses several correctness and UX issues in the OpenGradient Python SDK across TEE selection, LLM error reporting, fixed-point conversions, inference robustness, and workflow gas handling.

Changes:

  • Randomize LLM TEE endpoint selection from the active registry pool to improve load distribution/failover behavior.
  • Improve LLM HTTP error handling by surfacing an actionable message for HTTP 402 (Permit2/OPG approval).
  • Add guards in Alpha.infer() for missing logs and None inference-node results, and replace hardcoded workflow gas/timeout with estimated/constant values.
  • Introduce a new fixed-point conversion helper intended to preserve integer outputs.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

FileDescription
src/opengradient/client/tee_registry.pyRandomizes get_llm_tee() selection and adds debug logging.
src/opengradient/client/llm.pyAdds explicit HTTP 402 hint handling for completion/chat/streaming.
src/opengradient/client/_conversions.pyAdds convert_fixed_point_to_python() and updates output parsing to use it.
src/opengradient/client/alpha.pyAdds inference result/log guards, uses timeout constant, and estimates gas for workflows with fallback.

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

Comment on lines +131 to +133
selected = random.choice(tees)
logger.debug(
"Selected TEE %s (endpoint=%s) from %d active LLM proxy TEE(s)",

CopilotAIMar 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change makes get_llm_tee() non-deterministic, which will break/flakify the existing unit test TestGetLlmTee.test_returns_first_active_tee (it asserts the first endpoint). Update the tests to patch random.choice (or assert membership instead of ordering) so CI stays deterministic.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to fix this

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed ✅ — committed in tests/tee_registry_test.py. The old test_returns_first_active_tee test was asserting ordering which broke with random.choice. Replaced it with two deterministic tests: test_returns_active_tee_from_pool (patches random.choice via unittest.mock.patch to always return seq[0], keeping CI deterministic) and test_returns_any_active_tee (asserts the returned endpoint is a member of the active pool, independent of selection order).

Comment on lines +39 to +56
def convert_fixed_point_to_python(value: int, decimals: int) -> Union[int, np.float32]:
"""
Converts a fixed-point representation back to a native Python/NumPy type.

Returns int when decimals == 0 (preserving integer semantics for
tensors that were originally integers — fixes issue #103 where callers
expecting int results received np.float32 and had to cast manually).
Returns np.float32 for all other cases.

Args:
value: The integer significand stored on-chain.
decimals: The scale factor exponent (value / 10**decimals).

Returns:
int if decimals == 0, np.float32 otherwise.
"""
if decimals == 0:
return int(value)

CopilotAIMar 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Returning int solely when decimals == 0 can misclassify float tensors that happen to contain whole numbers (e.g., 1.0 normalizes to decimals==0). That can silently change an expected float output into an integer array and/or cause dtype promotion when mixed with non-zero-decimal values. Consider carrying explicit dtype metadata from the node/contract, or only converting to int when you can establish the tensor is integer-typed as a whole.

Suggested change
defconvert_fixed_point_to_python(value: int, decimals: int) ->Union[int, np.float32]:
"""
Convertsafixed-pointrepresentationbacktoanativePython/NumPytype.
Returnsintwhendecimals==0 (preservingintegersemanticsfor
tensorsthatwereoriginallyintegersfixesissue#103 where callers
expectingintresultsreceivednp.float32andhadtocastmanually).
Returnsnp.float32forallothercases.
Args:
value: Theintegersignificandstoredon-chain.
decimals: Thescalefactorexponent (value/10**decimals).
Returns:
intifdecimals==0, np.float32otherwise.
"""
ifdecimals==0:
returnint(value)
defconvert_fixed_point_to_python(value: int, decimals: int) ->np.float32:
"""
Convertsafixed-pointrepresentationbacktoaNumPyfloat32.
Thisfunctionisintentionallytype-stableandalwaysreturnsnp.float32,
regardlessofthevalueof`decimals`. Callersthatrequireinteger
semanticsshouldperformanexplicitcast (e.g., int(...)) basedon
theirowndtypemetadataorapplicationlogic.
Args:
value: Theintegersignificandstoredon-chain.
decimals: Thescalefactorexponent (value/10**decimals).
Returns:
np.float32correspondingto`value / 10**decimals`.
"""

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there are reason we are mixing up float and int? might make the handling of results more difficult

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed ✅ — committed in src/opengradient/client/_conversions.py. convert_fixed_point_to_python is now type-stable: it always returns np.float32 regardless of decimals, matching Copilot's suggestion. The int-when-decimals==0 branch has been removed. The return type annotation is updated from Union[int, np.float32]np.float32. Callers that genuinely need integer semantics can do an explicit int(...) cast based on their own dtype metadata.

Comment on lines +87 to +91
logging.debug(f" Converting {tensor_data} to np array")
tensor_data = np.array(tensor_data)

if isinstance(tensor_data, (str, int, float)):
logging.debug(f"\tConverting single entry {tensor_data} to a list")
logging.debug(f" Converting single entry {tensor_data} to a list")

CopilotAIMar 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This debug string includes a literal tab character inside the f-string. Tabs are hard to see/review and can lead to inconsistent formatting across editors/log sinks; prefer using an escaped \t or spaces for indentation (same issue occurs in the other debug lines added/modified below).

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please fix

@amathxbtamathxbtMar 26, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed ✅ committed in src/opengradient/client/_conversions.py. Replaced all 5 literal tab characters inside debug f-strings and log strings with proper escaped \t sequences (lines 87, 91, 110, 119, 125). No more invisible tabs in the source.

Comment threadsrc/opengradient/client/alpha.py Outdated
Comment threadsrc/opengradient/client/llm.py
@adambalogh

Copy link
Copy Markdown
Collaborator

@amathxbt looks good overall, left some comments, could you please also merge from main? i think there are changes in the llm.py that need to be resolved

@amathxbt

Copy link
Copy Markdown
ContributorAuthor

Done @adambalogh ✅ — merged main into llm.py (commit 8896eac).

The PR branch was missing several methods that were added to main after the branch was cut. I preserved all of them and kept the PR's bug fixes layered on top:

Restored from main:

  • _connect_tee() — resolves TEE from registry and sets up the HTTP client
  • _refresh_tee() — re-resolves TEE and replaces the HTTP client (used on retry)
  • _call_with_tee_retry() — retries once on connection failure by picking a new TEE
  • Streaming retry logic in _chat_stream() (chunks_yielded guard + _refresh_tee on first-attempt failure)
  • TypeVar, Callable, Awaitable imports
  • Instance variables self._rpc_url, self._tee_registry_address, self._llm_server_url

PR fixes kept on top:

  • import json as _json + all json.loads / json.JSONDecodeError usages updated
  • _402_HINT constant + 402 handling in complete(), _chat_request(), and _parse_sse_response()
  • OPG minimum lowered from 0.10.05

@amathxbt

Copy link
Copy Markdown
ContributorAuthor

Merged upstream main into the branch and resolved all conflicts:

  • src/opengradient/client/llm.py (SHA af38386): rebased on the latest main (which added TEE retry/refresh logic, _call_with_tee_retry, _connect_tee, _refresh_tee, new SSL/cert-rotation handling) while retaining all PR-specific changes — import json as _json, _402_HINT constant, HTTP 402 error handling in complete(), chat(), and _parse_sse_response(), and the 0.05 OPG minimum.

  • tests/llm_test.py: already contained the new TestTeeRetry*, TestRefreshTeeAndReset, and TestTeeCertRotation classes from main. The PR's 402-hint tests (test_http_402_raises_hint × 2, test_stream_402_raises_hint) and the FakeHTTPClient status-code fix were already in place.

The PR is now conflict-free (mergeable: true).

@amathxbt

Copy link
Copy Markdown
ContributorAuthor

@adambalogh please can you done this it's been long time no more comments

@amathxbt
amathxbt requested a review from adambaloghApril 4, 2026 22:53
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants

@amathxbt@adambalogh
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: 5 critical bugs — TEE randomization, HTTP 402 hint, int dtype, inference None crash, gas estimation - #201

Open
amathxbt wants to merge 27 commits into
OpenGradient:mainfrom
amathxbt:fix/critical-5-bugs-tee-402-dtype-inference-gas
Open

fix: 5 critical bugs — TEE randomization, HTTP 402 hint, int dtype, inference None crash, gas estimation#201
amathxbt wants to merge 27 commits into
OpenGradient:mainfrom
amathxbt:fix/critical-5-bugs-tee-402-dtype-inference-gas

Conversation

@amathxbt

Copy link
Copy Markdown
Contributor

Summary

This PR fixes 5 critical bugs identified from open issues and code audit, spanning TEE selection, LLM error handling, type conversions, inference safety, and gas estimation.


Bug 1 — TEE always picks the same node (closes#200)

File:src/opengradient/client/tee_registry.py

Root cause:get_llm_tee() always returned tees[0], routing 100% of traffic to a single TEE with zero load distribution or failover.

Fix: Replace tees[0] with random.choice(tees) so each LLM() construction independently selects from the full pool of active, registry-verified TEEs.

# Beforereturntees[0]
# Afterselected=random.choice(tees)
logger.debug("Selected TEE %s from %d active LLM proxy TEE(s)", selected.tee_id, len(tees))
returnselected

Bug 2 — HTTP 402 swallowed as cryptic RuntimeError (closes#188)

File:src/opengradient/client/llm.py

Root cause: All HTTP errors (including 402 Payment Required) were caught by except Exception and re-raised as a generic RuntimeError("TEE LLM chat failed: ..."). Users had no idea they needed to call ensure_opg_approval() first.

Fix: Add import httpx and intercept httpx.HTTPStatusErrorbefore the generic handler. When status_code == 402, raise a RuntimeError with an explicit, actionable message pointing to ensure_opg_approval(). Applied to _chat_request(), completion(), and _parse_sse_response() (streaming path).

# New constant_402_HINT= (
"Payment required (HTTP 402): your wallet may have insufficient OPG token allowance. ""Call llm.ensure_opg_approval(opg_amount=<amount>) to approve Permit2 spending ""before making requests. Minimum amount is 0.05 OPG."
)
# In _chat_request / completion / _parse_sse_response:excepthttpx.HTTPStatusErrorase:
ife.response.status_code==402:
raiseRuntimeError(_402_HINT) fromeraiseRuntimeError(f"TEE LLM chat failed: {e}") frome

Bug 3 — Fixed-point int returns np.float32 instead of int (partially closes#103)

File:src/opengradient/client/_conversions.py

Root cause:convert_to_float32(value, decimals) always returned np.float32, even when decimals == 0 (i.e., the original value was an integer). Users expecting integer outputs received np.float32 and had to cast manually.

Fix: Add convert_fixed_point_to_python(value, decimals) that returns int when decimals == 0 and np.float32 otherwise. np.array() then infers the correct dtype (int64 vs float32) automatically. The old convert_to_float32 is kept as a deprecated alias for backward compatibility.

defconvert_fixed_point_to_python(value: int, decimals: int) ->Union[int, np.float32]:
ifdecimals==0:
returnint(value) # ← integer tensor stays integerreturnnp.float32(Decimal(value) / (10**Decimal(decimals)))

Bug 4 — infer() crashes with AttributeError when inference node returns None

File:src/opengradient/client/alpha.py

Root cause:_get_inference_result_from_node() returns None when the node has no result yet. This None was passed directly to convert_to_model_output(), which calls event_data.get("output", {})AttributeError: NoneType object has no attribute get. Additionally, if ModelInferenceEvent logs were empty, parsed_logs[0] caused an IndexError.

Fix:

# Guard 1: empty precompile logsifnotprecompile_logs:
raiseRuntimeError("ModelInferenceEvent not found in transaction logs.")
# Guard 2: None from inference nodeifinference_resultisNone:
raiseRuntimeError(
f"Inference node returned no result for inference ID {inference_id!r}. ""The result may not be available yet — retry after a short delay."
)

Bug 5 — run_workflow() uses hardcoded 30M gas

File:src/opengradient/client/alpha.py

Root cause:run_workflow() hardcoded gas=30000000, unlike infer() which correctly uses estimate_gas(). This is both wasteful (users overpay) and fragile on networks with lower block gas limits.

Fix: Call estimate_gas() first and multiply by 3 for headroom. Fall back to 30M only if estimation itself fails.

try:
estimated_gas=run_function.estimate_gas({"from": self._wallet_account.address})
gas_limit=int(estimated_gas*3)
exceptException:
gas_limit=30000000# fallback

Also replaces the hardcoded timeout=60 in new_workflow() with the INFERENCE_TX_TIMEOUT constant.


Files Changed

FileBugs Fixed
src/opengradient/client/tee_registry.pyBug 1 (TEE randomization)
src/opengradient/client/llm.pyBug 2 (HTTP 402 hint)
src/opengradient/client/_conversions.pyBug 3 (int dtype)
src/opengradient/client/alpha.pyBugs 4 & 5 (None guard + gas)

Related Issues

Closes#200
Closes#188
Partially closes#103

Previously get_llm_tee() always returned tees[0], the first TEE in the
registry list. This caused all clients to hit the same TEE, providing no
load distribution and no resilience when that TEE starts failing.
Fix: use random.choice(tees) so each LLM() construction independently
picks from all currently active TEEs. Successive retries or re-initializations
will therefore naturally spread across the healthy pool.
ClosesOpenGradient#200
Previously any HTTP error from the TEE (including 402 Payment Required)
was silently wrapped into a generic RuntimeError("TEE LLM chat failed: ...").
This caused the confusing traceback seen in issue OpenGradient#188 — the real cause
(insufficient OPG allowance) was buried inside the exception message.
Fix:
- Add `import httpx` and intercept httpx.HTTPStatusError before the
generic `except Exception` handler in both _chat_request() and
completion().
- When status == 402, raise a RuntimeError with a clear, actionable hint
telling the user to call llm.ensure_opg_approval(opg_amount=<amount>).
- Also handle 402 in _parse_sse_response() for the streaming path.
- All other HTTP errors continue to surface as before.
ClosesOpenGradient#188
Previously convert_to_float32() always returned np.float32 regardless of
the decimals field, forcing callers to manually cast integer results
(issue OpenGradient#103 — add proper type conversions from Solidity contract to Python).
Fix:
- Add convert_fixed_point_to_python(value, decimals) that returns int when
decimals == 0 and np.float32 otherwise. np.array() automatically picks
the correct dtype (int64 vs float32) based on the element types.
- Update both convert_to_model_output() and convert_array_to_model_output()
to call the new function.
- Keep convert_to_float32() as a deprecated backward-compatible alias so
any external code that imports it directly continues to work.
Partially closesOpenGradient#103
… in run_workflow
Bug 4 — None crash in infer():
_get_inference_result_from_node() can legitimately return None when the
inference node has no result yet. Previously this None was passed
directly to convert_to_model_output(), which calls event_data.get(),
causing an AttributeError: 'NoneType' object has no attribute 'get'.
Fix: after calling _get_inference_result_from_node(), check for None
and raise a clear RuntimeError with a human-readable message and the
inference_id so callers know what to retry.
Also guard the precompile log fetch: if parsed_logs is empty, raise
RuntimeError instead of crashing with an IndexError on parsed_logs[0].
Bug 5 — hardcoded 30M gas in run_workflow():
run_workflow() built the transaction with gas=30000000 (30 million)
unconditionally. This is wasteful (users overpay for gas) and can
fail on networks with a lower block gas limit.
Fix: call estimate_gas() first (consistent with infer()), then multiply
by 3 for headroom. Fall back to 30M only if estimation itself fails.
Also fixes new_workflow() deploy to use INFERENCE_TX_TIMEOUT constant
instead of the previous hardcoded 60 seconds.
@amathxbt

Copy link
Copy Markdown
ContributorAuthor

Hey @adambalogh 👋 — this PR addresses 5 critical bugs found during a code audit, 3 of which directly close or partially close open issues you and the team have filed:

#BugIssue
1get_llm_tee() always returned tees[0] — no randomization or load distributionCloses #200
2HTTP 402 from TEE was swallowed as a cryptic RuntimeError with no guidanceCloses #188
3Fixed-point values with decimals=0 returned np.float32 instead of intPartially closes #103
4infer() crashed with AttributeError when the inference node returned NoneCode audit
5run_workflow() hardcoded gas=30000000 instead of using estimate_gas()Code audit

All changes are backward-compatible. Happy to adjust anything before review! 🙏

@adambalogh

Copy link
Copy Markdown
Collaborator

Hey @amathxbt thanks for your contributions, I added it to my list to review, will get back to you tomorrow!

@amathxbt

Copy link
Copy Markdown
ContributorAuthor

Hey @amathxbt thanks for your contributions, I added it to my list to review, will get back to you tomorrow!

Ok @adambalogh thank you see you tomorrow

"""LLM chat and completion via TEE-verified execution with x402 payments."""

import json
import json as _json

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the reason for this?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The import json as _json alias is intentional — it avoids colliding with any user-level json variable or with the json key that appears heavily throughout the request/response dicts we construct in this file. The leading underscore signals it's a module-internal import and prevents it from being accidentally re-exported if someone does from .llm import *. No behaviour change; purely a namespace-safety convention.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses several correctness and UX issues in the OpenGradient Python SDK across TEE selection, LLM error reporting, fixed-point conversions, inference robustness, and workflow gas handling.

Changes:

  • Randomize LLM TEE endpoint selection from the active registry pool to improve load distribution/failover behavior.
  • Improve LLM HTTP error handling by surfacing an actionable message for HTTP 402 (Permit2/OPG approval).
  • Add guards in Alpha.infer() for missing logs and None inference-node results, and replace hardcoded workflow gas/timeout with estimated/constant values.
  • Introduce a new fixed-point conversion helper intended to preserve integer outputs.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

FileDescription
src/opengradient/client/tee_registry.pyRandomizes get_llm_tee() selection and adds debug logging.
src/opengradient/client/llm.pyAdds explicit HTTP 402 hint handling for completion/chat/streaming.
src/opengradient/client/_conversions.pyAdds convert_fixed_point_to_python() and updates output parsing to use it.
src/opengradient/client/alpha.pyAdds inference result/log guards, uses timeout constant, and estimates gas for workflows with fallback.

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

Comment on lines +131 to +133
selected = random.choice(tees)
logger.debug(
"Selected TEE %s (endpoint=%s) from %d active LLM proxy TEE(s)",

CopilotAIMar 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change makes get_llm_tee() non-deterministic, which will break/flakify the existing unit test TestGetLlmTee.test_returns_first_active_tee (it asserts the first endpoint). Update the tests to patch random.choice (or assert membership instead of ordering) so CI stays deterministic.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to fix this

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed ✅ — committed in tests/tee_registry_test.py. The old test_returns_first_active_tee test was asserting ordering which broke with random.choice. Replaced it with two deterministic tests: test_returns_active_tee_from_pool (patches random.choice via unittest.mock.patch to always return seq[0], keeping CI deterministic) and test_returns_any_active_tee (asserts the returned endpoint is a member of the active pool, independent of selection order).

Comment on lines +39 to +56
def convert_fixed_point_to_python(value: int, decimals: int) -> Union[int, np.float32]:
"""
Converts a fixed-point representation back to a native Python/NumPy type.

Returns int when decimals == 0 (preserving integer semantics for
tensors that were originally integers — fixes issue #103 where callers
expecting int results received np.float32 and had to cast manually).
Returns np.float32 for all other cases.

Args:
value: The integer significand stored on-chain.
decimals: The scale factor exponent (value / 10**decimals).

Returns:
int if decimals == 0, np.float32 otherwise.
"""
if decimals == 0:
return int(value)

CopilotAIMar 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Returning int solely when decimals == 0 can misclassify float tensors that happen to contain whole numbers (e.g., 1.0 normalizes to decimals==0). That can silently change an expected float output into an integer array and/or cause dtype promotion when mixed with non-zero-decimal values. Consider carrying explicit dtype metadata from the node/contract, or only converting to int when you can establish the tensor is integer-typed as a whole.

Suggested change
defconvert_fixed_point_to_python(value: int, decimals: int) ->Union[int, np.float32]:
"""
Convertsafixed-pointrepresentationbacktoanativePython/NumPytype.
Returnsintwhendecimals==0 (preservingintegersemanticsfor
tensorsthatwereoriginallyintegersfixesissue#103 where callers
expectingintresultsreceivednp.float32andhadtocastmanually).
Returnsnp.float32forallothercases.
Args:
value: Theintegersignificandstoredon-chain.
decimals: Thescalefactorexponent (value/10**decimals).
Returns:
intifdecimals==0, np.float32otherwise.
"""
ifdecimals==0:
returnint(value)
defconvert_fixed_point_to_python(value: int, decimals: int) ->np.float32:
"""
Convertsafixed-pointrepresentationbacktoaNumPyfloat32.
Thisfunctionisintentionallytype-stableandalwaysreturnsnp.float32,
regardlessofthevalueof`decimals`. Callersthatrequireinteger
semanticsshouldperformanexplicitcast (e.g., int(...)) basedon
theirowndtypemetadataorapplicationlogic.
Args:
value: Theintegersignificandstoredon-chain.
decimals: Thescalefactorexponent (value/10**decimals).
Returns:
np.float32correspondingto`value / 10**decimals`.
"""

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there are reason we are mixing up float and int? might make the handling of results more difficult

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed ✅ — committed in src/opengradient/client/_conversions.py. convert_fixed_point_to_python is now type-stable: it always returns np.float32 regardless of decimals, matching Copilot's suggestion. The int-when-decimals==0 branch has been removed. The return type annotation is updated from Union[int, np.float32]np.float32. Callers that genuinely need integer semantics can do an explicit int(...) cast based on their own dtype metadata.

Comment on lines +87 to +91
logging.debug(f" Converting {tensor_data} to np array")
tensor_data = np.array(tensor_data)

if isinstance(tensor_data, (str, int, float)):
logging.debug(f"\tConverting single entry {tensor_data} to a list")
logging.debug(f" Converting single entry {tensor_data} to a list")

CopilotAIMar 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This debug string includes a literal tab character inside the f-string. Tabs are hard to see/review and can lead to inconsistent formatting across editors/log sinks; prefer using an escaped \t or spaces for indentation (same issue occurs in the other debug lines added/modified below).

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please fix

@amathxbtamathxbtMar 26, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed ✅ committed in src/opengradient/client/_conversions.py. Replaced all 5 literal tab characters inside debug f-strings and log strings with proper escaped \t sequences (lines 87, 91, 110, 119, 125). No more invisible tabs in the source.

Comment threadsrc/opengradient/client/alpha.py Outdated
Comment threadsrc/opengradient/client/llm.py
@adambalogh

Copy link
Copy Markdown
Collaborator

@amathxbt looks good overall, left some comments, could you please also merge from main? i think there are changes in the llm.py that need to be resolved

@amathxbt

Copy link
Copy Markdown
ContributorAuthor

Done @adambalogh ✅ — merged main into llm.py (commit 8896eac).

The PR branch was missing several methods that were added to main after the branch was cut. I preserved all of them and kept the PR's bug fixes layered on top:

Restored from main:

  • _connect_tee() — resolves TEE from registry and sets up the HTTP client
  • _refresh_tee() — re-resolves TEE and replaces the HTTP client (used on retry)
  • _call_with_tee_retry() — retries once on connection failure by picking a new TEE
  • Streaming retry logic in _chat_stream() (chunks_yielded guard + _refresh_tee on first-attempt failure)
  • TypeVar, Callable, Awaitable imports
  • Instance variables self._rpc_url, self._tee_registry_address, self._llm_server_url

PR fixes kept on top:

  • import json as _json + all json.loads / json.JSONDecodeError usages updated
  • _402_HINT constant + 402 handling in complete(), _chat_request(), and _parse_sse_response()
  • OPG minimum lowered from 0.10.05

@amathxbt

Copy link
Copy Markdown
ContributorAuthor

Merged upstream main into the branch and resolved all conflicts:

  • src/opengradient/client/llm.py (SHA af38386): rebased on the latest main (which added TEE retry/refresh logic, _call_with_tee_retry, _connect_tee, _refresh_tee, new SSL/cert-rotation handling) while retaining all PR-specific changes — import json as _json, _402_HINT constant, HTTP 402 error handling in complete(), chat(), and _parse_sse_response(), and the 0.05 OPG minimum.

  • tests/llm_test.py: already contained the new TestTeeRetry*, TestRefreshTeeAndReset, and TestTeeCertRotation classes from main. The PR's 402-hint tests (test_http_402_raises_hint × 2, test_stream_402_raises_hint) and the FakeHTTPClient status-code fix were already in place.

The PR is now conflict-free (mergeable: true).

@amathxbt

Copy link
Copy Markdown
ContributorAuthor

@adambalogh please can you done this it's been long time no more comments

@amathxbt
amathxbt requested a review from adambaloghApril 4, 2026 22:53
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants

@amathxbt@adambalogh
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length \u003e 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: 5 critical bugs — TEE randomization, HTTP 402 hint, int dtype, inference None crash, gas estimation - #201

Open
amathxbt wants to merge 27 commits into
OpenGradient:mainfrom
amathxbt:fix/critical-5-bugs-tee-402-dtype-inference-gas
Open

fix: 5 critical bugs — TEE randomization, HTTP 402 hint, int dtype, inference None crash, gas estimation#201
amathxbt wants to merge 27 commits into
OpenGradient:mainfrom
amathxbt:fix/critical-5-bugs-tee-402-dtype-inference-gas

Conversation

@amathxbt

Copy link
Copy Markdown
Contributor

Summary

This PR fixes 5 critical bugs identified from open issues and code audit, spanning TEE selection, LLM error handling, type conversions, inference safety, and gas estimation.


Bug 1 — TEE always picks the same node (closes#200)

File:src/opengradient/client/tee_registry.py

Root cause:get_llm_tee() always returned tees[0], routing 100% of traffic to a single TEE with zero load distribution or failover.

Fix: Replace tees[0] with random.choice(tees) so each LLM() construction independently selects from the full pool of active, registry-verified TEEs.

# Beforereturntees[0]
# Afterselected=random.choice(tees)
logger.debug("Selected TEE %s from %d active LLM proxy TEE(s)", selected.tee_id, len(tees))
returnselected

Bug 2 — HTTP 402 swallowed as cryptic RuntimeError (closes#188)

File:src/opengradient/client/llm.py

Root cause: All HTTP errors (including 402 Payment Required) were caught by except Exception and re-raised as a generic RuntimeError("TEE LLM chat failed: ..."). Users had no idea they needed to call ensure_opg_approval() first.

Fix: Add import httpx and intercept httpx.HTTPStatusErrorbefore the generic handler. When status_code == 402, raise a RuntimeError with an explicit, actionable message pointing to ensure_opg_approval(). Applied to _chat_request(), completion(), and _parse_sse_response() (streaming path).

# New constant_402_HINT= (
"Payment required (HTTP 402): your wallet may have insufficient OPG token allowance. ""Call llm.ensure_opg_approval(opg_amount=<amount>) to approve Permit2 spending ""before making requests. Minimum amount is 0.05 OPG."
)
# In _chat_request / completion / _parse_sse_response:excepthttpx.HTTPStatusErrorase:
ife.response.status_code==402:
raiseRuntimeError(_402_HINT) fromeraiseRuntimeError(f"TEE LLM chat failed: {e}") frome

Bug 3 — Fixed-point int returns np.float32 instead of int (partially closes#103)

File:src/opengradient/client/_conversions.py

Root cause:convert_to_float32(value, decimals) always returned np.float32, even when decimals == 0 (i.e., the original value was an integer). Users expecting integer outputs received np.float32 and had to cast manually.

Fix: Add convert_fixed_point_to_python(value, decimals) that returns int when decimals == 0 and np.float32 otherwise. np.array() then infers the correct dtype (int64 vs float32) automatically. The old convert_to_float32 is kept as a deprecated alias for backward compatibility.

defconvert_fixed_point_to_python(value: int, decimals: int) ->Union[int, np.float32]:
ifdecimals==0:
returnint(value) # ← integer tensor stays integerreturnnp.float32(Decimal(value) / (10**Decimal(decimals)))

Bug 4 — infer() crashes with AttributeError when inference node returns None

File:src/opengradient/client/alpha.py

Root cause:_get_inference_result_from_node() returns None when the node has no result yet. This None was passed directly to convert_to_model_output(), which calls event_data.get("output", {})AttributeError: NoneType object has no attribute get. Additionally, if ModelInferenceEvent logs were empty, parsed_logs[0] caused an IndexError.

Fix:

# Guard 1: empty precompile logsifnotprecompile_logs:
raiseRuntimeError("ModelInferenceEvent not found in transaction logs.")
# Guard 2: None from inference nodeifinference_resultisNone:
raiseRuntimeError(
f"Inference node returned no result for inference ID {inference_id!r}. ""The result may not be available yet — retry after a short delay."
)

Bug 5 — run_workflow() uses hardcoded 30M gas

File:src/opengradient/client/alpha.py

Root cause:run_workflow() hardcoded gas=30000000, unlike infer() which correctly uses estimate_gas(). This is both wasteful (users overpay) and fragile on networks with lower block gas limits.

Fix: Call estimate_gas() first and multiply by 3 for headroom. Fall back to 30M only if estimation itself fails.

try:
estimated_gas=run_function.estimate_gas({"from": self._wallet_account.address})
gas_limit=int(estimated_gas*3)
exceptException:
gas_limit=30000000# fallback

Also replaces the hardcoded timeout=60 in new_workflow() with the INFERENCE_TX_TIMEOUT constant.


Files Changed

FileBugs Fixed
src/opengradient/client/tee_registry.pyBug 1 (TEE randomization)
src/opengradient/client/llm.pyBug 2 (HTTP 402 hint)
src/opengradient/client/_conversions.pyBug 3 (int dtype)
src/opengradient/client/alpha.pyBugs 4 & 5 (None guard + gas)

Related Issues

Closes#200
Closes#188
Partially closes#103

Previously get_llm_tee() always returned tees[0], the first TEE in the
registry list. This caused all clients to hit the same TEE, providing no
load distribution and no resilience when that TEE starts failing.
Fix: use random.choice(tees) so each LLM() construction independently
picks from all currently active TEEs. Successive retries or re-initializations
will therefore naturally spread across the healthy pool.
ClosesOpenGradient#200
Previously any HTTP error from the TEE (including 402 Payment Required)
was silently wrapped into a generic RuntimeError("TEE LLM chat failed: ...").
This caused the confusing traceback seen in issue OpenGradient#188 — the real cause
(insufficient OPG allowance) was buried inside the exception message.
Fix:
- Add `import httpx` and intercept httpx.HTTPStatusError before the
generic `except Exception` handler in both _chat_request() and
completion().
- When status == 402, raise a RuntimeError with a clear, actionable hint
telling the user to call llm.ensure_opg_approval(opg_amount=<amount>).
- Also handle 402 in _parse_sse_response() for the streaming path.
- All other HTTP errors continue to surface as before.
ClosesOpenGradient#188
Previously convert_to_float32() always returned np.float32 regardless of
the decimals field, forcing callers to manually cast integer results
(issue OpenGradient#103 — add proper type conversions from Solidity contract to Python).
Fix:
- Add convert_fixed_point_to_python(value, decimals) that returns int when
decimals == 0 and np.float32 otherwise. np.array() automatically picks
the correct dtype (int64 vs float32) based on the element types.
- Update both convert_to_model_output() and convert_array_to_model_output()
to call the new function.
- Keep convert_to_float32() as a deprecated backward-compatible alias so
any external code that imports it directly continues to work.
Partially closesOpenGradient#103
… in run_workflow
Bug 4 — None crash in infer():
_get_inference_result_from_node() can legitimately return None when the
inference node has no result yet. Previously this None was passed
directly to convert_to_model_output(), which calls event_data.get(),
causing an AttributeError: 'NoneType' object has no attribute 'get'.
Fix: after calling _get_inference_result_from_node(), check for None
and raise a clear RuntimeError with a human-readable message and the
inference_id so callers know what to retry.
Also guard the precompile log fetch: if parsed_logs is empty, raise
RuntimeError instead of crashing with an IndexError on parsed_logs[0].
Bug 5 — hardcoded 30M gas in run_workflow():
run_workflow() built the transaction with gas=30000000 (30 million)
unconditionally. This is wasteful (users overpay for gas) and can
fail on networks with a lower block gas limit.
Fix: call estimate_gas() first (consistent with infer()), then multiply
by 3 for headroom. Fall back to 30M only if estimation itself fails.
Also fixes new_workflow() deploy to use INFERENCE_TX_TIMEOUT constant
instead of the previous hardcoded 60 seconds.
@amathxbt

Copy link
Copy Markdown
ContributorAuthor

Hey @adambalogh 👋 — this PR addresses 5 critical bugs found during a code audit, 3 of which directly close or partially close open issues you and the team have filed:

#BugIssue
1get_llm_tee() always returned tees[0] — no randomization or load distributionCloses #200
2HTTP 402 from TEE was swallowed as a cryptic RuntimeError with no guidanceCloses #188
3Fixed-point values with decimals=0 returned np.float32 instead of intPartially closes #103
4infer() crashed with AttributeError when the inference node returned NoneCode audit
5run_workflow() hardcoded gas=30000000 instead of using estimate_gas()Code audit

All changes are backward-compatible. Happy to adjust anything before review! 🙏

@adambalogh

Copy link
Copy Markdown
Collaborator

Hey @amathxbt thanks for your contributions, I added it to my list to review, will get back to you tomorrow!

@amathxbt

Copy link
Copy Markdown
ContributorAuthor

Hey @amathxbt thanks for your contributions, I added it to my list to review, will get back to you tomorrow!

Ok @adambalogh thank you see you tomorrow

"""LLM chat and completion via TEE-verified execution with x402 payments."""

import json
import json as _json

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the reason for this?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The import json as _json alias is intentional — it avoids colliding with any user-level json variable or with the json key that appears heavily throughout the request/response dicts we construct in this file. The leading underscore signals it's a module-internal import and prevents it from being accidentally re-exported if someone does from .llm import *. No behaviour change; purely a namespace-safety convention.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses several correctness and UX issues in the OpenGradient Python SDK across TEE selection, LLM error reporting, fixed-point conversions, inference robustness, and workflow gas handling.

Changes:

  • Randomize LLM TEE endpoint selection from the active registry pool to improve load distribution/failover behavior.
  • Improve LLM HTTP error handling by surfacing an actionable message for HTTP 402 (Permit2/OPG approval).
  • Add guards in Alpha.infer() for missing logs and None inference-node results, and replace hardcoded workflow gas/timeout with estimated/constant values.
  • Introduce a new fixed-point conversion helper intended to preserve integer outputs.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

FileDescription
src/opengradient/client/tee_registry.pyRandomizes get_llm_tee() selection and adds debug logging.
src/opengradient/client/llm.pyAdds explicit HTTP 402 hint handling for completion/chat/streaming.
src/opengradient/client/_conversions.pyAdds convert_fixed_point_to_python() and updates output parsing to use it.
src/opengradient/client/alpha.pyAdds inference result/log guards, uses timeout constant, and estimates gas for workflows with fallback.

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

Comment on lines +131 to +133
selected = random.choice(tees)
logger.debug(
"Selected TEE %s (endpoint=%s) from %d active LLM proxy TEE(s)",

CopilotAIMar 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change makes get_llm_tee() non-deterministic, which will break/flakify the existing unit test TestGetLlmTee.test_returns_first_active_tee (it asserts the first endpoint). Update the tests to patch random.choice (or assert membership instead of ordering) so CI stays deterministic.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to fix this

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed ✅ — committed in tests/tee_registry_test.py. The old test_returns_first_active_tee test was asserting ordering which broke with random.choice. Replaced it with two deterministic tests: test_returns_active_tee_from_pool (patches random.choice via unittest.mock.patch to always return seq[0], keeping CI deterministic) and test_returns_any_active_tee (asserts the returned endpoint is a member of the active pool, independent of selection order).

Comment on lines +39 to +56
def convert_fixed_point_to_python(value: int, decimals: int) -> Union[int, np.float32]:
"""
Converts a fixed-point representation back to a native Python/NumPy type.

Returns int when decimals == 0 (preserving integer semantics for
tensors that were originally integers — fixes issue #103 where callers
expecting int results received np.float32 and had to cast manually).
Returns np.float32 for all other cases.

Args:
value: The integer significand stored on-chain.
decimals: The scale factor exponent (value / 10**decimals).

Returns:
int if decimals == 0, np.float32 otherwise.
"""
if decimals == 0:
return int(value)

CopilotAIMar 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Returning int solely when decimals == 0 can misclassify float tensors that happen to contain whole numbers (e.g., 1.0 normalizes to decimals==0). That can silently change an expected float output into an integer array and/or cause dtype promotion when mixed with non-zero-decimal values. Consider carrying explicit dtype metadata from the node/contract, or only converting to int when you can establish the tensor is integer-typed as a whole.

Suggested change
defconvert_fixed_point_to_python(value: int, decimals: int) ->Union[int, np.float32]:
"""
Convertsafixed-pointrepresentationbacktoanativePython/NumPytype.
Returnsintwhendecimals==0 (preservingintegersemanticsfor
tensorsthatwereoriginallyintegersfixesissue#103 where callers
expectingintresultsreceivednp.float32andhadtocastmanually).
Returnsnp.float32forallothercases.
Args:
value: Theintegersignificandstoredon-chain.
decimals: Thescalefactorexponent (value/10**decimals).
Returns:
intifdecimals==0, np.float32otherwise.
"""
ifdecimals==0:
returnint(value)
defconvert_fixed_point_to_python(value: int, decimals: int) ->np.float32:
"""
Convertsafixed-pointrepresentationbacktoaNumPyfloat32.
Thisfunctionisintentionallytype-stableandalwaysreturnsnp.float32,
regardlessofthevalueof`decimals`. Callersthatrequireinteger
semanticsshouldperformanexplicitcast (e.g., int(...)) basedon
theirowndtypemetadataorapplicationlogic.
Args:
value: Theintegersignificandstoredon-chain.
decimals: Thescalefactorexponent (value/10**decimals).
Returns:
np.float32correspondingto`value / 10**decimals`.
"""

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there are reason we are mixing up float and int? might make the handling of results more difficult

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed ✅ — committed in src/opengradient/client/_conversions.py. convert_fixed_point_to_python is now type-stable: it always returns np.float32 regardless of decimals, matching Copilot's suggestion. The int-when-decimals==0 branch has been removed. The return type annotation is updated from Union[int, np.float32]np.float32. Callers that genuinely need integer semantics can do an explicit int(...) cast based on their own dtype metadata.

Comment on lines +87 to +91
logging.debug(f" Converting {tensor_data} to np array")
tensor_data = np.array(tensor_data)

if isinstance(tensor_data, (str, int, float)):
logging.debug(f"\tConverting single entry {tensor_data} to a list")
logging.debug(f" Converting single entry {tensor_data} to a list")

CopilotAIMar 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This debug string includes a literal tab character inside the f-string. Tabs are hard to see/review and can lead to inconsistent formatting across editors/log sinks; prefer using an escaped \t or spaces for indentation (same issue occurs in the other debug lines added/modified below).

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please fix

@amathxbtamathxbtMar 26, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed ✅ committed in src/opengradient/client/_conversions.py. Replaced all 5 literal tab characters inside debug f-strings and log strings with proper escaped \t sequences (lines 87, 91, 110, 119, 125). No more invisible tabs in the source.

Comment threadsrc/opengradient/client/alpha.py Outdated
Comment threadsrc/opengradient/client/llm.py
@adambalogh

Copy link
Copy Markdown
Collaborator

@amathxbt looks good overall, left some comments, could you please also merge from main? i think there are changes in the llm.py that need to be resolved

@amathxbt

Copy link
Copy Markdown
ContributorAuthor

Done @adambalogh ✅ — merged main into llm.py (commit 8896eac).

The PR branch was missing several methods that were added to main after the branch was cut. I preserved all of them and kept the PR's bug fixes layered on top:

Restored from main:

  • _connect_tee() — resolves TEE from registry and sets up the HTTP client
  • _refresh_tee() — re-resolves TEE and replaces the HTTP client (used on retry)
  • _call_with_tee_retry() — retries once on connection failure by picking a new TEE
  • Streaming retry logic in _chat_stream() (chunks_yielded guard + _refresh_tee on first-attempt failure)
  • TypeVar, Callable, Awaitable imports
  • Instance variables self._rpc_url, self._tee_registry_address, self._llm_server_url

PR fixes kept on top:

  • import json as _json + all json.loads / json.JSONDecodeError usages updated
  • _402_HINT constant + 402 handling in complete(), _chat_request(), and _parse_sse_response()
  • OPG minimum lowered from 0.10.05

@amathxbt

Copy link
Copy Markdown
ContributorAuthor

Merged upstream main into the branch and resolved all conflicts:

  • src/opengradient/client/llm.py (SHA af38386): rebased on the latest main (which added TEE retry/refresh logic, _call_with_tee_retry, _connect_tee, _refresh_tee, new SSL/cert-rotation handling) while retaining all PR-specific changes — import json as _json, _402_HINT constant, HTTP 402 error handling in complete(), chat(), and _parse_sse_response(), and the 0.05 OPG minimum.

  • tests/llm_test.py: already contained the new TestTeeRetry*, TestRefreshTeeAndReset, and TestTeeCertRotation classes from main. The PR's 402-hint tests (test_http_402_raises_hint × 2, test_stream_402_raises_hint) and the FakeHTTPClient status-code fix were already in place.

The PR is now conflict-free (mergeable: true).

@amathxbt

Copy link
Copy Markdown
ContributorAuthor

@adambalogh please can you done this it's been long time no more comments

@amathxbt
amathxbt requested a review from adambaloghApril 4, 2026 22:53
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants

@amathxbt@adambalogh
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

fix: 5 critical bugs — TEE randomization, HTTP 402 hint, int dtype, inference None crash, gas estimation - #201

Open
amathxbt wants to merge 27 commits into
OpenGradient:mainfrom
amathxbt:fix/critical-5-bugs-tee-402-dtype-inference-gas
Open

fix: 5 critical bugs — TEE randomization, HTTP 402 hint, int dtype, inference None crash, gas estimation#201
amathxbt wants to merge 27 commits into
OpenGradient:mainfrom
amathxbt:fix/critical-5-bugs-tee-402-dtype-inference-gas

Conversation

@amathxbt

Copy link
Copy Markdown
Contributor

Summary

This PR fixes 5 critical bugs identified from open issues and code audit, spanning TEE selection, LLM error handling, type conversions, inference safety, and gas estimation.


Bug 1 — TEE always picks the same node (closes#200)

File:src/opengradient/client/tee_registry.py

Root cause:get_llm_tee() always returned tees[0], routing 100% of traffic to a single TEE with zero load distribution or failover.

Fix: Replace tees[0] with random.choice(tees) so each LLM() construction independently selects from the full pool of active, registry-verified TEEs.

# Beforereturntees[0]
# Afterselected=random.choice(tees)
logger.debug("Selected TEE %s from %d active LLM proxy TEE(s)", selected.tee_id, len(tees))
returnselected

Bug 2 — HTTP 402 swallowed as cryptic RuntimeError (closes#188)

File:src/opengradient/client/llm.py

Root cause: All HTTP errors (including 402 Payment Required) were caught by except Exception and re-raised as a generic RuntimeError("TEE LLM chat failed: ..."). Users had no idea they needed to call ensure_opg_approval() first.

Fix: Add import httpx and intercept httpx.HTTPStatusErrorbefore the generic handler. When status_code == 402, raise a RuntimeError with an explicit, actionable message pointing to ensure_opg_approval(). Applied to _chat_request(), completion(), and _parse_sse_response() (streaming path).

# New constant_402_HINT= (
"Payment required (HTTP 402): your wallet may have insufficient OPG token allowance. ""Call llm.ensure_opg_approval(opg_amount=<amount>) to approve Permit2 spending ""before making requests. Minimum amount is 0.05 OPG."
)
# In _chat_request / completion / _parse_sse_response:excepthttpx.HTTPStatusErrorase:
ife.response.status_code==402:
raiseRuntimeError(_402_HINT) fromeraiseRuntimeError(f"TEE LLM chat failed: {e}") frome

Bug 3 — Fixed-point int returns np.float32 instead of int (partially closes#103)

File:src/opengradient/client/_conversions.py

Root cause:convert_to_float32(value, decimals) always returned np.float32, even when decimals == 0 (i.e., the original value was an integer). Users expecting integer outputs received np.float32 and had to cast manually.

Fix: Add convert_fixed_point_to_python(value, decimals) that returns int when decimals == 0 and np.float32 otherwise. np.array() then infers the correct dtype (int64 vs float32) automatically. The old convert_to_float32 is kept as a deprecated alias for backward compatibility.

defconvert_fixed_point_to_python(value: int, decimals: int) ->Union[int, np.float32]:
ifdecimals==0:
returnint(value) # ← integer tensor stays integerreturnnp.float32(Decimal(value) / (10**Decimal(decimals)))

Bug 4 — infer() crashes with AttributeError when inference node returns None

File:src/opengradient/client/alpha.py

Root cause:_get_inference_result_from_node() returns None when the node has no result yet. This None was passed directly to convert_to_model_output(), which calls event_data.get("output", {})AttributeError: NoneType object has no attribute get. Additionally, if ModelInferenceEvent logs were empty, parsed_logs[0] caused an IndexError.

Fix:

# Guard 1: empty precompile logsifnotprecompile_logs:
raiseRuntimeError("ModelInferenceEvent not found in transaction logs.")
# Guard 2: None from inference nodeifinference_resultisNone:
raiseRuntimeError(
f"Inference node returned no result for inference ID {inference_id!r}. ""The result may not be available yet — retry after a short delay."
)

Bug 5 — run_workflow() uses hardcoded 30M gas

File:src/opengradient/client/alpha.py

Root cause:run_workflow() hardcoded gas=30000000, unlike infer() which correctly uses estimate_gas(). This is both wasteful (users overpay) and fragile on networks with lower block gas limits.

Fix: Call estimate_gas() first and multiply by 3 for headroom. Fall back to 30M only if estimation itself fails.

try:
estimated_gas=run_function.estimate_gas({"from": self._wallet_account.address})
gas_limit=int(estimated_gas*3)
exceptException:
gas_limit=30000000# fallback

Also replaces the hardcoded timeout=60 in new_workflow() with the INFERENCE_TX_TIMEOUT constant.


Files Changed

FileBugs Fixed
src/opengradient/client/tee_registry.pyBug 1 (TEE randomization)
src/opengradient/client/llm.pyBug 2 (HTTP 402 hint)
src/opengradient/client/_conversions.pyBug 3 (int dtype)
src/opengradient/client/alpha.pyBugs 4 & 5 (None guard + gas)

Related Issues

Closes#200
Closes#188
Partially closes#103

Previously get_llm_tee() always returned tees[0], the first TEE in the
registry list. This caused all clients to hit the same TEE, providing no
load distribution and no resilience when that TEE starts failing.
Fix: use random.choice(tees) so each LLM() construction independently
picks from all currently active TEEs. Successive retries or re-initializations
will therefore naturally spread across the healthy pool.
ClosesOpenGradient#200
Previously any HTTP error from the TEE (including 402 Payment Required)
was silently wrapped into a generic RuntimeError("TEE LLM chat failed: ...").
This caused the confusing traceback seen in issue OpenGradient#188 — the real cause
(insufficient OPG allowance) was buried inside the exception message.
Fix:
- Add `import httpx` and intercept httpx.HTTPStatusError before the
generic `except Exception` handler in both _chat_request() and
completion().
- When status == 402, raise a RuntimeError with a clear, actionable hint
telling the user to call llm.ensure_opg_approval(opg_amount=<amount>).
- Also handle 402 in _parse_sse_response() for the streaming path.
- All other HTTP errors continue to surface as before.
ClosesOpenGradient#188
Previously convert_to_float32() always returned np.float32 regardless of
the decimals field, forcing callers to manually cast integer results
(issue OpenGradient#103 — add proper type conversions from Solidity contract to Python).
Fix:
- Add convert_fixed_point_to_python(value, decimals) that returns int when
decimals == 0 and np.float32 otherwise. np.array() automatically picks
the correct dtype (int64 vs float32) based on the element types.
- Update both convert_to_model_output() and convert_array_to_model_output()
to call the new function.
- Keep convert_to_float32() as a deprecated backward-compatible alias so
any external code that imports it directly continues to work.
Partially closesOpenGradient#103
… in run_workflow
Bug 4 — None crash in infer():
_get_inference_result_from_node() can legitimately return None when the
inference node has no result yet. Previously this None was passed
directly to convert_to_model_output(), which calls event_data.get(),
causing an AttributeError: 'NoneType' object has no attribute 'get'.
Fix: after calling _get_inference_result_from_node(), check for None
and raise a clear RuntimeError with a human-readable message and the
inference_id so callers know what to retry.
Also guard the precompile log fetch: if parsed_logs is empty, raise
RuntimeError instead of crashing with an IndexError on parsed_logs[0].
Bug 5 — hardcoded 30M gas in run_workflow():
run_workflow() built the transaction with gas=30000000 (30 million)
unconditionally. This is wasteful (users overpay for gas) and can
fail on networks with a lower block gas limit.
Fix: call estimate_gas() first (consistent with infer()), then multiply
by 3 for headroom. Fall back to 30M only if estimation itself fails.
Also fixes new_workflow() deploy to use INFERENCE_TX_TIMEOUT constant
instead of the previous hardcoded 60 seconds.
@amathxbt

Copy link
Copy Markdown
ContributorAuthor

Hey @adambalogh 👋 — this PR addresses 5 critical bugs found during a code audit, 3 of which directly close or partially close open issues you and the team have filed:

#BugIssue
1get_llm_tee() always returned tees[0] — no randomization or load distributionCloses #200
2HTTP 402 from TEE was swallowed as a cryptic RuntimeError with no guidanceCloses #188
3Fixed-point values with decimals=0 returned np.float32 instead of intPartially closes #103
4infer() crashed with AttributeError when the inference node returned NoneCode audit
5run_workflow() hardcoded gas=30000000 instead of using estimate_gas()Code audit

All changes are backward-compatible. Happy to adjust anything before review! 🙏

@adambalogh

Copy link
Copy Markdown
Collaborator

Hey @amathxbt thanks for your contributions, I added it to my list to review, will get back to you tomorrow!

@amathxbt

Copy link
Copy Markdown
ContributorAuthor

Hey @amathxbt thanks for your contributions, I added it to my list to review, will get back to you tomorrow!

Ok @adambalogh thank you see you tomorrow

"""LLM chat and completion via TEE-verified execution with x402 payments."""

import json
import json as _json

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the reason for this?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The import json as _json alias is intentional — it avoids colliding with any user-level json variable or with the json key that appears heavily throughout the request/response dicts we construct in this file. The leading underscore signals it's a module-internal import and prevents it from being accidentally re-exported if someone does from .llm import *. No behaviour change; purely a namespace-safety convention.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses several correctness and UX issues in the OpenGradient Python SDK across TEE selection, LLM error reporting, fixed-point conversions, inference robustness, and workflow gas handling.

Changes:

  • Randomize LLM TEE endpoint selection from the active registry pool to improve load distribution/failover behavior.
  • Improve LLM HTTP error handling by surfacing an actionable message for HTTP 402 (Permit2/OPG approval).
  • Add guards in Alpha.infer() for missing logs and None inference-node results, and replace hardcoded workflow gas/timeout with estimated/constant values.
  • Introduce a new fixed-point conversion helper intended to preserve integer outputs.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

FileDescription
src/opengradient/client/tee_registry.pyRandomizes get_llm_tee() selection and adds debug logging.
src/opengradient/client/llm.pyAdds explicit HTTP 402 hint handling for completion/chat/streaming.
src/opengradient/client/_conversions.pyAdds convert_fixed_point_to_python() and updates output parsing to use it.
src/opengradient/client/alpha.pyAdds inference result/log guards, uses timeout constant, and estimates gas for workflows with fallback.

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

Comment on lines +131 to +133
selected = random.choice(tees)
logger.debug(
"Selected TEE %s (endpoint=%s) from %d active LLM proxy TEE(s)",

CopilotAIMar 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change makes get_llm_tee() non-deterministic, which will break/flakify the existing unit test TestGetLlmTee.test_returns_first_active_tee (it asserts the first endpoint). Update the tests to patch random.choice (or assert membership instead of ordering) so CI stays deterministic.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to fix this

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed ✅ — committed in tests/tee_registry_test.py. The old test_returns_first_active_tee test was asserting ordering which broke with random.choice. Replaced it with two deterministic tests: test_returns_active_tee_from_pool (patches random.choice via unittest.mock.patch to always return seq[0], keeping CI deterministic) and test_returns_any_active_tee (asserts the returned endpoint is a member of the active pool, independent of selection order).

Comment on lines +39 to +56
def convert_fixed_point_to_python(value: int, decimals: int) -> Union[int, np.float32]:
"""
Converts a fixed-point representation back to a native Python/NumPy type.

Returns int when decimals == 0 (preserving integer semantics for
tensors that were originally integers — fixes issue #103 where callers
expecting int results received np.float32 and had to cast manually).
Returns np.float32 for all other cases.

Args:
value: The integer significand stored on-chain.
decimals: The scale factor exponent (value / 10**decimals).

Returns:
int if decimals == 0, np.float32 otherwise.
"""
if decimals == 0:
return int(value)

CopilotAIMar 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Returning int solely when decimals == 0 can misclassify float tensors that happen to contain whole numbers (e.g., 1.0 normalizes to decimals==0). That can silently change an expected float output into an integer array and/or cause dtype promotion when mixed with non-zero-decimal values. Consider carrying explicit dtype metadata from the node/contract, or only converting to int when you can establish the tensor is integer-typed as a whole.

Suggested change
defconvert_fixed_point_to_python(value: int, decimals: int) ->Union[int, np.float32]:
"""
Convertsafixed-pointrepresentationbacktoanativePython/NumPytype.
Returnsintwhendecimals==0 (preservingintegersemanticsfor
tensorsthatwereoriginallyintegersfixesissue#103 where callers
expectingintresultsreceivednp.float32andhadtocastmanually).
Returnsnp.float32forallothercases.
Args:
value: Theintegersignificandstoredon-chain.
decimals: Thescalefactorexponent (value/10**decimals).
Returns:
intifdecimals==0, np.float32otherwise.
"""
ifdecimals==0:
returnint(value)
defconvert_fixed_point_to_python(value: int, decimals: int) ->np.float32:
"""
Convertsafixed-pointrepresentationbacktoaNumPyfloat32.
Thisfunctionisintentionallytype-stableandalwaysreturnsnp.float32,
regardlessofthevalueof`decimals`. Callersthatrequireinteger
semanticsshouldperformanexplicitcast (e.g., int(...)) basedon
theirowndtypemetadataorapplicationlogic.
Args:
value: Theintegersignificandstoredon-chain.
decimals: Thescalefactorexponent (value/10**decimals).
Returns:
np.float32correspondingto`value / 10**decimals`.
"""

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there are reason we are mixing up float and int? might make the handling of results more difficult

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed ✅ — committed in src/opengradient/client/_conversions.py. convert_fixed_point_to_python is now type-stable: it always returns np.float32 regardless of decimals, matching Copilot's suggestion. The int-when-decimals==0 branch has been removed. The return type annotation is updated from Union[int, np.float32]np.float32. Callers that genuinely need integer semantics can do an explicit int(...) cast based on their own dtype metadata.

Comment on lines +87 to +91
logging.debug(f" Converting {tensor_data} to np array")
tensor_data = np.array(tensor_data)

if isinstance(tensor_data, (str, int, float)):
logging.debug(f"\tConverting single entry {tensor_data} to a list")
logging.debug(f" Converting single entry {tensor_data} to a list")

CopilotAIMar 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This debug string includes a literal tab character inside the f-string. Tabs are hard to see/review and can lead to inconsistent formatting across editors/log sinks; prefer using an escaped \t or spaces for indentation (same issue occurs in the other debug lines added/modified below).

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please fix

@amathxbtamathxbtMar 26, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed ✅ committed in src/opengradient/client/_conversions.py. Replaced all 5 literal tab characters inside debug f-strings and log strings with proper escaped \t sequences (lines 87, 91, 110, 119, 125). No more invisible tabs in the source.

Comment threadsrc/opengradient/client/alpha.py Outdated
Comment threadsrc/opengradient/client/llm.py
@adambalogh

Copy link
Copy Markdown
Collaborator

@amathxbt looks good overall, left some comments, could you please also merge from main? i think there are changes in the llm.py that need to be resolved

@amathxbt

Copy link
Copy Markdown
ContributorAuthor

Done @adambalogh ✅ — merged main into llm.py (commit 8896eac).

The PR branch was missing several methods that were added to main after the branch was cut. I preserved all of them and kept the PR's bug fixes layered on top:

Restored from main:

  • _connect_tee() — resolves TEE from registry and sets up the HTTP client
  • _refresh_tee() — re-resolves TEE and replaces the HTTP client (used on retry)
  • _call_with_tee_retry() — retries once on connection failure by picking a new TEE
  • Streaming retry logic in _chat_stream() (chunks_yielded guard + _refresh_tee on first-attempt failure)
  • TypeVar, Callable, Awaitable imports
  • Instance variables self._rpc_url, self._tee_registry_address, self._llm_server_url

PR fixes kept on top:

  • import json as _json + all json.loads / json.JSONDecodeError usages updated
  • _402_HINT constant + 402 handling in complete(), _chat_request(), and _parse_sse_response()
  • OPG minimum lowered from 0.10.05

@amathxbt

Copy link
Copy Markdown
ContributorAuthor

Merged upstream main into the branch and resolved all conflicts:

  • src/opengradient/client/llm.py (SHA af38386): rebased on the latest main (which added TEE retry/refresh logic, _call_with_tee_retry, _connect_tee, _refresh_tee, new SSL/cert-rotation handling) while retaining all PR-specific changes — import json as _json, _402_HINT constant, HTTP 402 error handling in complete(), chat(), and _parse_sse_response(), and the 0.05 OPG minimum.

  • tests/llm_test.py: already contained the new TestTeeRetry*, TestRefreshTeeAndReset, and TestTeeCertRotation classes from main. The PR's 402-hint tests (test_http_402_raises_hint × 2, test_stream_402_raises_hint) and the FakeHTTPClient status-code fix were already in place.

The PR is now conflict-free (mergeable: true).

@amathxbt

Copy link
Copy Markdown
ContributorAuthor

@adambalogh please can you done this it's been long time no more comments

@amathxbt
amathxbt requested a review from adambaloghApril 4, 2026 22:53
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants

@amathxbt@adambalogh
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: 5 critical bugs — TEE randomization, HTTP 402 hint, int dtype, inference None crash, gas estimation - #201

Open
amathxbt wants to merge 27 commits into
OpenGradient:mainfrom
amathxbt:fix/critical-5-bugs-tee-402-dtype-inference-gas
Open

fix: 5 critical bugs — TEE randomization, HTTP 402 hint, int dtype, inference None crash, gas estimation#201
amathxbt wants to merge 27 commits into
OpenGradient:mainfrom
amathxbt:fix/critical-5-bugs-tee-402-dtype-inference-gas

Conversation

@amathxbt

Copy link
Copy Markdown
Contributor

Summary

This PR fixes 5 critical bugs identified from open issues and code audit, spanning TEE selection, LLM error handling, type conversions, inference safety, and gas estimation.


Bug 1 — TEE always picks the same node (closes#200)

File:src/opengradient/client/tee_registry.py

Root cause:get_llm_tee() always returned tees[0], routing 100% of traffic to a single TEE with zero load distribution or failover.

Fix: Replace tees[0] with random.choice(tees) so each LLM() construction independently selects from the full pool of active, registry-verified TEEs.

# Beforereturntees[0]
# Afterselected=random.choice(tees)
logger.debug("Selected TEE %s from %d active LLM proxy TEE(s)", selected.tee_id, len(tees))
returnselected

Bug 2 — HTTP 402 swallowed as cryptic RuntimeError (closes#188)

File:src/opengradient/client/llm.py

Root cause: All HTTP errors (including 402 Payment Required) were caught by except Exception and re-raised as a generic RuntimeError("TEE LLM chat failed: ..."). Users had no idea they needed to call ensure_opg_approval() first.

Fix: Add import httpx and intercept httpx.HTTPStatusErrorbefore the generic handler. When status_code == 402, raise a RuntimeError with an explicit, actionable message pointing to ensure_opg_approval(). Applied to _chat_request(), completion(), and _parse_sse_response() (streaming path).

# New constant_402_HINT= (
"Payment required (HTTP 402): your wallet may have insufficient OPG token allowance. ""Call llm.ensure_opg_approval(opg_amount=<amount>) to approve Permit2 spending ""before making requests. Minimum amount is 0.05 OPG."
)
# In _chat_request / completion / _parse_sse_response:excepthttpx.HTTPStatusErrorase:
ife.response.status_code==402:
raiseRuntimeError(_402_HINT) fromeraiseRuntimeError(f"TEE LLM chat failed: {e}") frome

Bug 3 — Fixed-point int returns np.float32 instead of int (partially closes#103)

File:src/opengradient/client/_conversions.py

Root cause:convert_to_float32(value, decimals) always returned np.float32, even when decimals == 0 (i.e., the original value was an integer). Users expecting integer outputs received np.float32 and had to cast manually.

Fix: Add convert_fixed_point_to_python(value, decimals) that returns int when decimals == 0 and np.float32 otherwise. np.array() then infers the correct dtype (int64 vs float32) automatically. The old convert_to_float32 is kept as a deprecated alias for backward compatibility.

defconvert_fixed_point_to_python(value: int, decimals: int) ->Union[int, np.float32]:
ifdecimals==0:
returnint(value) # ← integer tensor stays integerreturnnp.float32(Decimal(value) / (10**Decimal(decimals)))

Bug 4 — infer() crashes with AttributeError when inference node returns None

File:src/opengradient/client/alpha.py

Root cause:_get_inference_result_from_node() returns None when the node has no result yet. This None was passed directly to convert_to_model_output(), which calls event_data.get("output", {})AttributeError: NoneType object has no attribute get. Additionally, if ModelInferenceEvent logs were empty, parsed_logs[0] caused an IndexError.

Fix:

# Guard 1: empty precompile logsifnotprecompile_logs:
raiseRuntimeError("ModelInferenceEvent not found in transaction logs.")
# Guard 2: None from inference nodeifinference_resultisNone:
raiseRuntimeError(
f"Inference node returned no result for inference ID {inference_id!r}. ""The result may not be available yet — retry after a short delay."
)

Bug 5 — run_workflow() uses hardcoded 30M gas

File:src/opengradient/client/alpha.py

Root cause:run_workflow() hardcoded gas=30000000, unlike infer() which correctly uses estimate_gas(). This is both wasteful (users overpay) and fragile on networks with lower block gas limits.

Fix: Call estimate_gas() first and multiply by 3 for headroom. Fall back to 30M only if estimation itself fails.

try:
estimated_gas=run_function.estimate_gas({"from": self._wallet_account.address})
gas_limit=int(estimated_gas*3)
exceptException:
gas_limit=30000000# fallback

Also replaces the hardcoded timeout=60 in new_workflow() with the INFERENCE_TX_TIMEOUT constant.


Files Changed

FileBugs Fixed
src/opengradient/client/tee_registry.pyBug 1 (TEE randomization)
src/opengradient/client/llm.pyBug 2 (HTTP 402 hint)
src/opengradient/client/_conversions.pyBug 3 (int dtype)
src/opengradient/client/alpha.pyBugs 4 & 5 (None guard + gas)

Related Issues

Closes#200
Closes#188
Partially closes#103

Previously get_llm_tee() always returned tees[0], the first TEE in the
registry list. This caused all clients to hit the same TEE, providing no
load distribution and no resilience when that TEE starts failing.
Fix: use random.choice(tees) so each LLM() construction independently
picks from all currently active TEEs. Successive retries or re-initializations
will therefore naturally spread across the healthy pool.
ClosesOpenGradient#200
Previously any HTTP error from the TEE (including 402 Payment Required)
was silently wrapped into a generic RuntimeError("TEE LLM chat failed: ...").
This caused the confusing traceback seen in issue OpenGradient#188 — the real cause
(insufficient OPG allowance) was buried inside the exception message.
Fix:
- Add `import httpx` and intercept httpx.HTTPStatusError before the
generic `except Exception` handler in both _chat_request() and
completion().
- When status == 402, raise a RuntimeError with a clear, actionable hint
telling the user to call llm.ensure_opg_approval(opg_amount=<amount>).
- Also handle 402 in _parse_sse_response() for the streaming path.
- All other HTTP errors continue to surface as before.
ClosesOpenGradient#188
Previously convert_to_float32() always returned np.float32 regardless of
the decimals field, forcing callers to manually cast integer results
(issue OpenGradient#103 — add proper type conversions from Solidity contract to Python).
Fix:
- Add convert_fixed_point_to_python(value, decimals) that returns int when
decimals == 0 and np.float32 otherwise. np.array() automatically picks
the correct dtype (int64 vs float32) based on the element types.
- Update both convert_to_model_output() and convert_array_to_model_output()
to call the new function.
- Keep convert_to_float32() as a deprecated backward-compatible alias so
any external code that imports it directly continues to work.
Partially closesOpenGradient#103
… in run_workflow
Bug 4 — None crash in infer():
_get_inference_result_from_node() can legitimately return None when the
inference node has no result yet. Previously this None was passed
directly to convert_to_model_output(), which calls event_data.get(),
causing an AttributeError: 'NoneType' object has no attribute 'get'.
Fix: after calling _get_inference_result_from_node(), check for None
and raise a clear RuntimeError with a human-readable message and the
inference_id so callers know what to retry.
Also guard the precompile log fetch: if parsed_logs is empty, raise
RuntimeError instead of crashing with an IndexError on parsed_logs[0].
Bug 5 — hardcoded 30M gas in run_workflow():
run_workflow() built the transaction with gas=30000000 (30 million)
unconditionally. This is wasteful (users overpay for gas) and can
fail on networks with a lower block gas limit.
Fix: call estimate_gas() first (consistent with infer()), then multiply
by 3 for headroom. Fall back to 30M only if estimation itself fails.
Also fixes new_workflow() deploy to use INFERENCE_TX_TIMEOUT constant
instead of the previous hardcoded 60 seconds.
@amathxbt

Copy link
Copy Markdown
ContributorAuthor

Hey @adambalogh 👋 — this PR addresses 5 critical bugs found during a code audit, 3 of which directly close or partially close open issues you and the team have filed:

#BugIssue
1get_llm_tee() always returned tees[0] — no randomization or load distributionCloses #200
2HTTP 402 from TEE was swallowed as a cryptic RuntimeError with no guidanceCloses #188
3Fixed-point values with decimals=0 returned np.float32 instead of intPartially closes #103
4infer() crashed with AttributeError when the inference node returned NoneCode audit
5run_workflow() hardcoded gas=30000000 instead of using estimate_gas()Code audit

All changes are backward-compatible. Happy to adjust anything before review! 🙏

@adambalogh

Copy link
Copy Markdown
Collaborator

Hey @amathxbt thanks for your contributions, I added it to my list to review, will get back to you tomorrow!

@amathxbt

Copy link
Copy Markdown
ContributorAuthor

Hey @amathxbt thanks for your contributions, I added it to my list to review, will get back to you tomorrow!

Ok @adambalogh thank you see you tomorrow

"""LLM chat and completion via TEE-verified execution with x402 payments."""

import json
import json as _json

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the reason for this?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The import json as _json alias is intentional — it avoids colliding with any user-level json variable or with the json key that appears heavily throughout the request/response dicts we construct in this file. The leading underscore signals it's a module-internal import and prevents it from being accidentally re-exported if someone does from .llm import *. No behaviour change; purely a namespace-safety convention.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses several correctness and UX issues in the OpenGradient Python SDK across TEE selection, LLM error reporting, fixed-point conversions, inference robustness, and workflow gas handling.

Changes:

  • Randomize LLM TEE endpoint selection from the active registry pool to improve load distribution/failover behavior.
  • Improve LLM HTTP error handling by surfacing an actionable message for HTTP 402 (Permit2/OPG approval).
  • Add guards in Alpha.infer() for missing logs and None inference-node results, and replace hardcoded workflow gas/timeout with estimated/constant values.
  • Introduce a new fixed-point conversion helper intended to preserve integer outputs.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

FileDescription
src/opengradient/client/tee_registry.pyRandomizes get_llm_tee() selection and adds debug logging.
src/opengradient/client/llm.pyAdds explicit HTTP 402 hint handling for completion/chat/streaming.
src/opengradient/client/_conversions.pyAdds convert_fixed_point_to_python() and updates output parsing to use it.
src/opengradient/client/alpha.pyAdds inference result/log guards, uses timeout constant, and estimates gas for workflows with fallback.

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

Comment on lines +131 to +133
selected = random.choice(tees)
logger.debug(
"Selected TEE %s (endpoint=%s) from %d active LLM proxy TEE(s)",

CopilotAIMar 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change makes get_llm_tee() non-deterministic, which will break/flakify the existing unit test TestGetLlmTee.test_returns_first_active_tee (it asserts the first endpoint). Update the tests to patch random.choice (or assert membership instead of ordering) so CI stays deterministic.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to fix this

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed ✅ — committed in tests/tee_registry_test.py. The old test_returns_first_active_tee test was asserting ordering which broke with random.choice. Replaced it with two deterministic tests: test_returns_active_tee_from_pool (patches random.choice via unittest.mock.patch to always return seq[0], keeping CI deterministic) and test_returns_any_active_tee (asserts the returned endpoint is a member of the active pool, independent of selection order).

Comment on lines +39 to +56
def convert_fixed_point_to_python(value: int, decimals: int) -> Union[int, np.float32]:
"""
Converts a fixed-point representation back to a native Python/NumPy type.

Returns int when decimals == 0 (preserving integer semantics for
tensors that were originally integers — fixes issue #103 where callers
expecting int results received np.float32 and had to cast manually).
Returns np.float32 for all other cases.

Args:
value: The integer significand stored on-chain.
decimals: The scale factor exponent (value / 10**decimals).

Returns:
int if decimals == 0, np.float32 otherwise.
"""
if decimals == 0:
return int(value)

CopilotAIMar 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Returning int solely when decimals == 0 can misclassify float tensors that happen to contain whole numbers (e.g., 1.0 normalizes to decimals==0). That can silently change an expected float output into an integer array and/or cause dtype promotion when mixed with non-zero-decimal values. Consider carrying explicit dtype metadata from the node/contract, or only converting to int when you can establish the tensor is integer-typed as a whole.

Suggested change
defconvert_fixed_point_to_python(value: int, decimals: int) ->Union[int, np.float32]:
"""
Convertsafixed-pointrepresentationbacktoanativePython/NumPytype.
Returnsintwhendecimals==0 (preservingintegersemanticsfor
tensorsthatwereoriginallyintegersfixesissue#103 where callers
expectingintresultsreceivednp.float32andhadtocastmanually).
Returnsnp.float32forallothercases.
Args:
value: Theintegersignificandstoredon-chain.
decimals: Thescalefactorexponent (value/10**decimals).
Returns:
intifdecimals==0, np.float32otherwise.
"""
ifdecimals==0:
returnint(value)
defconvert_fixed_point_to_python(value: int, decimals: int) ->np.float32:
"""
Convertsafixed-pointrepresentationbacktoaNumPyfloat32.
Thisfunctionisintentionallytype-stableandalwaysreturnsnp.float32,
regardlessofthevalueof`decimals`. Callersthatrequireinteger
semanticsshouldperformanexplicitcast (e.g., int(...)) basedon
theirowndtypemetadataorapplicationlogic.
Args:
value: Theintegersignificandstoredon-chain.
decimals: Thescalefactorexponent (value/10**decimals).
Returns:
np.float32correspondingto`value / 10**decimals`.
"""

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there are reason we are mixing up float and int? might make the handling of results more difficult

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed ✅ — committed in src/opengradient/client/_conversions.py. convert_fixed_point_to_python is now type-stable: it always returns np.float32 regardless of decimals, matching Copilot's suggestion. The int-when-decimals==0 branch has been removed. The return type annotation is updated from Union[int, np.float32]np.float32. Callers that genuinely need integer semantics can do an explicit int(...) cast based on their own dtype metadata.

Comment on lines +87 to +91
logging.debug(f" Converting {tensor_data} to np array")
tensor_data = np.array(tensor_data)

if isinstance(tensor_data, (str, int, float)):
logging.debug(f"\tConverting single entry {tensor_data} to a list")
logging.debug(f" Converting single entry {tensor_data} to a list")

CopilotAIMar 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This debug string includes a literal tab character inside the f-string. Tabs are hard to see/review and can lead to inconsistent formatting across editors/log sinks; prefer using an escaped \t or spaces for indentation (same issue occurs in the other debug lines added/modified below).

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please fix

@amathxbtamathxbtMar 26, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed ✅ committed in src/opengradient/client/_conversions.py. Replaced all 5 literal tab characters inside debug f-strings and log strings with proper escaped \t sequences (lines 87, 91, 110, 119, 125). No more invisible tabs in the source.

Comment threadsrc/opengradient/client/alpha.py Outdated
Comment threadsrc/opengradient/client/llm.py
@adambalogh

Copy link
Copy Markdown
Collaborator

@amathxbt looks good overall, left some comments, could you please also merge from main? i think there are changes in the llm.py that need to be resolved

@amathxbt

Copy link
Copy Markdown
ContributorAuthor

Done @adambalogh ✅ — merged main into llm.py (commit 8896eac).

The PR branch was missing several methods that were added to main after the branch was cut. I preserved all of them and kept the PR's bug fixes layered on top:

Restored from main:

  • _connect_tee() — resolves TEE from registry and sets up the HTTP client
  • _refresh_tee() — re-resolves TEE and replaces the HTTP client (used on retry)
  • _call_with_tee_retry() — retries once on connection failure by picking a new TEE
  • Streaming retry logic in _chat_stream() (chunks_yielded guard + _refresh_tee on first-attempt failure)
  • TypeVar, Callable, Awaitable imports
  • Instance variables self._rpc_url, self._tee_registry_address, self._llm_server_url

PR fixes kept on top:

  • import json as _json + all json.loads / json.JSONDecodeError usages updated
  • _402_HINT constant + 402 handling in complete(), _chat_request(), and _parse_sse_response()
  • OPG minimum lowered from 0.10.05

@amathxbt

Copy link
Copy Markdown
ContributorAuthor

Merged upstream main into the branch and resolved all conflicts:

  • src/opengradient/client/llm.py (SHA af38386): rebased on the latest main (which added TEE retry/refresh logic, _call_with_tee_retry, _connect_tee, _refresh_tee, new SSL/cert-rotation handling) while retaining all PR-specific changes — import json as _json, _402_HINT constant, HTTP 402 error handling in complete(), chat(), and _parse_sse_response(), and the 0.05 OPG minimum.

  • tests/llm_test.py: already contained the new TestTeeRetry*, TestRefreshTeeAndReset, and TestTeeCertRotation classes from main. The PR's 402-hint tests (test_http_402_raises_hint × 2, test_stream_402_raises_hint) and the FakeHTTPClient status-code fix were already in place.

The PR is now conflict-free (mergeable: true).

@amathxbt

Copy link
Copy Markdown
ContributorAuthor

@adambalogh please can you done this it's been long time no more comments

@amathxbt
amathxbt requested a review from adambaloghApril 4, 2026 22:53
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants

@amathxbt@adambalogh
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: 5 critical bugs — TEE randomization, HTTP 402 hint, int dtype, inference None crash, gas estimation - #201

Open
amathxbt wants to merge 27 commits into
OpenGradient:mainfrom
amathxbt:fix/critical-5-bugs-tee-402-dtype-inference-gas
Open

fix: 5 critical bugs — TEE randomization, HTTP 402 hint, int dtype, inference None crash, gas estimation#201
amathxbt wants to merge 27 commits into
OpenGradient:mainfrom
amathxbt:fix/critical-5-bugs-tee-402-dtype-inference-gas

Conversation

@amathxbt

Copy link
Copy Markdown
Contributor

Summary

This PR fixes 5 critical bugs identified from open issues and code audit, spanning TEE selection, LLM error handling, type conversions, inference safety, and gas estimation.


Bug 1 — TEE always picks the same node (closes#200)

File:src/opengradient/client/tee_registry.py

Root cause:get_llm_tee() always returned tees[0], routing 100% of traffic to a single TEE with zero load distribution or failover.

Fix: Replace tees[0] with random.choice(tees) so each LLM() construction independently selects from the full pool of active, registry-verified TEEs.

# Beforereturntees[0]
# Afterselected=random.choice(tees)
logger.debug("Selected TEE %s from %d active LLM proxy TEE(s)", selected.tee_id, len(tees))
returnselected

Bug 2 — HTTP 402 swallowed as cryptic RuntimeError (closes#188)

File:src/opengradient/client/llm.py

Root cause: All HTTP errors (including 402 Payment Required) were caught by except Exception and re-raised as a generic RuntimeError("TEE LLM chat failed: ..."). Users had no idea they needed to call ensure_opg_approval() first.

Fix: Add import httpx and intercept httpx.HTTPStatusErrorbefore the generic handler. When status_code == 402, raise a RuntimeError with an explicit, actionable message pointing to ensure_opg_approval(). Applied to _chat_request(), completion(), and _parse_sse_response() (streaming path).

# New constant_402_HINT= (
"Payment required (HTTP 402): your wallet may have insufficient OPG token allowance. ""Call llm.ensure_opg_approval(opg_amount=<amount>) to approve Permit2 spending ""before making requests. Minimum amount is 0.05 OPG."
)
# In _chat_request / completion / _parse_sse_response:excepthttpx.HTTPStatusErrorase:
ife.response.status_code==402:
raiseRuntimeError(_402_HINT) fromeraiseRuntimeError(f"TEE LLM chat failed: {e}") frome

Bug 3 — Fixed-point int returns np.float32 instead of int (partially closes#103)

File:src/opengradient/client/_conversions.py

Root cause:convert_to_float32(value, decimals) always returned np.float32, even when decimals == 0 (i.e., the original value was an integer). Users expecting integer outputs received np.float32 and had to cast manually.

Fix: Add convert_fixed_point_to_python(value, decimals) that returns int when decimals == 0 and np.float32 otherwise. np.array() then infers the correct dtype (int64 vs float32) automatically. The old convert_to_float32 is kept as a deprecated alias for backward compatibility.

defconvert_fixed_point_to_python(value: int, decimals: int) ->Union[int, np.float32]:
ifdecimals==0:
returnint(value) # ← integer tensor stays integerreturnnp.float32(Decimal(value) / (10**Decimal(decimals)))

Bug 4 — infer() crashes with AttributeError when inference node returns None

File:src/opengradient/client/alpha.py

Root cause:_get_inference_result_from_node() returns None when the node has no result yet. This None was passed directly to convert_to_model_output(), which calls event_data.get("output", {})AttributeError: NoneType object has no attribute get. Additionally, if ModelInferenceEvent logs were empty, parsed_logs[0] caused an IndexError.

Fix:

# Guard 1: empty precompile logsifnotprecompile_logs:
raiseRuntimeError("ModelInferenceEvent not found in transaction logs.")
# Guard 2: None from inference nodeifinference_resultisNone:
raiseRuntimeError(
f"Inference node returned no result for inference ID {inference_id!r}. ""The result may not be available yet — retry after a short delay."
)

Bug 5 — run_workflow() uses hardcoded 30M gas

File:src/opengradient/client/alpha.py

Root cause:run_workflow() hardcoded gas=30000000, unlike infer() which correctly uses estimate_gas(). This is both wasteful (users overpay) and fragile on networks with lower block gas limits.

Fix: Call estimate_gas() first and multiply by 3 for headroom. Fall back to 30M only if estimation itself fails.

try:
estimated_gas=run_function.estimate_gas({"from": self._wallet_account.address})
gas_limit=int(estimated_gas*3)
exceptException:
gas_limit=30000000# fallback

Also replaces the hardcoded timeout=60 in new_workflow() with the INFERENCE_TX_TIMEOUT constant.


Files Changed

FileBugs Fixed
src/opengradient/client/tee_registry.pyBug 1 (TEE randomization)
src/opengradient/client/llm.pyBug 2 (HTTP 402 hint)
src/opengradient/client/_conversions.pyBug 3 (int dtype)
src/opengradient/client/alpha.pyBugs 4 & 5 (None guard + gas)

Related Issues

Closes#200
Closes#188
Partially closes#103

Previously get_llm_tee() always returned tees[0], the first TEE in the
registry list. This caused all clients to hit the same TEE, providing no
load distribution and no resilience when that TEE starts failing.
Fix: use random.choice(tees) so each LLM() construction independently
picks from all currently active TEEs. Successive retries or re-initializations
will therefore naturally spread across the healthy pool.
ClosesOpenGradient#200
Previously any HTTP error from the TEE (including 402 Payment Required)
was silently wrapped into a generic RuntimeError("TEE LLM chat failed: ...").
This caused the confusing traceback seen in issue OpenGradient#188 — the real cause
(insufficient OPG allowance) was buried inside the exception message.
Fix:
- Add `import httpx` and intercept httpx.HTTPStatusError before the
generic `except Exception` handler in both _chat_request() and
completion().
- When status == 402, raise a RuntimeError with a clear, actionable hint
telling the user to call llm.ensure_opg_approval(opg_amount=<amount>).
- Also handle 402 in _parse_sse_response() for the streaming path.
- All other HTTP errors continue to surface as before.
ClosesOpenGradient#188
Previously convert_to_float32() always returned np.float32 regardless of
the decimals field, forcing callers to manually cast integer results
(issue OpenGradient#103 — add proper type conversions from Solidity contract to Python).
Fix:
- Add convert_fixed_point_to_python(value, decimals) that returns int when
decimals == 0 and np.float32 otherwise. np.array() automatically picks
the correct dtype (int64 vs float32) based on the element types.
- Update both convert_to_model_output() and convert_array_to_model_output()
to call the new function.
- Keep convert_to_float32() as a deprecated backward-compatible alias so
any external code that imports it directly continues to work.
Partially closesOpenGradient#103
… in run_workflow
Bug 4 — None crash in infer():
_get_inference_result_from_node() can legitimately return None when the
inference node has no result yet. Previously this None was passed
directly to convert_to_model_output(), which calls event_data.get(),
causing an AttributeError: 'NoneType' object has no attribute 'get'.
Fix: after calling _get_inference_result_from_node(), check for None
and raise a clear RuntimeError with a human-readable message and the
inference_id so callers know what to retry.
Also guard the precompile log fetch: if parsed_logs is empty, raise
RuntimeError instead of crashing with an IndexError on parsed_logs[0].
Bug 5 — hardcoded 30M gas in run_workflow():
run_workflow() built the transaction with gas=30000000 (30 million)
unconditionally. This is wasteful (users overpay for gas) and can
fail on networks with a lower block gas limit.
Fix: call estimate_gas() first (consistent with infer()), then multiply
by 3 for headroom. Fall back to 30M only if estimation itself fails.
Also fixes new_workflow() deploy to use INFERENCE_TX_TIMEOUT constant
instead of the previous hardcoded 60 seconds.
@amathxbt

Copy link
Copy Markdown
ContributorAuthor

Hey @adambalogh 👋 — this PR addresses 5 critical bugs found during a code audit, 3 of which directly close or partially close open issues you and the team have filed:

#BugIssue
1get_llm_tee() always returned tees[0] — no randomization or load distributionCloses #200
2HTTP 402 from TEE was swallowed as a cryptic RuntimeError with no guidanceCloses #188
3Fixed-point values with decimals=0 returned np.float32 instead of intPartially closes #103
4infer() crashed with AttributeError when the inference node returned NoneCode audit
5run_workflow() hardcoded gas=30000000 instead of using estimate_gas()Code audit

All changes are backward-compatible. Happy to adjust anything before review! 🙏

@adambalogh

Copy link
Copy Markdown
Collaborator

Hey @amathxbt thanks for your contributions, I added it to my list to review, will get back to you tomorrow!

@amathxbt

Copy link
Copy Markdown
ContributorAuthor

Hey @amathxbt thanks for your contributions, I added it to my list to review, will get back to you tomorrow!

Ok @adambalogh thank you see you tomorrow

"""LLM chat and completion via TEE-verified execution with x402 payments."""

import json
import json as _json

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the reason for this?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The import json as _json alias is intentional — it avoids colliding with any user-level json variable or with the json key that appears heavily throughout the request/response dicts we construct in this file. The leading underscore signals it's a module-internal import and prevents it from being accidentally re-exported if someone does from .llm import *. No behaviour change; purely a namespace-safety convention.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses several correctness and UX issues in the OpenGradient Python SDK across TEE selection, LLM error reporting, fixed-point conversions, inference robustness, and workflow gas handling.

Changes:

  • Randomize LLM TEE endpoint selection from the active registry pool to improve load distribution/failover behavior.
  • Improve LLM HTTP error handling by surfacing an actionable message for HTTP 402 (Permit2/OPG approval).
  • Add guards in Alpha.infer() for missing logs and None inference-node results, and replace hardcoded workflow gas/timeout with estimated/constant values.
  • Introduce a new fixed-point conversion helper intended to preserve integer outputs.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

FileDescription
src/opengradient/client/tee_registry.pyRandomizes get_llm_tee() selection and adds debug logging.
src/opengradient/client/llm.pyAdds explicit HTTP 402 hint handling for completion/chat/streaming.
src/opengradient/client/_conversions.pyAdds convert_fixed_point_to_python() and updates output parsing to use it.
src/opengradient/client/alpha.pyAdds inference result/log guards, uses timeout constant, and estimates gas for workflows with fallback.

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

Comment on lines +131 to +133
selected = random.choice(tees)
logger.debug(
"Selected TEE %s (endpoint=%s) from %d active LLM proxy TEE(s)",

CopilotAIMar 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change makes get_llm_tee() non-deterministic, which will break/flakify the existing unit test TestGetLlmTee.test_returns_first_active_tee (it asserts the first endpoint). Update the tests to patch random.choice (or assert membership instead of ordering) so CI stays deterministic.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to fix this

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed ✅ — committed in tests/tee_registry_test.py. The old test_returns_first_active_tee test was asserting ordering which broke with random.choice. Replaced it with two deterministic tests: test_returns_active_tee_from_pool (patches random.choice via unittest.mock.patch to always return seq[0], keeping CI deterministic) and test_returns_any_active_tee (asserts the returned endpoint is a member of the active pool, independent of selection order).

Comment on lines +39 to +56
def convert_fixed_point_to_python(value: int, decimals: int) -> Union[int, np.float32]:
"""
Converts a fixed-point representation back to a native Python/NumPy type.

Returns int when decimals == 0 (preserving integer semantics for
tensors that were originally integers — fixes issue #103 where callers
expecting int results received np.float32 and had to cast manually).
Returns np.float32 for all other cases.

Args:
value: The integer significand stored on-chain.
decimals: The scale factor exponent (value / 10**decimals).

Returns:
int if decimals == 0, np.float32 otherwise.
"""
if decimals == 0:
return int(value)

CopilotAIMar 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Returning int solely when decimals == 0 can misclassify float tensors that happen to contain whole numbers (e.g., 1.0 normalizes to decimals==0). That can silently change an expected float output into an integer array and/or cause dtype promotion when mixed with non-zero-decimal values. Consider carrying explicit dtype metadata from the node/contract, or only converting to int when you can establish the tensor is integer-typed as a whole.

Suggested change
defconvert_fixed_point_to_python(value: int, decimals: int) ->Union[int, np.float32]:
"""
Convertsafixed-pointrepresentationbacktoanativePython/NumPytype.
Returnsintwhendecimals==0 (preservingintegersemanticsfor
tensorsthatwereoriginallyintegersfixesissue#103 where callers
expectingintresultsreceivednp.float32andhadtocastmanually).
Returnsnp.float32forallothercases.
Args:
value: Theintegersignificandstoredon-chain.
decimals: Thescalefactorexponent (value/10**decimals).
Returns:
intifdecimals==0, np.float32otherwise.
"""
ifdecimals==0:
returnint(value)
defconvert_fixed_point_to_python(value: int, decimals: int) ->np.float32:
"""
Convertsafixed-pointrepresentationbacktoaNumPyfloat32.
Thisfunctionisintentionallytype-stableandalwaysreturnsnp.float32,
regardlessofthevalueof`decimals`. Callersthatrequireinteger
semanticsshouldperformanexplicitcast (e.g., int(...)) basedon
theirowndtypemetadataorapplicationlogic.
Args:
value: Theintegersignificandstoredon-chain.
decimals: Thescalefactorexponent (value/10**decimals).
Returns:
np.float32correspondingto`value / 10**decimals`.
"""

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there are reason we are mixing up float and int? might make the handling of results more difficult

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed ✅ — committed in src/opengradient/client/_conversions.py. convert_fixed_point_to_python is now type-stable: it always returns np.float32 regardless of decimals, matching Copilot's suggestion. The int-when-decimals==0 branch has been removed. The return type annotation is updated from Union[int, np.float32]np.float32. Callers that genuinely need integer semantics can do an explicit int(...) cast based on their own dtype metadata.

Comment on lines +87 to +91
logging.debug(f" Converting {tensor_data} to np array")
tensor_data = np.array(tensor_data)

if isinstance(tensor_data, (str, int, float)):
logging.debug(f"\tConverting single entry {tensor_data} to a list")
logging.debug(f" Converting single entry {tensor_data} to a list")

CopilotAIMar 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This debug string includes a literal tab character inside the f-string. Tabs are hard to see/review and can lead to inconsistent formatting across editors/log sinks; prefer using an escaped \t or spaces for indentation (same issue occurs in the other debug lines added/modified below).

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please fix

@amathxbtamathxbtMar 26, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed ✅ committed in src/opengradient/client/_conversions.py. Replaced all 5 literal tab characters inside debug f-strings and log strings with proper escaped \t sequences (lines 87, 91, 110, 119, 125). No more invisible tabs in the source.

Comment threadsrc/opengradient/client/alpha.py Outdated
Comment threadsrc/opengradient/client/llm.py
@adambalogh

Copy link
Copy Markdown
Collaborator

@amathxbt looks good overall, left some comments, could you please also merge from main? i think there are changes in the llm.py that need to be resolved

@amathxbt

Copy link
Copy Markdown
ContributorAuthor

Done @adambalogh ✅ — merged main into llm.py (commit 8896eac).

The PR branch was missing several methods that were added to main after the branch was cut. I preserved all of them and kept the PR's bug fixes layered on top:

Restored from main:

  • _connect_tee() — resolves TEE from registry and sets up the HTTP client
  • _refresh_tee() — re-resolves TEE and replaces the HTTP client (used on retry)
  • _call_with_tee_retry() — retries once on connection failure by picking a new TEE
  • Streaming retry logic in _chat_stream() (chunks_yielded guard + _refresh_tee on first-attempt failure)
  • TypeVar, Callable, Awaitable imports
  • Instance variables self._rpc_url, self._tee_registry_address, self._llm_server_url

PR fixes kept on top:

  • import json as _json + all json.loads / json.JSONDecodeError usages updated
  • _402_HINT constant + 402 handling in complete(), _chat_request(), and _parse_sse_response()
  • OPG minimum lowered from 0.10.05

@amathxbt

Copy link
Copy Markdown
ContributorAuthor

Merged upstream main into the branch and resolved all conflicts:

  • src/opengradient/client/llm.py (SHA af38386): rebased on the latest main (which added TEE retry/refresh logic, _call_with_tee_retry, _connect_tee, _refresh_tee, new SSL/cert-rotation handling) while retaining all PR-specific changes — import json as _json, _402_HINT constant, HTTP 402 error handling in complete(), chat(), and _parse_sse_response(), and the 0.05 OPG minimum.

  • tests/llm_test.py: already contained the new TestTeeRetry*, TestRefreshTeeAndReset, and TestTeeCertRotation classes from main. The PR's 402-hint tests (test_http_402_raises_hint × 2, test_stream_402_raises_hint) and the FakeHTTPClient status-code fix were already in place.

The PR is now conflict-free (mergeable: true).

@amathxbt

Copy link
Copy Markdown
ContributorAuthor

@adambalogh please can you done this it's been long time no more comments

@amathxbt
amathxbt requested a review from adambaloghApril 4, 2026 22:53
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants

@amathxbt@adambalogh
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

fix: 5 critical bugs — TEE randomization, HTTP 402 hint, int dtype, inference None crash, gas estimation - #201

Open
amathxbt wants to merge 27 commits into
OpenGradient:mainfrom
amathxbt:fix/critical-5-bugs-tee-402-dtype-inference-gas
Open

fix: 5 critical bugs — TEE randomization, HTTP 402 hint, int dtype, inference None crash, gas estimation#201
amathxbt wants to merge 27 commits into
OpenGradient:mainfrom
amathxbt:fix/critical-5-bugs-tee-402-dtype-inference-gas

Conversation

@amathxbt

Copy link
Copy Markdown
Contributor

Summary

This PR fixes 5 critical bugs identified from open issues and code audit, spanning TEE selection, LLM error handling, type conversions, inference safety, and gas estimation.


Bug 1 — TEE always picks the same node (closes#200)

File:src/opengradient/client/tee_registry.py

Root cause:get_llm_tee() always returned tees[0], routing 100% of traffic to a single TEE with zero load distribution or failover.

Fix: Replace tees[0] with random.choice(tees) so each LLM() construction independently selects from the full pool of active, registry-verified TEEs.

# Beforereturntees[0]
# Afterselected=random.choice(tees)
logger.debug("Selected TEE %s from %d active LLM proxy TEE(s)", selected.tee_id, len(tees))
returnselected

Bug 2 — HTTP 402 swallowed as cryptic RuntimeError (closes#188)

File:src/opengradient/client/llm.py

Root cause: All HTTP errors (including 402 Payment Required) were caught by except Exception and re-raised as a generic RuntimeError("TEE LLM chat failed: ..."). Users had no idea they needed to call ensure_opg_approval() first.

Fix: Add import httpx and intercept httpx.HTTPStatusErrorbefore the generic handler. When status_code == 402, raise a RuntimeError with an explicit, actionable message pointing to ensure_opg_approval(). Applied to _chat_request(), completion(), and _parse_sse_response() (streaming path).

# New constant_402_HINT= (
"Payment required (HTTP 402): your wallet may have insufficient OPG token allowance. ""Call llm.ensure_opg_approval(opg_amount=<amount>) to approve Permit2 spending ""before making requests. Minimum amount is 0.05 OPG."
)
# In _chat_request / completion / _parse_sse_response:excepthttpx.HTTPStatusErrorase:
ife.response.status_code==402:
raiseRuntimeError(_402_HINT) fromeraiseRuntimeError(f"TEE LLM chat failed: {e}") frome

Bug 3 — Fixed-point int returns np.float32 instead of int (partially closes#103)

File:src/opengradient/client/_conversions.py

Root cause:convert_to_float32(value, decimals) always returned np.float32, even when decimals == 0 (i.e., the original value was an integer). Users expecting integer outputs received np.float32 and had to cast manually.

Fix: Add convert_fixed_point_to_python(value, decimals) that returns int when decimals == 0 and np.float32 otherwise. np.array() then infers the correct dtype (int64 vs float32) automatically. The old convert_to_float32 is kept as a deprecated alias for backward compatibility.

defconvert_fixed_point_to_python(value: int, decimals: int) ->Union[int, np.float32]:
ifdecimals==0:
returnint(value) # ← integer tensor stays integerreturnnp.float32(Decimal(value) / (10**Decimal(decimals)))

Bug 4 — infer() crashes with AttributeError when inference node returns None

File:src/opengradient/client/alpha.py

Root cause:_get_inference_result_from_node() returns None when the node has no result yet. This None was passed directly to convert_to_model_output(), which calls event_data.get("output", {})AttributeError: NoneType object has no attribute get. Additionally, if ModelInferenceEvent logs were empty, parsed_logs[0] caused an IndexError.

Fix:

# Guard 1: empty precompile logsifnotprecompile_logs:
raiseRuntimeError("ModelInferenceEvent not found in transaction logs.")
# Guard 2: None from inference nodeifinference_resultisNone:
raiseRuntimeError(
f"Inference node returned no result for inference ID {inference_id!r}. ""The result may not be available yet — retry after a short delay."
)

Bug 5 — run_workflow() uses hardcoded 30M gas

File:src/opengradient/client/alpha.py

Root cause:run_workflow() hardcoded gas=30000000, unlike infer() which correctly uses estimate_gas(). This is both wasteful (users overpay) and fragile on networks with lower block gas limits.

Fix: Call estimate_gas() first and multiply by 3 for headroom. Fall back to 30M only if estimation itself fails.

try:
estimated_gas=run_function.estimate_gas({"from": self._wallet_account.address})
gas_limit=int(estimated_gas*3)
exceptException:
gas_limit=30000000# fallback

Also replaces the hardcoded timeout=60 in new_workflow() with the INFERENCE_TX_TIMEOUT constant.


Files Changed

FileBugs Fixed
src/opengradient/client/tee_registry.pyBug 1 (TEE randomization)
src/opengradient/client/llm.pyBug 2 (HTTP 402 hint)
src/opengradient/client/_conversions.pyBug 3 (int dtype)
src/opengradient/client/alpha.pyBugs 4 & 5 (None guard + gas)

Related Issues

Closes#200
Closes#188
Partially closes#103

Previously get_llm_tee() always returned tees[0], the first TEE in the
registry list. This caused all clients to hit the same TEE, providing no
load distribution and no resilience when that TEE starts failing.
Fix: use random.choice(tees) so each LLM() construction independently
picks from all currently active TEEs. Successive retries or re-initializations
will therefore naturally spread across the healthy pool.
ClosesOpenGradient#200
Previously any HTTP error from the TEE (including 402 Payment Required)
was silently wrapped into a generic RuntimeError("TEE LLM chat failed: ...").
This caused the confusing traceback seen in issue OpenGradient#188 — the real cause
(insufficient OPG allowance) was buried inside the exception message.
Fix:
- Add `import httpx` and intercept httpx.HTTPStatusError before the
generic `except Exception` handler in both _chat_request() and
completion().
- When status == 402, raise a RuntimeError with a clear, actionable hint
telling the user to call llm.ensure_opg_approval(opg_amount=<amount>).
- Also handle 402 in _parse_sse_response() for the streaming path.
- All other HTTP errors continue to surface as before.
ClosesOpenGradient#188
Previously convert_to_float32() always returned np.float32 regardless of
the decimals field, forcing callers to manually cast integer results
(issue OpenGradient#103 — add proper type conversions from Solidity contract to Python).
Fix:
- Add convert_fixed_point_to_python(value, decimals) that returns int when
decimals == 0 and np.float32 otherwise. np.array() automatically picks
the correct dtype (int64 vs float32) based on the element types.
- Update both convert_to_model_output() and convert_array_to_model_output()
to call the new function.
- Keep convert_to_float32() as a deprecated backward-compatible alias so
any external code that imports it directly continues to work.
Partially closesOpenGradient#103
… in run_workflow
Bug 4 — None crash in infer():
_get_inference_result_from_node() can legitimately return None when the
inference node has no result yet. Previously this None was passed
directly to convert_to_model_output(), which calls event_data.get(),
causing an AttributeError: 'NoneType' object has no attribute 'get'.
Fix: after calling _get_inference_result_from_node(), check for None
and raise a clear RuntimeError with a human-readable message and the
inference_id so callers know what to retry.
Also guard the precompile log fetch: if parsed_logs is empty, raise
RuntimeError instead of crashing with an IndexError on parsed_logs[0].
Bug 5 — hardcoded 30M gas in run_workflow():
run_workflow() built the transaction with gas=30000000 (30 million)
unconditionally. This is wasteful (users overpay for gas) and can
fail on networks with a lower block gas limit.
Fix: call estimate_gas() first (consistent with infer()), then multiply
by 3 for headroom. Fall back to 30M only if estimation itself fails.
Also fixes new_workflow() deploy to use INFERENCE_TX_TIMEOUT constant
instead of the previous hardcoded 60 seconds.
@amathxbt

Copy link
Copy Markdown
ContributorAuthor

Hey @adambalogh 👋 — this PR addresses 5 critical bugs found during a code audit, 3 of which directly close or partially close open issues you and the team have filed:

#BugIssue
1get_llm_tee() always returned tees[0] — no randomization or load distributionCloses #200
2HTTP 402 from TEE was swallowed as a cryptic RuntimeError with no guidanceCloses #188
3Fixed-point values with decimals=0 returned np.float32 instead of intPartially closes #103
4infer() crashed with AttributeError when the inference node returned NoneCode audit
5run_workflow() hardcoded gas=30000000 instead of using estimate_gas()Code audit

All changes are backward-compatible. Happy to adjust anything before review! 🙏

@adambalogh

Copy link
Copy Markdown
Collaborator

Hey @amathxbt thanks for your contributions, I added it to my list to review, will get back to you tomorrow!

@amathxbt

Copy link
Copy Markdown
ContributorAuthor

Hey @amathxbt thanks for your contributions, I added it to my list to review, will get back to you tomorrow!

Ok @adambalogh thank you see you tomorrow

"""LLM chat and completion via TEE-verified execution with x402 payments."""

import json
import json as _json

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the reason for this?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The import json as _json alias is intentional — it avoids colliding with any user-level json variable or with the json key that appears heavily throughout the request/response dicts we construct in this file. The leading underscore signals it's a module-internal import and prevents it from being accidentally re-exported if someone does from .llm import *. No behaviour change; purely a namespace-safety convention.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses several correctness and UX issues in the OpenGradient Python SDK across TEE selection, LLM error reporting, fixed-point conversions, inference robustness, and workflow gas handling.

Changes:

  • Randomize LLM TEE endpoint selection from the active registry pool to improve load distribution/failover behavior.
  • Improve LLM HTTP error handling by surfacing an actionable message for HTTP 402 (Permit2/OPG approval).
  • Add guards in Alpha.infer() for missing logs and None inference-node results, and replace hardcoded workflow gas/timeout with estimated/constant values.
  • Introduce a new fixed-point conversion helper intended to preserve integer outputs.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

FileDescription
src/opengradient/client/tee_registry.pyRandomizes get_llm_tee() selection and adds debug logging.
src/opengradient/client/llm.pyAdds explicit HTTP 402 hint handling for completion/chat/streaming.
src/opengradient/client/_conversions.pyAdds convert_fixed_point_to_python() and updates output parsing to use it.
src/opengradient/client/alpha.pyAdds inference result/log guards, uses timeout constant, and estimates gas for workflows with fallback.

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

Comment on lines +131 to +133
selected = random.choice(tees)
logger.debug(
"Selected TEE %s (endpoint=%s) from %d active LLM proxy TEE(s)",

CopilotAIMar 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change makes get_llm_tee() non-deterministic, which will break/flakify the existing unit test TestGetLlmTee.test_returns_first_active_tee (it asserts the first endpoint). Update the tests to patch random.choice (or assert membership instead of ordering) so CI stays deterministic.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to fix this

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed ✅ — committed in tests/tee_registry_test.py. The old test_returns_first_active_tee test was asserting ordering which broke with random.choice. Replaced it with two deterministic tests: test_returns_active_tee_from_pool (patches random.choice via unittest.mock.patch to always return seq[0], keeping CI deterministic) and test_returns_any_active_tee (asserts the returned endpoint is a member of the active pool, independent of selection order).

Comment on lines +39 to +56
def convert_fixed_point_to_python(value: int, decimals: int) -> Union[int, np.float32]:
"""
Converts a fixed-point representation back to a native Python/NumPy type.

Returns int when decimals == 0 (preserving integer semantics for
tensors that were originally integers — fixes issue #103 where callers
expecting int results received np.float32 and had to cast manually).
Returns np.float32 for all other cases.

Args:
value: The integer significand stored on-chain.
decimals: The scale factor exponent (value / 10**decimals).

Returns:
int if decimals == 0, np.float32 otherwise.
"""
if decimals == 0:
return int(value)

CopilotAIMar 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Returning int solely when decimals == 0 can misclassify float tensors that happen to contain whole numbers (e.g., 1.0 normalizes to decimals==0). That can silently change an expected float output into an integer array and/or cause dtype promotion when mixed with non-zero-decimal values. Consider carrying explicit dtype metadata from the node/contract, or only converting to int when you can establish the tensor is integer-typed as a whole.

Suggested change
defconvert_fixed_point_to_python(value: int, decimals: int) ->Union[int, np.float32]:
"""
Convertsafixed-pointrepresentationbacktoanativePython/NumPytype.
Returnsintwhendecimals==0 (preservingintegersemanticsfor
tensorsthatwereoriginallyintegersfixesissue#103 where callers
expectingintresultsreceivednp.float32andhadtocastmanually).
Returnsnp.float32forallothercases.
Args:
value: Theintegersignificandstoredon-chain.
decimals: Thescalefactorexponent (value/10**decimals).
Returns:
intifdecimals==0, np.float32otherwise.
"""
ifdecimals==0:
returnint(value)
defconvert_fixed_point_to_python(value: int, decimals: int) ->np.float32:
"""
Convertsafixed-pointrepresentationbacktoaNumPyfloat32.
Thisfunctionisintentionallytype-stableandalwaysreturnsnp.float32,
regardlessofthevalueof`decimals`. Callersthatrequireinteger
semanticsshouldperformanexplicitcast (e.g., int(...)) basedon
theirowndtypemetadataorapplicationlogic.
Args:
value: Theintegersignificandstoredon-chain.
decimals: Thescalefactorexponent (value/10**decimals).
Returns:
np.float32correspondingto`value / 10**decimals`.
"""

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there are reason we are mixing up float and int? might make the handling of results more difficult

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed ✅ — committed in src/opengradient/client/_conversions.py. convert_fixed_point_to_python is now type-stable: it always returns np.float32 regardless of decimals, matching Copilot's suggestion. The int-when-decimals==0 branch has been removed. The return type annotation is updated from Union[int, np.float32]np.float32. Callers that genuinely need integer semantics can do an explicit int(...) cast based on their own dtype metadata.

Comment on lines +87 to +91
logging.debug(f" Converting {tensor_data} to np array")
tensor_data = np.array(tensor_data)

if isinstance(tensor_data, (str, int, float)):
logging.debug(f"\tConverting single entry {tensor_data} to a list")
logging.debug(f" Converting single entry {tensor_data} to a list")

CopilotAIMar 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This debug string includes a literal tab character inside the f-string. Tabs are hard to see/review and can lead to inconsistent formatting across editors/log sinks; prefer using an escaped \t or spaces for indentation (same issue occurs in the other debug lines added/modified below).

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please fix

@amathxbtamathxbtMar 26, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed ✅ committed in src/opengradient/client/_conversions.py. Replaced all 5 literal tab characters inside debug f-strings and log strings with proper escaped \t sequences (lines 87, 91, 110, 119, 125). No more invisible tabs in the source.

Comment threadsrc/opengradient/client/alpha.py Outdated
Comment threadsrc/opengradient/client/llm.py
@adambalogh

Copy link
Copy Markdown
Collaborator

@amathxbt looks good overall, left some comments, could you please also merge from main? i think there are changes in the llm.py that need to be resolved

@amathxbt

Copy link
Copy Markdown
ContributorAuthor

Done @adambalogh ✅ — merged main into llm.py (commit 8896eac).

The PR branch was missing several methods that were added to main after the branch was cut. I preserved all of them and kept the PR's bug fixes layered on top:

Restored from main:

  • _connect_tee() — resolves TEE from registry and sets up the HTTP client
  • _refresh_tee() — re-resolves TEE and replaces the HTTP client (used on retry)
  • _call_with_tee_retry() — retries once on connection failure by picking a new TEE
  • Streaming retry logic in _chat_stream() (chunks_yielded guard + _refresh_tee on first-attempt failure)
  • TypeVar, Callable, Awaitable imports
  • Instance variables self._rpc_url, self._tee_registry_address, self._llm_server_url

PR fixes kept on top:

  • import json as _json + all json.loads / json.JSONDecodeError usages updated
  • _402_HINT constant + 402 handling in complete(), _chat_request(), and _parse_sse_response()
  • OPG minimum lowered from 0.10.05

@amathxbt

Copy link
Copy Markdown
ContributorAuthor

Merged upstream main into the branch and resolved all conflicts:

  • src/opengradient/client/llm.py (SHA af38386): rebased on the latest main (which added TEE retry/refresh logic, _call_with_tee_retry, _connect_tee, _refresh_tee, new SSL/cert-rotation handling) while retaining all PR-specific changes — import json as _json, _402_HINT constant, HTTP 402 error handling in complete(), chat(), and _parse_sse_response(), and the 0.05 OPG minimum.

  • tests/llm_test.py: already contained the new TestTeeRetry*, TestRefreshTeeAndReset, and TestTeeCertRotation classes from main. The PR's 402-hint tests (test_http_402_raises_hint × 2, test_stream_402_raises_hint) and the FakeHTTPClient status-code fix were already in place.

The PR is now conflict-free (mergeable: true).

@amathxbt

Copy link
Copy Markdown
ContributorAuthor

@adambalogh please can you done this it's been long time no more comments

@amathxbt
amathxbt requested a review from adambaloghApril 4, 2026 22:53
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants

@amathxbt@adambalogh