Skip to content

Package shared technical contracts and retire root modules - #453

Merged
Dong1017 merged 2 commits into
mainfrom
impl/c1-contracts-t370
Nov 15, 2026
Merged

Dong1017 merged 2 commits into
mainfrom
impl/c1-contracts-t370

Conversation

@Dong1017

Copy link
Copy Markdown
Collaborator

Governing Issue and delivery slice

Refs #451

  • Accepted scope / delegation: C1 of the accepted four-Issue/eight-PR plan, using the refreshed Issue requirements. One implementation session; the next slice remained read-only planning.
  • What this PR delivers: Package the shared technical contracts under inferrsi.contracts (json_v1, runspec, claim); retire the four corresponding root files in this PR without forwarding shells; migrate active consumers; retain exactly one RunSpec initialization path and same-object facade/consumer identities; enforce the exact 23-file root inventory and bounded technical-contract import rules.
  • What it does not deliver: C2 resources/evidence migration, MindIE relocation/version-path or worker-isolation work, Harness relocation, permanent R1 import rules, final R3 zero-exception gate, or the other seven PRs. This slice does not claim the final target tree already exists.

Knowledge authorship declaration

{"schema_version":1,"head_sha":"7b4f142ee5f224f25731c7bfc3a5dff762df1641","human_only":false,"material_ai_authors":[{"tool":"Claude Code","model_id":"claude-fable-5-2","effort":"max","role":"author"},{"tool":"Grok Bot","model_id":"unknown","effort":"unknown","role":"author"}]}

The Claude model value is the requested/reported client alias, not independent backend attestation. The coordinator directly made mechanical updates to AGENTS.md, CHANGELOG.md, docs/contracts/current-design.md, and docs/module-layout.md; these are not attributed to Claude. Both narrow same-session documentation continuations ended with API request timeouts before editing. Across all 52 changed paths, the existing protected-path and policy-path matchers each matched zero paths; expected knowledge-policy result is Not applicable, not an exemption or admission of unknown model settings.

Changed artifacts and evidence

Requirement Actual change Independent local evidence Remaining gap
Preserve canonical bytes and validation behavior Technical implementation moved into canonical package leaves 83 baseline/candidate canonical-byte, error, RunSpec and Verdict vectors; implementation AST equivalence apart from module docstrings CI/reviewer confirmation
Unique initialization and shared identities RunSpec package export plus migrated consumers Cold process: exactly one RunSpec dataclass initialization; facade/arguments/Harness runner identities and Verdict/parse_verdict identities pass CI/reviewer confirmation
Cold contracts without business IO/imports Bounded technical package imports Business-filesystem-IO and core-host-import guards, including a positive negative-control check, pass in source and installed-wheel acceptance No claim that later Harness boundary work is complete
Retire old files without shells Four old root files removed, canonical package added Retired _json_v1, runspec, claim imports/pickle paths fail as intended; inferrsi.contracts remains the canonical package, not a retired import name; exact root guard and mutation negatives pass Old pickle import paths are intentionally incompatible; see changelog
Package actual distributable Real SCM-versioned sdist, fresh wheel, install outside checkout Installed checker: 128 source-grounded records and 923 resource files byte-identical; version agrees across module, metadata and wheel; installed identity/noIO/root/retired-path checks pass CI packaging confirmation

Checks and dependencies

  • Integration identity: checkpoint f05118ea1f74005ef869f0b41c64d1f26af2f102, then an ordinary clean merge of main/Connect report Dashboard and trace to fixed attempt records #450, producing head 7b4f142ee5f224f25731c7bfc3a5dff762df1641. Latest pre-push main read was a92c77ccaed68426c83154cff7619745f34d0058; no rebasing, history rewriting or force push. Main's eight-file Connect report Dashboard and trace to fixed attempt records #450 change and all C1 work were retained. Preserved candidate-file hashes were compared across integration, with three genuine Connect report Dashboard and trace to fixed attempt records #450 overlap files checked separately.
  • Checks actually run: seven-file focused core pytest; 83-vector and cold identity/noIO/initialization/root acceptance; sequential broader regression with the two subprocess environment cases rechecked; fresh real wheel/install acceptance on the merged head; 24 post-merge dashboard/root/boundary cases passed. Final affected tests/test_apps_assess.py alone: 28 passed on the exact merged head. Already-passed suites were not rerun in the final fixture-only segment.
  • Wheel: inferrsi-0.1.11.dev18+g7b4f142ee-py3-none-any.whl, SHA256 c759cadafa1b6bb9b2d97a3b172bccfc074170372a7d86d746afcd00f63c8bca; 1102 entries, no mandatory runtime dependencies.
  • Environment failures and recovery: the initial sdist succeeded, but extraction exceeded Windows path lengths before a wheel existed. The same verified sdist was extracted to a fresh short path and the real wheel built and installed successfully, without changing global settings. Two subprocess import failures reproduced on unchanged baseline when source was not exported; both passed with exact per-process source PYTHONPATH. Post-merge Apps tests initially had 25 fixture-setup errors because inherited sparse-checkout omitted tracked frozen fixtures; only missing exact HEAD fixtures were materialized and every tracked fixture compared byte-for-byte before the 28-test pass. No frozen data/source edits, test deletions, global sparse changes or other worktree changes were used to obtain that pass.
  • Known failed/skipped checks: changed-Python Ruff reports four existing F811 request_digest shadow diagnostics in harness/run_store.py; exact diagnostic signatures reproduce on current main. No suppression or public-keyword rename was introduced. Four broader-regression Windows skips remain (symlink privilege/POSIX process semantics). These are not presented as passing checks. Remote CI had not yet been observed when this body was created; no all-green claim.
  • Protected work: old task: A-entry — canonical Apps entrypoints and application composition #408 worktree and its four dirty paths were left untouched and its binary diff fingerprint rechecked. No C2 implementation was started.

Remaining work and Issue closure

  • Remaining Issue requirements / follow-up: C2 is still a separate PR; other Issue slices and final acceptance remain. MindIE version-path verification belongs to M1; the permanent neutral-leaf import rule and elimination of temporary exceptions belong to R1/R3, not this C1 expansion.
  • Goal-acceptance owner or accepted closure delegation: human owner/reviewers; no delegated automatic merge or Issue closure.
  • Goal ready for acceptance? Not claimed. This is the C1 review delivery, not completion of task: consolidate shared contracts and packaged-resource access #451 or all four Issues.

Handoff

C2's retained implementation plan is explicitly coordinator-verified planning, not a final Opus output or approval: the original read-only planner ended without a final response. It preserves resource identity, splits pure evidence paths from app-side directory creation, retires the old two roots in its own slice, and adds only the exact contracts.evidence boundary exception rather than a broad package allowance. No next implementation is running.

Agent-assisted-by: Claude Code (author)
Agent-assisted-by: Grok Bot (author)
Signed-off-by: GUOGUO <xwdong1998@163.com>
Agent-assisted-by: Grok Bot (author)
Signed-off-by: GUOGUO <xwdong1998@163.com>
@Dong1017 Dong1017 added the kind:task Bounded delivery task with a delegated slice label Nov 15, 2026

@amy-why-3459 amy-why-3459 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

PR review verdict — PASS
Reviewed base a92c77c and head 7b4f142.
Verified #451 C1 delivery: json_v1/runspec/claim moved into inferrsi.contracts at 100% similarity; contracts/__init__.py re-exports only RunSpec; the four root modules are deleted without forwarding shells; consumers, isolation gates, changelog and layout docs were updated; the exact 23-file root inventory guard is in place and shrinks in later removal PRs. Leaves import only the stdlib, and the cold-import / no-I/O / identity / retirement tests match the accepted acceptance criteria. Remote CI is green (lint-and-smoke, declarations, pr-references, installed-package 3.10/3.11, full-check 3.10/3.11, dashboard-browser). Knowledge-model-policy: Not applicable.

Copy link
Copy Markdown
Collaborator Author

@amy-why-3459 @zengchuang-hw Please review C1, tracked by Issue451. This PR packages shared contracts, retires four root modules, migrates consumers, and adds root-inventory and RunSpec identity/no-IO checks. It includes the latest main changes; validation results and baseline limitations are documented in the PR body.

@Dong1017
Dong1017 enabled auto-merge (squash) November 15, 2026 13:26

@zengchuang-hw zengchuang-hw left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

PR review verdict — PASS (C1 only)

Initial independent review requested in #issuecomment-6894982574.
Reviewed base a92c77c → head 7b4f142. Previous review: none from this account for this PR; resolved prior findings: not applicable.

I read the actual diff, #451 C1 requirements, AGENTS and the repository review guidance, without using other reviewers' opinions. The three moved implementation files are byte-for-byte identical to their base versions. Active consumers use the new canonical modules; the initializer has only the RunSpec re-export. The exact 23-file root guard, bounded import-closure changes and negative mutation tests fit this slice, and the changelog explicitly discloses retired import/pickle paths. I found no blocking defect in C1.

Independently executed in an isolated Linux/Python 3.12 checkout:

  • Eight focused suites (contracts package, root inventory, first-wave isolation, target-module import gate, claim, Apps assess, Harness run store and execution): 208 passed, 1 skipped (Windows Job Object), 86 subtests passed.
  • Changed existing/new Python files: Ruff passed; git diff --check passed.
  • Built real SCM sdist and wheel, installed wheel into a fresh venv with the skills extra, ran check_installed.py outside checkout with Python -I and no PYTHONPATH: 128 source-grounded records and 923 resource files verified against checkout, including retirement/identity/root checks.
  • Verified implementation byte equality and absence of active source imports of the retired modules.
  • Observed eight successful hosted checks on this exact head, including ready full-check and installed-package on Python 3.10/3.11 and dashboard-browser. These supplement, not replace, the source review.

The first local test invocation used a relative PYTHONPATH and failed one subprocess import after changing its working directory; the complete eight-suite rerun with an absolute source path passed as reported. The first wheel checker lacked the documented optional PyYAML dependency; installing the skills dependency and rerunning the isolated checker passed.

Not independently executed locally: whole-repository pytest, Windows tests, browser suite, hardware/performance experiments. No hardware or performance claim is made. Local SCM version reflects the tags present in this clone and is not a reproduction of the author's wheel hash.

Before approval: no remaining C1 blocker or required evidence gap found. Optional suggestions: none. C2 and the other architecture slices remain separate; this approval neither closes #451 nor establishes final target-tree acceptance. No merge or auto-merge performed.

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

Labels

kind:task Bounded delivery task with a delegated slice

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants