|
1 | | -## 2026-02-25 — Add 9 gen\_\*\_v0 Go wrappers with conformance tests |
| 1 | +## 2026-02-25 — Review of: Add 9 gen\_\*\_v0 Go wrappers with conformance tests |
2 | 2 |
|
3 | | -**Done:** Implemented all 9 `gen_*_v0` Go wrappers on `*Runtime` (GenMetaCodeV0, GenTextCodeV0, |
4 | | -GenImageCodeV0, GenAudioCodeV0, GenVideoCodeV0, GenMixedCodeV0, GenDataCodeV0, GenInstanceCodeV0, |
5 | | -GenIsccCodeV0) with 4 memory helper functions (writeBytes, writeI32Slice, writeStringArray, |
6 | | -writeI32ArrayOfArrays) and a shared callStringResult helper. Added conformance tests covering all 46 |
7 | | -vectors from data.json across all 9 functions. |
| 3 | +**Verdict:** PASS |
8 | 4 |
|
9 | | -**Files changed:** |
| 5 | +**Summary:** All 9 `Gen*CodeV0` Go wrappers implemented on `*Runtime` with 4 memory helpers |
| 6 | +(`writeBytes`, `writeI32Slice`, `writeStringArray`, `writeI32ArrayOfArrays`) and a shared |
| 7 | +`callStringResult` helper. Conformance tests cover all 46 vectors from data.json across all 9 |
| 8 | +functions. Clean, well-structured code with proper error handling and memory cleanup on all paths. |
10 | 9 |
|
11 | | -- `packages/go/iscc.go`: Added `encoding/binary` import, 4 memory helpers (writeBytes, |
12 | | - writeI32Slice, writeStringArray, writeI32ArrayOfArrays), callStringResult helper, and 9 |
13 | | - gen\_\*\_v0 public methods. Updated package docstring. |
14 | | -- `packages/go/iscc_test.go`: Added 9 conformance test functions (TestGenMetaCodeV0 through |
15 | | - TestGenIsccCodeV0) with JSON vector parsing helpers. Each function iterates all vectors for its |
16 | | - function type using Go subtests. |
| 10 | +**Verification:** |
17 | 11 |
|
18 | | -**Verification:** All tests pass: |
| 12 | +- [x] `cd packages/go && CGO_ENABLED=0 go test -v -count=1 ./...` passes — 14 tests pass (5 existing |
| 13 | + \+ 9 new conformance tests covering 46 vectors) |
| 14 | +- [x] `cd packages/go && go vet ./...` exits 0 — clean |
| 15 | +- [x] All 9 `Gen*CodeV0` methods exist on `*Runtime` type — confirmed in iscc.go |
| 16 | +- [x] Each conformance test vector produces an ISCC string matching expected output from data.json — |
| 17 | + all 46 subtests pass |
| 18 | +- [x] `mise run check` — all 14 pre-commit hooks pass |
| 19 | +- [x] No quality gate circumvention — no lint suppressions, test skips, or hook weakening in diff |
19 | 20 |
|
20 | | -- `cd packages/go && CGO_ENABLED=0 go test -v -count=1 ./...` — 14 tests pass (5 existing + 9 new |
21 | | - conformance test functions covering 46 total vectors) |
22 | | -- `cd packages/go && go vet ./...` — clean |
23 | | -- `mise run check` — all 14 pre-commit hooks pass |
| 21 | +**Issues found:** |
| 22 | + |
| 23 | +- (none) |
24 | 24 |
|
25 | 25 | **Next:** Add Go CI job in `.github/workflows/ci.yml` to run `go test` and `go vet` in CI. The Go |
26 | 26 | module scaffold and all 9 gen functions are complete — CI integration is the natural next step to |
27 | | -protect against regressions. |
| 27 | +protect against regressions. After CI, consider adding the remaining 12 Tier 1 utility function |
| 28 | +wrappers (text utilities, algorithm primitives, streaming types). |
28 | 29 |
|
29 | 30 | **Notes:** |
30 | 31 |
|
31 | | -- Empty slice handling required careful treatment: `iscc_alloc(0)` returns a dangling pointer with |
32 | | - alignment 1, which is fine for `*const u8` (bytes) but causes `slice::from_raw_parts` to panic |
33 | | - for `*const i32` (requires alignment 4). Fixed `writeI32Slice` to allocate minimum 4 bytes for |
34 | | - empty slices, ensuring proper i32 alignment. `writeBytes` uses `iscc_alloc(0)` directly since u8 |
35 | | - has alignment 1. |
36 | | -- Meta test vectors with dict values (e.g., `{"some": "object"}`) are JSON-serialized in the Go test |
37 | | - before passing to the FFI, matching how the Rust conformance tests handle them. |
38 | | -- The `writeI32Slice` helper returns 3 values (ptr, allocSize, count) instead of 2, because |
39 | | - allocSize may differ from count\*4 when allocating a minimum 4 bytes for empty slices. This |
40 | | - ensures correct deallocation. |
| 32 | +- The `allocEntry` type is defined locally in both `writeStringArray` and `writeI32ArrayOfArrays` — |
| 33 | + minor duplication but acceptable since they're unexported local types. Could be extracted to a |
| 34 | + package-level type in a future cleanup. |
| 35 | +- Go tests create a new Runtime per test function (not per subtest). This is efficient since wazero |
| 36 | + module instantiation is the expensive part and subtests share it. |
| 37 | +- The `TextClean` method could be refactored to use `callStringResult` for consistency, but it was |
| 38 | + pre-existing code and out of scope for this iteration. |
0 commit comments