Skip to content

feat: support shared YAML task input - #367

Closed
zengchuang-hw wants to merge 3 commits into
InferMatrixLab:mainfrom
zengchuang-hw:fix/365-task-yaml
Closed

zengchuang-hw wants to merge 3 commits into
InferMatrixLab:mainfrom
zengchuang-hw:fix/365-task-yaml

Conversation

@zengchuang-hw

@zengchuang-hw zengchuang-hw commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Governing Issue and delivery slice

Refs #365

This implements the core YAML task contract for CLI and direct Agent use. DSH consumer migration remains tracked in #152.

Changed artifacts and evidence

Requirement Delivery and verification
Shared v1 task input The core wheel ships the schema and template. CLI and Agent workflows use the same normalization path. The shared fixture covers non-default values, explicit CLI overrides and field-level errors.
Read-only validation and preview Drafts report required-field blockers and effective values. Regression tests assert that validation and preview launch no subprocesses and leave the workspace unchanged.
Existing plan and run paths Complete tasks feed the existing RunSpec. Task-file runs require explicit confirmation that the caller checked the frozen acceptance gates. The CLI checks the baseline Git revision before runtime probes. A synthetic CPU test exercises both operations and checks retained evidence.
Packaging The no-extras wheel check installs outside the checkout and reads the packaged schema and template, then validates and previews the shared fixture. The same task resources are included by the shared plugin bundler.

Checks and dependencies

  • ruff check src tests tools and git diff --check passed on the rebased head.

  • The implementation sweep passed 127 targeted tests and 2 subtests. A clean Windows Python 3.12 wheel install passed both the task-specific and existing package checks. The wheel sources, package metadata inputs and shared fixture are unchanged across the rebase.

  • An independent rebase review found only the expected move of the template inclusion rule into the upstream shared bundler. The rebased DSH test suite passed both tests.

  • The Windows full suite was stopped after Anthropic test errors reproduced on the upstream base. A separate inspection/runtime sweep passed 76 tests and 26 subtests and reproduced one existing Git submodule failure.

  • Scope correction at 24ef10a passed 34 targeted tests, the fresh-install smoke, Ruff and git diff --check. A standard wheel installed without extras in a clean environment passed both the package and YAML task checks. A separate reviewer verified the patch against the PR base and current main.

Scope boundary

The shared YAML task contract uses PyYAML as a core dependency. The scope correction restores the protected knowledge operations guide and skill loader to the PR base, along with the assertions for the existing loader hint. The standard wheel and the existing [skills] install syntax both resolve PyYAML. The protected-path exception request is superseded by the narrower diff.

Deferred knowledge documentation follow-up

The protected knowledge operations guide and missing-PyYAML loader hint still describe [skills] as required. The standard wheel now installs PyYAML for shared YAML tasks, and [skills] is a compatible alias. This PR defers those text corrections to a separate knowledge-policy-admitted change. The follow-up is to update the guide, the loader hint and the hint assertions together, then verify the standard wheel and the legacy install syntax. Review discussion

Remaining work and Issue acceptance

This PR is ready for review. All applicable Ready CI jobs passed at 24ef10a, and the knowledge declaration check passed as not applicable. Goal acceptance remains with the Issue owner.

Use one versioned contract for CLI and Agent task previews and pass complete tasks to the existing RunSpec workflow. Verify ordinary wheel installs and preserve flag-only and bundled plugin behavior.

Agent-assisted-by: Codex / unknown [effort=unknown] (author)
Signed-off-by: zengchuang <zengchuang3@huawei.com>

Copy link
Copy Markdown
Collaborator

Independent COMMENT-only review

Reviewed range: 3a67f2456156f1071d4821e172a5bb55393f3f08..1eba8b2c4ceb72cae853a7691ebad39a983811e8 (PR #367, Draft, governing #365).

This is an ordinary conversation comment only. No approval is implied. Please re-request review when the PR is marked ready-for-review and the items below are addressed.

Blockers

  1. knowledge-model-policy is noncompliant on this head

    • Check run declarations / status context knowledge-model-policy failed on 1eba8b2c4ceb72cae853a7691ebad39a983811e8 with: Material AI author 1: model is unapproved or suspended.
    • The PR body declares model_id: "unknown" / effort: "unknown" while touching protected paths (docs/knowledge-operations.md, src/inferrsi/skill_catalog.py). Per docs/knowledge-model-policy.md, unknown settings stay truthful but do not qualify; an approved model+effort or a maintainer exception approval for the exact protected paths is required before this head can pass the required check.
  2. Draft gate

    • The PR is still Draft. Please mark ready-for-review when the contract slice and admission are intended for formal review; I will re-review that head.

Contract / implementation notes (non-blocking unless they turn into defects)

Relative to the accepted #365 contract, the slice looks largely aligned on a static read of the diff and the green Ready jobs:

  • Shared normalize_task path for CLI validate/preview/plan/run; draft/partial validation; read-only preview (tests assert no inspect_environment / Popen and unchanged workspace).
  • Flag precedence via argument_default=argparse.SUPPRESS + YAML-then-defaults in task_input.normalize_task; acceptance CLI conflicts rejected; complete tasks feed existing RunSpec with --confirm-acceptance for execute.
  • PyYAML moved to core deps; schema/template packaged; check_task_installed.py wired into Ready installed-package; plugin bundler includes data/task_input/*.yaml.
  • DSH migration correctly left to [RFC] Agent plugins: a consistent InferRSI workflow in Codex, DSH and openJiuwen #152.

Ready CI on this head: full-check (3.10/3.11) and installed-package (3.10/3.11) succeeded; pr-references succeeded; only declarations / knowledge-model-policy failed.

Questions / follow-ups (please clarify or fix)

  1. src/inferrsi/skill_catalog.py still tells users to install the 'skills' extra on missing PyYAML, while docs/quickstart now say the base install includes skill reads. Is the leftover extra hint intentional for plugin/direct-source runtimes only, or should the message match the new core dependency?
  2. Acceptance float equality in task_input.normalize_task (override != value) — is exact float match the intended conflict rule for --min-improvement-pct, or should there be an explicit tolerance / decimal normalization?
  3. Please confirm whether you will (a) re-declare an approved model_id+effort, or (b) request a knowledge-policy-approval exception for the protected doc/loader touch, before leaving Draft.

Happy to re-review a ready head once policy admission is green and any of the above are settled.

Direct-source users now see the standard package install command when the core YAML dependency is missing. Keep the checkout and lightweight-controller error paths covered.

Agent-assisted-by: Codex / unknown [effort=unknown] (author)
Signed-off-by: zengchuang <zengchuang3@huawei.com>

Copy link
Copy Markdown
Contributor Author

Thanks for the review.

  1. e76189e updates the checkout error to recommend the standard InferRSI install when PyYAML is missing. I verified 34 task and knowledge tests, six bundled-plugin regression cases, Ruff and the staged diff. A separate reviewer checked the three-file patch and reran the 34 tests.
  2. Exact equality of the parsed finite float values is intentional. For the YAML threshold 7.5, 7.500e0 is accepted and 7.500000000000001 is rejected with an acceptance.min_improvement_pct conflict. The CLI comparison uses the same numeric representation as RunSpec and applies no tolerance. #365 contract
  3. The PR body requests a knowledge policy exception for this head and the two protected paths, docs/knowledge-operations.md and src/inferrsi/skill_catalog.py. The declaration keeps the actual model IDs and effort as unknown. The PR remains Draft while Dong1017 decides the exception and the checks run. Policy check · Ready CI

@Dong1017 Dong1017 added the area:usability Usability and collaboration: issue navigation, templates, readable reports label Nov 6, 2026 — with ChatGPT Codex Connector

Copy link
Copy Markdown
Collaborator

Independent COMMENT-only re-review

Reviewed range: 1eba8b2c4ceb72cae853a7691ebad39a983811e8..e76189ed9889870d94b520dfba267a2760c583a2 (PR #367, still Draft, governing #365).

This is an ordinary conversation comment only. No approval is implied. Please re-request review when the PR is marked ready-for-review and the remaining blockers below are cleared.

Incremental (this head)

Three-file follow-up commit e76189ed9889870d94b520dfba267a2760c583a2:

  • src/inferrsi/skill_catalog.py now recommends python -m pip install '<inferrsi-checkout>' (standard package) when PyYAML is missing, matching the core-deps move.
  • Tests in tests/test_knowledge_install.py and tests/test_skill_eval_optional_dependencies.py assert the new hint.

That closes the prior open question on the leftover 'skills' extra install message.

Prior open questions — disposition

  1. PyYAML checkout hint — addressed on this head (see above).
  2. Acceptance float equality — accept author clarification: exact equality of the parsed finite floats is intentional and matches RunSpec numeric representation (no tolerance).
  3. Policy admission — still open (see blockers).

Remaining blockers (still prevent Approve)

  1. knowledge-model-policy / declarations still failing on this head

    • Check run declarations (workflow CI — knowledge declaration policy, runs 42236838872 / 42236793902) concluded failure on e76189ed9889870d94b520dfba267a2760c583a2 at step Reconcile declarations with trusted main code.
    • PR body still declares model_id: "unknown" / effort: "unknown" for material AI authors while touching protected paths docs/knowledge-operations.md and src/inferrsi/skill_catalog.py, and requests a knowledge-policy exception. No maintainer exception approval is visible yet on this PR (no approval label / recorded exception for this exact head+paths). Until an approved model+effort is declared or a maintainer exception for those paths on this head lands and the required check goes green, this head cannot be Approved.
  2. Draft gate

    • The PR is still Draft. Please mark ready-for-review when the contract slice and admission are intended for formal review; I will re-review that head.

Ready CI (unchanged positive signal)

On e76189ed: full-check (3.10/3.11), installed-package (3.10/3.11), and pr-references succeeded. Only declarations failed.

Happy to re-review a ready head once policy admission is green.

Restore the knowledge operations guide, the existing skill loader hint, and its assertions. The shared YAML task contract still uses PyYAML as a core dependency and validates the standard wheel without extras.

Agent-assisted-by: Codex (scope correction and validation)
Signed-off-by: zengchuang <zengchuang3@huawei.com>

Copy link
Copy Markdown
Contributor Author

Scope update after the re-review.

24ef10a restores the knowledge operations guide, the skill loader and its prior hint assertions to the PR base. The shared YAML task contract keeps PyYAML in the standard install. The existing [skills] install syntax remains compatible for users following the knowledge guide.

The narrower diff has zero protected knowledge paths. The latest knowledge declaration check reports Not applicable and passed, so the earlier exception request is superseded.

I verified 34 targeted tests, the fresh source-install smoke, Ruff and git diff --check. Both package smoke scripts also passed on a standard wheel installed without extras. A separate reviewer confirmed the four restorations and the narrower scope. Ready CI for this commit

Copy link
Copy Markdown
Collaborator

Independent COMMENT-only re-review

Reviewed range: e76189ed9889870d94b520dfba267a2760c583a2..24ef10a5062ccd6d005f43658216fa01e0fc9599 (PR #367, still Draft, governing #365). Ordinary comment only, no approval implied.

Prior blockers — disposition

  1. knowledge-model-policy / declarations: resolved. The diff no longer touches docs/knowledge-operations.md or src/inferrsi/skill_catalog.py relative to base 3a67f245, and both declarations check runs on 24ef10a5 concluded success. The earlier exception request is superseded.
  2. Draft gate: still open. The PR is still Draft; please mark it ready-for-review when you want the formal review.

New issue introduced by the restoration (please fix or explicitly defer)

Restoring the two protected files to base reintroduces wording that contradicts this PR's own dependency change. At this head pyproject.toml declares dependencies = ["PyYAML>=6.0"] and skills = [] (alias only), so:

  • src/inferrsi/skill_catalog.py L10: the comment The base install stays PyYAML-free by contract. is no longer true; L12–L14 tell users the 'skills' extra provides PyYAML, while the extra is now empty.
  • docs/knowledge-operations.md L35–L36 and L228–L229: say --skill-root / resource-path resolution "require the skills extra", but after this PR the base install already provides PyYAML.

The [skills] install syntax still works, so this is a documentation/message inconsistency rather than a runtime defect. Expected: either (a) note in the PR body / #365 that these protected-path texts are knowingly stale and track a follow-up PR (with the policy admission it needs) to align them, or (b) handle them in this PR with the required admission. Please say which.

CI on 24ef10a5

declarations, installed-package (3.10/3.11) and pr-references succeeded; full-check (3.10/3.11) was still in progress when I looked, so it is not verified by me.

I'll re-review the ready-for-review head.

Copy link
Copy Markdown
Contributor Author

Thanks for the re-review. I recorded the deferral under Deferred knowledge documentation follow-up. The old guide and loader hint still say [skills] is required. Standard installation now provides PyYAML for YAML task input, and the existing [skills] syntax still works. The separate follow-up covers both protected texts and the associated hint assertions under knowledge policy admission.

Both full-check jobs have since passed, along with the two installed-package jobs and PR references, on 24ef10a5062ccd6d005f43658216fa01e0fc9599. Ready CI · Knowledge declaration check

@zengchuang-hw
zengchuang-hw marked this pull request as ready for review November 9, 2026 07:19
@zengchuang-hw

Copy link
Copy Markdown
Contributor Author

@cursoragent @amy-why-3459 @Dong1017 PTAL

@cursor

cursor Bot commented Nov 9, 2026

Copy link
Copy Markdown

You need to increase your spend limit or enable usage-based billing to run background agents. Go to Cursor

@cursor

cursor Bot commented Nov 9, 2026

Copy link
Copy Markdown

On 24ef10a5062ccd6d005f43658216fa01e0fc9599, the YAML task slice matches the accepted #365 contract and the earlier policy blocker is closed. Formal Approve still waits on ready-for-review. This comment does not Approve.

Still open

Settled on this head

  • knowledge-model-policy / declarations: relative to base 3a67f245, the diff no longer touches docs/knowledge-operations.md or src/inferrsi/skill_catalog.py. Check run 42575749429 on 24ef10a passed as Not applicable. The earlier exception request is superseded.
  • Stale [skills] wording: recorded under Deferred knowledge documentation follow-up. The guide and loader hint still describe [skills] as required; the standard wheel now installs PyYAML, and [skills] stays a compatible alias. The follow-up updates the guide, the loader hint, and the hint assertions together under knowledge-policy admission.
  • Acceptance float equality stays an exact match of the parsed finite floats (same numeric representation as RunSpec), per the task: support shared YAML task input for the InferRSI CLI and agents #365 contract note.
  • Author reports Ready CI green on this head: full-check (3.10/3.11), installed-package (3.10/3.11), and pr-references (run 42575692993).

Verification limit: this read is the PR body and the thread through the PTAL. The diff and live CI were not re-checked in this reply.

Copy link
Copy Markdown
Contributor Author

@amy-why-3459 Thanks for the re-review. This PR is now ready for formal review at 24ef10a5062ccd6d005f43658216fa01e0fc9599. The [skills] wording follow-up is recorded in the PR description. All five Ready CI jobs and the knowledge policy check passed.

Could you please submit your formal review on this revision?

@zengchuang-hw

Copy link
Copy Markdown
Contributor Author

@Dong1017 @amy-why-3459 @cursoragent PTAL again

@cursor

cursor Bot commented Nov 10, 2026

Copy link
Copy Markdown

Head 24ef10a5062ccd6d005f43658216fa01e0fc9599 has no open blockers left on this thread. A formal Approve is an LGTM review of that head by an eligible reviewer. This reply is the thread assessment (PR body + comments).

Closed since the last independent review:

  • knowledge-model-policy. 24ef10a restores docs/knowledge-operations.md and src/inferrsi/skill_catalog.py to the PR base, so the diff no longer touches those protected paths. Declarations on that head passed as not applicable (42575749429). The author also reports 42617769597 passed. The earlier exception request is superseded.
  • Draft gate. The author states this PR is ready for formal review at 24ef10a (comment). The PR body says the same.
  • Stale [skills] wording. Recorded under Deferred knowledge documentation follow-up, the deferral requested in the re-review. The guide and loader hint still describe [skills] as required. The standard wheel installs PyYAML, and [skills] stays a compatible alias. The follow-up updates the guide, the loader hint, and the hint assertions together under knowledge-policy admission.
  • Acceptance float equality. Exact match of the parsed finite floats (same numeric representation as RunSpec), per the task: support shared YAML task input for the InferRSI CLI and agents #365 contract note.

Author-reported checks on this head: all five Ready CI jobs in 42578631181, plus the knowledge declaration check above.

Not verified in this pass: a live refetch of those runs, and a re-read of the diff (this seed is the PR body and thread).

Outside this PR: the deferred knowledge-doc alignment, DSH consumer migration (#152), and #365 goal acceptance with the Issue owner.

@zengchuang-hw

Copy link
Copy Markdown
Contributor Author

closed as newer design in #338

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:usability Usability and collaboration: issue navigation, templates, readable reports kind:task Bounded delivery task with a delegated slice

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants