fix(honua-gp): bind Result outputs to real artifacts, reject unsupported parameter semantics - #227
Open
mikemcdougall wants to merge 1 commit into
Open
fix(honua-gp): bind Result outputs to real artifacts, reject unsupported parameter semantics#227mikemcdougall wants to merge 1 commit into
mikemcdougall wants to merge 1 commit into
Conversation
…ted params run_layer_process registered a GP tool's output name as a session alias before the job ran and resolved it against the literal arcpy name, so a later GetCount/cursor/GP call could target layerId 0 or an unrelated source instead of the artifact the tool actually created. The output name is now only reserved (fail-fast duplicate check) before submission; the alias is bound to the job's real results-document artifact after the job succeeds, and a missing/empty output raises a typed ExecuteError instead of a successful Result. A failed/dismissed job never touches the alias map, so a prior output survives a failed overwrite attempt. Buffer/SpatialJoin/Dissolve/Project also validate every accepted parameter now: a nondefault line_side/line_end_type/method, join_operation/join_type/field_mapping/distance_field_name, statistics_fields/multi_part/unsplit_lines, or transform_method/in_coor_system/preserve_shape/max_deviation/vertical raises HonuaGpConfigurationError before submission instead of being silently ignored. Also fixes a latent NameError in resolve_layer_id (HonuaArcpyResolveError does not exist) that fired whenever a honua://services/<svc> URI with no layer segment reached it.
mikemcdougall
had a problem deploying
to
staging
September 10, 2026 21:18 — with
GitHub Actions
Failure
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for security reviews. Please try again later. |
mikemcdougall
had a problem deploying
to
staging
September 10, 2026 21:20 — with
GitHub Actions
Failure
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
run_layer_process(the shared entry point foranalysis.Buffer,analysis.SpatialJoin,management.Dissolve,management.Project)registered a tool's requested output name as a session alias before the
job ran, resolved against the literal arcpy name via
resolve_or_register_output/descriptor_mapping. That resolution had norelationship to what the server actually created and could default to
layerId=0of an unrelated dataset. A laterGetCount/SearchCursor/chained GP call against that output name could therefore silently read the
wrong data instead of the tool's real result.
Separately,
Buffer/SpatialJoin/Dissolve/Projectaccepted (andignored) several arcpy parameters that change tool semantics
(
line_side,join_operation,multi_part,preserve_shape, ...) withouttelling the caller.
Fix
submission (a fail-fast duplicate check honoring
env.overwriteOutput, mirroring arcpy) — no alias is published yet. Afterthe job reaches
successful, the real output artifact is extracted fromthe job's results document and that identity is what the alias, and
therefore every later
Result[0]/getOutput(0)/str(result)/downstreamcall, resolves through. A missing/empty output on a "successful" job now
raises a typed
ExecuteError(error_kind="missing_output")instead ofreturning a successful
Result.success, a failed/dismissed job leaves any prior alias under that name
completely untouched — no separate rollback bookkeeping required.
arcpy_paramsentry theprojector doesn't translate into a process input is now validated:
nondefault
line_side/line_end_type/method(Buffer),join_operation/join_type/field_mapping/distance_field_name(SpatialJoin),
statistics_fields/multi_part/unsplit_lines(Dissolve),and
transform_method/in_coor_system/preserve_shape/max_deviation/vertical(Project) now raise
HonuaGpConfigurationErrorbefore submission insteadof being silently dropped.
NameError.resolve_layer_idraisedHonuaArcpyResolveError, a name that doesn't exist in_errors.py(
HonuaGpResolveErroris the real class). This fired for anyhonua://services/<svc>URI with no layer segment — untested until now.Evidence
test_output_binding.py(new): drives a fullBuffer -> GetCount/SearchCursor -> Dissolvechain through a faketransport whose feature catalog is keyed by the exact resolved source
identity, with deliberately different feature counts/geometry at each
stage (5 line inputs -> 2 buffered polygons -> 1 dissolved polygon). Also
covers a subsequent read against an expired/gone artifact surfacing as a
typed
ExecuteError.test_process_tools.py: new coverage for every rejected/acceptedparameter combination per tool, a dismissed job, an empty-outputs
"successful" job, fail-fast duplicate-output rejection (no job
submitted), and a failed overwrite leaving the prior alias untouched.
test_resolve.py: regression test for theresolve_layer_idNameError.docs/honua-gp/compatibility-matrix.md/packages/honua-gp/docs/compatibility-matrix.mdregenerated from theupdated
_compat.pynotes (kept byte-equal per the existing drift gate).Full existing suite plus the new tests: 226 passed, 4 skipped.
ruff check .clean.
honua-gpeval harness: 50/50 (100%).Scope note on AC #4 / #6: this repo's test suite has no live
honua-server available (the opt-in live-server lanes —
tests/conformance,ephemeral-server-smoke,staging-smoke— needinfrastructure this sandbox doesn't have, and both smoke lanes are
currently failing on every open PR in this repo for unrelated,
pre-existing reasons: an expired
honua-demo-infrastaging binding andthe ephemeral seeded server not reaching
/healthz/ready, neither arequired status check). The chained proof above is the strongest
client-SDK-side evidence available here: it fully exercises projection,
alias-binding, and downstream resolution against a controlled transport
using the same fake-OGC pattern every other test in this package already
uses. A genuine real-server value/geometry proof, and the cross-repo
installed-consumer evidence coordination with #202/#205/#196, are left to
land once that infrastructure is reachable from CI.
Refs #226 (released: real-server execution proof and #202/#205/#196
cross-repo evidence coordination need live honua-server infrastructure not
reachable from this repo's CI today; every other acceptance criterion —
output binding correctness, typed missing-output failure, overwrite/alias
preservation, parameter-semantics validation, and the resolve_layer_id
NameError — is fixed and covered above)