fix(hitl): align HITL skill and tests to v1.0 node schema#529
fix(hitl): align HITL skill and tests to v1.0 node schema#529dushyant-uipath merged 1 commit intomainfrom
Conversation
|
Claude finished @dushyant-uipath's task in 2m 58s —— View job SummaryAligns the HITL skill docs and eval tests to the v1.0 node schema: renames Change-by-Change Review1.
|
| Area | Status | Notes |
|---|---|---|
| Frontmatter | OK | No changes to frontmatter, existing frontmatter valid |
| E2E Tests | OK | 4 new quality tests well-structured; existing tests correctly updated |
| Skill Body | OK | Critical Rule 9 is well-placed and accurate |
| References & Assets | Issue | coded-action-app.md:323 still references removed handles |
| Repo Hygiene | OK | CODEOWNERS present, no secrets, changes scoped to HITL skill |
Issues for Manual Review
- v1.0 schema accuracy: The
outputDefinitionin the definition entry was changed to"source": "=result.Action"for status. Verify this matches the actual HITL runtime's output contract — an incorrectsourceexpression would cause runtime variable resolution failures. typeVersionon node vsversionon definition: The node JSON example uses"typeVersion": "1.0.0"(line 82 of quickform.md) while the definition uses"version": "1.0". Confirm these are intentionally different fields with different version granularity.
Conclusion
Solid, well-scoped PR that correctly aligns the HITL skill to v1.0. The new quality tests are thoughtfully designed and cover real agent failure patterns. Two issues to fix before merge:
- High: Update
hitl-node-coded-action-app.md:323to removecancelled/timeouthandle references — this is the last inconsistent spot in the v1.0 migration - Medium: Add the missing
submittedDatedate-type assertion toquality_09to match the test's claimed coverage
- Read review criteria and project conventions
- Analyze full diff (
origin/main...HEAD) - Read existing PR conversation
- Review each changed file in full context
- Check skill-specific requirements (links, cross-refs, secrets, CODEOWNERS)
- Post review findings
3934779 to
c957ed7
Compare
- Update SKILL.md, hitl-node-quickform.md, hitl-node-apptask.md, hitl-node-coded-action-app.md to use output (not result) outputId, status source =result.Action, shape square, version 1.0, and document fieldId-vs-variable access rule - Remove stale cancelled/timeout handle references (v1.0 has completed only) - Update e2e_01, e2e_03, quality_04, quality_07 tests for .output path and remove timeout handle wiring requirement from e2e_03 - Add quality_08: fieldId vs variable access (ContractReview); negative check uses run_command + grep -L instead of unsupported excludes field - Add quality_09: correct field types — boolean/number/date vs text default (ExpenseReview) - Add quality_10: binding direction — inputs must have binding, outputs must not (SupplierOnboarding) - Add quality_11: inOut pre-fill + downstream output access (RCAReview) - Rewrite TEST_PLAN.md with v1.0 invariants, variable binding accuracy section, and real-world consequence coverage map
c957ed7 to
e95cdcf
Compare
|
Claude finished @dushyant-uipath's task in 2m 34s —— View job Review: PR #529 — fix(hitl): align HITL skill and tests to v1.0 node schema
SummaryAligns the HITL skill docs and eval tests to the v1.0 node schema: renames Change-by-Change Review1.
|
| Area | Status | Notes |
|---|---|---|
| Frontmatter | OK | No changes to frontmatter, existing frontmatter valid |
| E2E Tests | OK | 4 new quality tests well-structured; existing tests correctly updated |
| Skill Body | OK | Critical Rule 9 well-placed and accurate |
| References & Assets | OK | All three reference files consistently updated for v1.0 |
| Repo Hygiene | OK | CODEOWNERS present, no secrets, changes scoped to HITL skill |
Issues for Manual Review
- v1.0 schema accuracy: The
outputDefinitionin the definition entry uses"source": "=result"for output and"source": "=result.Action"for status. Verify these match the actual HITL runtime's output contract — an incorrectsourceexpression would cause runtime variable resolution failures. typeVersionvsversiongranularity: The node JSON example uses"typeVersion": "1.0.0"(line 82 of quickform.md) while the definition uses"version": "1.0". Confirm these are intentionally different fields with different version granularity —typeVersionon the node instance vsversionon the definition entry.
Conclusion
Clean, well-scoped PR. All result → output renames are consistent across skill docs and tests. The cancelled/timeout handle cleanup is complete — no stale references remain. Critical Rule 9 (fieldId vs variable) is a valuable addition that addresses a real agent failure pattern. The 4 new quality tests are thoughtfully designed with both positive and negative assertions. The two issues flagged by the prior automated review are already fixed in this commit. No issues found — this is ready to merge.
Summary
outputnotresult: Updates all skill docs and eval tests to use$vars.<nodeId>.output(v1.0 renamed the outputId fromresulttooutput)cancelled/timeouthandles removed: Cleaned up all references to non-existent handles; v1.0 exposes onlycompletedfieldIdvsvariablerule documented: Added Critical Rule 9 to SKILL.md and a callout in hitl-node-quickform.md — output object keys are the fieldid, notfield.variablequality_08: agent uses field ID (not variable name) for output accessquality_09: agent infers correct types (boolean/number/date) instead of defaulting to textquality_10: binding on input fields only — output fields must have no bindingquality_11: inOut fields are pre-filled via binding AND appear in$vars.<nodeId>.outputafter submitFiles changed
All files are under
skills/uipath-human-in-the-loop/andtests/tasks/uipath-human-in-the-loop/(Dushyant-owned).Test plan
quality_08— ContractReview: field ID vs variable name accessquality_09— ExpenseReview: boolean/number/date type inferencequality_10— SupplierOnboarding: binding direction correctnessquality_11— RCAReview: inOut pre-fill + downstream output accessquality_04,quality_07,e2e_01,e2e_03pass with updated.outputpaths