task_110926_15 — sdk/operations/ Layer, Part 1: BL-9 (Santiago)

[umermjd11]

cc @Santiagocetran
Assigned. Full spec: Developer/tasks/task_110926_15.md

The task file has the full specification — the exact scope boundaries, the two candidate operations, and the deliverables checklist. This post is the summary and assignment notice.

Summary

Developer/BACK_LOG.md BL-9 flags that dincli/sdk/serialize.py's to_envelope() has zero production call sites, and dind's JOB_HANDLERS registry has exactly one job type ("demo", a no-op) — the sdk/operations/ layer the SDK interface proposal (§8) has always planned never got built. It was explicitly blocked on the wallet/session/tx signing keystone (BL-2, task_300726_8) — that's done now, confirmed directly against feat/din-sdk @ dc83f92 (session.py, wallet.py, tx.py all real and tested).

This task is Part 1, not the whole surface: build dincli/sdk/operations/ with one read-only operation, prove it round-trips through to_envelope(), wire it as a real (non-"demo") dind job type, and refactor one existing CLI command onto it with byte-identical output. Two candidates named in the task file (din-info, read-stake) — pick one, a second is a bonus. Write-only operations (task registration, GI lifecycle, scoring) stay explicitly out of scope for this task.

Blockers

None that block starting. Everything needed — session.py, contracts.py, manifest.py/runtime.py, serialize.py, errors.py — already exists on your own feat/din-sdk branch. No other PR needs to merge first, no sign-off needed to start. If a specific operation turns out to need business logic that isn't a clean lift out of dincli/cli/modelownerd/*.py (interactive prompts, mid-flow IPFS uploads), that's expected — pick a different candidate rather than fighting it; see the task file's scope boundaries.

[Santiagocetran]

Hi @umermjd11 — Part 1 is done, and thanks for scoping it this way. Proving the pattern on a small read-only slice was the right call; it surfaced a few things a bigger first bite would have buried.

What landed:

• sdk/operations/ with both candidates, get_platform_addresses and get_stake.
• A test proving both round-trip through to_envelope() — including every network in the shipped din_info.json, which matters more than it sounds: mainnet's addresses are "0x..." placeholders, and checksumming them raises. So platform addresses are emitted as configured, with a test pinning that.
• A real dind job type alongside "demo", so queue → operation → envelope works end to end.
• din-info and read-stake refactored, output byte-for-byte identical. read-stake turned out to have three call sites, not one — dintoken, aggregator and auditor share the helper — so all three are covered.
• BL-9 marked Part 1 done, not resolved, per your scoping.

One declared behaviour change: a network missing from din_info.json used to dump a KeyError traceback and now prints a clean message and exits 1. And one existing test had to be adapted — it asserted an internal call the refactor removes, so it couldn't pass unchanged. The nine new golden tests cover the output guarantee instead, verified against the pre-refactor commit rather than just asserted.

Next operation, since the task asks — your call on priority, but my suggestion would be get_model against the model registry. It's the first one returning a multi-field struct, so it actually exercises the encoder rules rather than a single scalar. download_manifest already calls getModel directly, so routing it through an operation gives the manifest path and the CLI one shared call site. And it's the seam P4-IDX2 replaces later, so extracting it now creates that boundary before anything is built on top of it.

Still read-only, so Part 2 stays low-risk. Write operations probably want to wait for Part 3 and the plan/apply design — happy to take a different one if you'd rather.

Branches pushed. SDK at 497 passed, daemon at 620, no failures on either.

[Santiagocetran]

Update — a deeper review over the delivered work turned up a few things I want to fix before treating Part 1 as closed.

The operations layer still lets some raw exceptions through instead of DinError: a din_info.json whose root isn't a mapping, and an invalid network name, which raises out of session resolution before the operation's own handling. The daemon job loop has a related problem — its error handler resolves session.network while building the error envelope, so when network resolution is the thing that failed, the handler raises too and the job is left running with nothing recorded. Reproduced both directly.

Also want to settle the two CLI compatibility points properly rather than carrying them as declared exceptions.

The operations layer, the envelope round-trip and the real job type all work on their tested paths. Both PRs are in draft while I fix the above, and I'll re-report once verified.

[Santiagocetran]

Following up — the remaining issues are fixed.

The job loop no longer depends on the failed session to record its own failure, so a job that fails during network resolution lands as failed with a stable code instead of stranding running and blocking everything queued behind it (e02e5f8). And the operations layer no longer leaks raw exceptions for a malformed din_info.json root or an invalid network name (8f48721).

On the two compatibility points I raised: rather than ask you to amend the criteria, I restored the original behaviour instead (27885d7). The operations still raise typed errors internally, but where the SDK wraps a real underlying exception the CLI now re-raises that original exception — so output and exception type match the pre-refactor baseline exactly. That covers contract construction failures, getStake() RPC failures, and unreadable or invalid din_info.json.

One case genuinely can't be restored: a network missing from din_info.json. That was a bare KeyError from unchecked dict indexing, so there's no prior exception object to hand back, and fabricating one would misrepresent what happened. It keeps the clean message and exit 1. That's the single remaining output difference — the same one I declared originally, with every other instance now removed rather than covered by a blanket exception, and pinned by a full-output assertion instead of a substring check.

The one thing I couldn't reconcile is the original stake test. It asserts the CLI called ctx.obj.get_deployed_din_stake_contract().functions.getStake(...) — the exact coupling the extraction was meant to remove. Restoring it would mean either monkeypatching the original test never did, or letting the SDK operation accept a caller-supplied contract instead of building its own, which inverts the SDK/CLI dependency the import-boundary test fails the build over. I left it replaced rather than forcing it: nine golden tests now pin complete stdout for every command path, plus complete error-path assertions, all verified against the pre-refactor commit. Stricter than the original on what that criterion is actually for, but not literal compliance — flagging it rather than claiming otherwise.

Branches pushed. SDK at 520 passed, daemon at 665, no failures on either.

[umeradl]

Heads-up: dincli dindao is now dincli dinrep — PR No. 175 landed on develop (merge 5864f7d, follow-up 8f9a90b). The DIN-Representative command group is renamed to match the role: dincli/cli/dindao.py → dincli/cli/dinrep.py, Documentation/public/roles/dindao.md → dinrep.md. Breaking, no alias — dincli dindao … now returns "No such command". Subcommands are unchanged (dincli dinrep deploy …, dincli dinrep add-slasher …, dincli dinrep registry …).

For this task: no code conflict (PR No. 31 / No. 32 don't modify dindao.py), but their docs refer to dindao.py (e.g. dindao.py::list_pending_requests, the deploy-flow note on tx_receipt.contractAddress). Please update those references to dinrep.py when you next merge develop.

[Santiagocetran]

Addressed in PRs #31 and #32. Both branches now include the latest develop, the dinrep rename, and updated SDK documentation references. Related compatibility fixes are included; unit tests and local-chain registry/fee-sweep checks pass.