python

python — audit issues

Audited 2026-06-20. Repo: software/python. Criteria: completeness · tests · separation of concerns · verb-named functions · file size (<1000 lines) · organization.

Health summary

The binding is small, clean, and genuinely joyful: one 444-line module, a chainable Handle/Document API, native-value in/out, and 9 passing round-trip tests. But it surfaces only about two-thirds of the C ABI — the entire evaluator + node-registry API, world-matrix/visibility queries, all geometry-array accessors, and metadata enumeration are unbound, and get_matrix/get_spectrum are write-only (settable, never readable). Several mutators silently swallow C failure returns, so a bad connect/set looks like success. Separation of concerns, naming, file size, and packaging are all in good shape.

Issues

[HIGH] completeness — evaluator + node-registry API entirely unbound

• Where: kinogaki/__init__.py (no bindings); C ABI libraries/kinogaki-core/include/kinogaki/C.h:184-191
• Problem: kinogaki_node_registry_new/_free, kinogaki_evaluator_new/_free, and kinogaki_evaluator_output_float have no Python binding at all. The module exposes the registry-free kinogaki_evaluate (line 134) but not the node-graph evaluator that runs registered node behaviors. Python callers cannot evaluate a graph through a node registry — a whole capability tier of the C ABI is invisible.
• Fix: Bind the five functions; wrap as an Evaluator class (constructed from a Document + optional registry + time) with an evaluate_output(slot) -> float method and a __del__ that calls kinogaki_evaluator_free. Add a NodeRegistry class likewise. Add to __all__.

[HIGH] completeness — geometry-array accessors (float/float2/int) unbound

• Where: kinogaki/__init__.py (no bindings); C ABI C.h:147-152
• Problem: None of kinogaki_element_set_float2_array, set_int_array, set_float_array, get_float2_array, get_int_array, get_float_array are bound. Handle.set (line 192) only handles fixed 2/3/6/32-length sequences as float2/float3/matrix/spectrum; a variable-length point/index/scalar array (the KINOGAKI_TYPE_*_ARRAY types, C.h:39-41) cannot be authored or read. Any mesh/curve geometry — the point of these types — is unreachable from Python.
• Fix: Bind the six functions. Add Handle.set_point_array/set_index_array/set_scalar_array (or extend set to detect longer numeric sequences) and matching get_*_array readers. Getters use the cap=0 count-query then a second sized call (per the C contract at C.h:146).

[HIGH] completeness — matrix & spectrum are write-only (no public getter)

• Where: kinogaki/__init__.py:119-124 (_get_floatN binds 6 and 32) vs :260-266 (only get_float2/get_float3 exposed)
• Problem: _set_floatN (line 106) makes matrix(6) and spectrum(32) settable via Handle.set with a 6-/32-tuple, and _get_floatN binds get_matrix/get_spectrum — but no public method ever calls _get_floatN(key, 6, …) or (key, 32, …). So you can write a matrix or spectrum and never read it back through the API. Asymmetric and surprising.
• Fix: Add get_matrix/require_matrix and get_spectrum/require_spectrum methods (one-liners delegating to _get_floatN), parallel to get_float3.

[MEDIUM] completeness — worldmatrix & isvisible unbound

• Where: kinogaki/__init__.py (no bindings); C ABI C.h:178-179
• Problem: kinogaki_world_matrix (composed world transform at a time) and kinogaki_is_visible are not bound. These are the transform-query surface — a Python tool can author a transform hierarchy but cannot ask where an element ends up or whether it renders.
• Fix: Bind both; add Handle.world_matrix(time=0.0) -> tuple|None (6 floats) and Handle.is_visible(time=0.0) -> bool, or Document-level equivalents taking a path.

[MEDIUM] completeness — mutators silently swallow C failure returns

• Where: kinogaki/__init__.py:328-337 (connect/disconnect), :218-221 (set_meta), :192-216 (set)
• Problem: connect/disconnect discard the C bool result and unconditionally return self, so wiring a non-existent slot looks successful. set/set_meta likewise ignore the typed-setter return. append/rename correctly raise PrismError on False (lines 320, 345) — the inconsistency means some failures are loud and structurally identical ones are silent, which masks bugs (e.g. typo in a connect target).
• Fix: Check the returns; raise PrismError (or at minimum a documented bool) on failure, matching append/rename. Decide one policy and apply it uniformly across all mutators.

[MEDIUM] completeness — metadata & encoding enumeration unbound

• Where: kinogaki/__init__.py (no bindings); C ABI C.h:84 (kinogaki_detect_encoding), C.h:165-166 (kinogaki_element_metadata_count/_key)
• Problem: set_meta/get_meta exist (lines 218, 272) but you can only read metadata you already know the key of — kinogaki_element_metadata_count/kinogaki_element_metadata_key are unbound, so there is no way to list an element's metadata keys. kinogaki_detect_encoding (sniff text/binary/package without parsing) is also unbound.
• Fix: Bind the three; add Handle.metadata_keys() -> list[str] (or metadata() -> dict) and a module-level detect_encoding(data) -> str.

[MEDIUM] tests — large untested surface (errors, removal edges, evaluate-miss, repr/eq-negative)

• Where: tests/test_kinogaki.py (9 tests, all happy-path)
• Problem: Untested: rename (bound at __init__.py:343, never exercised), evaluate returning None for an unresolvable/UNKNOWN slot (line 436), get_* permissive defaults for mistyped (not just missing) values, PrismError from append on a missing parent / deserialize on garbage bytes (line 411) / compose_file on a cycle, Document.__eq__ negative case and __repr__, the matrix/spectrum round-trip, and set_meta overwriting. Because the failure-path mutators (above) silently swallow errors, no test would catch the regression.
• Fix: Add error-path tests (deserialize(b"junk") raises, append to missing parent raises, evaluate of a missing slot is None), a rename test, a matrix/spectrum round-trip, and an __eq__-False assertion. Migrate the self-runner toward pytest (the docstring already advertises python -m pytest).

[MEDIUM] completeness — Windows/Linux loadable but never built or shipped

• Where: build.sh:14-19 (macOS-only clang++ -dynamiclib), pyproject.toml:14-17 (Operating System :: MacOS only)
• Problem: _load_library (line 50-51) knows .so/.dll names, and package-data (pyproject.toml:29) globs them — but build.sh only emits a macOS universal .dylib, and the classifiers declare macOS only. The "ships a universal native library, nothing to compile" promise in the README holds on macOS exclusively; pip-installing on Linux/Windows yields an ImportError. Platform coverage is effectively macOS-only despite code that pretends otherwise.
• Fix: Either add Linux/Windows build paths (and wheels per-platform), or trim the .so/.dll handling and document macOS-only honestly. State the supported platforms in the README.

[LOW] separation — clean

• Where: kinogaki/__init__.py:75-136 (ctypes plumbing) vs :147-444 (Pythonic API)
• Problem: None — the ctypes layer (signatures via _bind, the _set_floatN/_get_floatN dicts, _utf8, _read_buffer) is cleanly cordoned at module top with leading-underscore names; the public Element/Connection/Handle/Document classes touch the C layer only through those helpers. byref/c_float never leak into the public signatures. No leakage worth flagging.
• Fix: none.

[LOW] naming — clean (one borderline noun)

• Where: kinogaki/__init__.py:349 (Document.has), :352 (element), :359 (elements), :369 (connections)
• Problem: Methods read as verbs or accepted-idiom predicates: append, edit, connect, disconnect, remove, rename, serialize, deserialize, evaluate, compose_file are all verbs; has(path) is a standard boolean-predicate idiom. The noun-named accessors element/elements/connections are collection getters where Python idiom tolerates nouns (cf. dict.keys); arguably elements()/connections() could be iter_* but this is stylistic, not a defect.
• Fix: none required; optionally rename element→find_element for verb-consistency.

[LOW] filesize — clean

• Where: kinogaki/__init__.py (444 lines), tests/test_kinogaki.py (136), build.sh (20)
• Problem: No source file exceeds 800 lines, let alone 1000. The binding is comfortably within a single readable module.
• Fix: none.

[LOW] organization — README install promise & loader env var; otherwise clean

• Where: README.md:39-45 vs build.sh; .gitignore (clean); pyproject.toml:28-29 (package-data correct)
• Problem: Layout, .gitignore (build artifacts + egg-info ignored; egg-info confirmed untracked), and package-data for the dylib are all correct. The only nits: the README's "pip install kinogaki — nothing to compile" claim is unbacked (no published wheel exists yet, and the build is macOS-only — see the platform issue), and the KINOGAKI_CORE_LIB override (__init__.py:53) is mentioned only as a one-line aside. No dead files.
• Fix: Soften the install section until wheels are published; document the supported-platform matrix and the KINOGAKI_CORE_LIB override more prominently.