Skip to content

fix(matrix): isolate generators and preserve old refs / 隔离生成器并兼容旧修订 - #3497

Merged
functionstackx merged 2 commits into
mainfrom
fix/pr3490-generator-isolation
Sep 26, 2026
Merged

functionstackx merged 2 commits into
mainfrom
fix/pr3490-generator-isolation

Conversation

@functionstackx

@functionstackx functionstackx commented Sep 26, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Follow-up to #3490, which merged while the requested ten-model review was running.

  • Pin historical and OperatorX generator subprocesses to the selected checkout through explicit PYTHONPATH. With inherited PYTHONSAFEPATH=1, relying on cwd could silently run another installed generator and compute the wrong append-only delta.
  • Restore pre-package revision support in manual E2E, profiling, and OperatorX generation. Prefer the selected checkout's module; otherwise run its legacy script with its sibling imports available. A missing generator fails explicitly.
  • Exercise the actual historical subprocess wrapper, not only snapshot extraction. Cover modern and legacy layouts, conflicting inherited imports, safe-path mode, and relative checkout paths.
  • Correct the bilingual model-prefix example and architecture description. No recipe, benchmark flag, image, or performance changelog changes.

Validation

  • Regression tests failed before the fixes: six cases reproduced wrong-checkout imports or missing legacy entrypoints.
  • Focused matrix and OperatorX generator tests: 436 passed.
  • Ruff check and format check, git diff --check, and Zizmor on both changed workflows passed.
  • Executed each workflow's actual generator-selection shell block against modern, real pre-migration, and missing-generator checkouts. Successful output matched the corresponding direct generator's JSON; missing generators failed. This local check substituted the test interpreter for uv, so hosted Actions remains separate validation.
  • Full CI command with RUNNER_NAME=local-review_00, matching the variable supplied by GitHub Actions: 2,106 passed, 7 skipped. Without that CI-provided variable, the unrelated H100 launcher test fails both before and after these changes; no launcher code was changed.
  • Hosted Tests and Lint, Zizmor, and OperatorX coverage planning passed on 24f2f4b521eee91c2b74a046edfceeb2128efbfc. GPU OperatorX jobs were skipped by the planner; no GPU benchmark or eval sweep is claimed.

The user requested fixes followed by an admin merge. This follow-up carries the remaining #3490 fixes because that PR was merged independently before this review completed. No performance-affecting recipe or config change requires a benchmark sweep or artifact reuse here; no human CODEOWNER approval is claimed.

All ten requested model reviews completed. Follow-up changes were also independently reviewed by GPT 6 Sol and GPT 5.6 Sol. Codex was not available in the execution environment; no Codex review is claimed.

AI model disclosure

Model Role
GPT 6 Astra Fast Orchestration, implementation, regression tests, verification, PR preparation. Runtime exposes this label; no finer identifier was available.
Claude Opus 5.5 Historical planning review
Claude Sonnet 5.0 Workflow review
Claude Fable 5 Subprocess and import-isolation review
GPT 6 Sol Test coverage review and independent workflow validation
GPT 5.6 Sol OperatorX review and follow-up verification
GPT 5.6 Terra Caller and removed-entrypoint audit
GPT 5.6 Luna Documentation review
Gemini 3.1 Pro Packaging review
Gemini 3.7 Flash Test-policy and discovery review
Grok 4.7 Append-only planning review
中文

概述

这是 #3490 的后续修复。该 PR 在用户要求的十模型审阅进行期间已被合并。

  • 通过明确设置 PYTHONPATH,让历史规划和 OperatorX 的生成器子进程使用所选检出版本。继承 PYTHONSAFEPATH=1 时,仅依赖 cwd 可能误用另一个已安装版本的生成器,导致 append-only 差集计算错误且不报错。
  • 恢复手动 E2E、性能分析和 OperatorX 对模块迁移前修订的支持。优先使用所选检出目录中的模块,否则运行其遗留脚本,并保留同目录模块的导入能力。缺少生成器时明确报错。
  • 测试直接调用历史生成器的实际子进程封装,而不只测试快照提取。覆盖新旧布局、继承的冲突导入路径、安全路径模式和相对检出路径。
  • 同步修正中英文模型前缀示例和架构说明。不修改配方、基准测试参数、镜像或性能变更日志。

验证

  • 修复前,六个回归用例复现了错误版本导入或遗留入口缺失。
  • 矩阵和 OperatorX 生成器专项测试:436 项通过。
  • Ruff 检查、格式检查、git diff --check 及两个工作流的 Zizmor 检查均通过。
  • 在模块布局、真实迁移前布局及缺少生成器的检出目录中,执行了工作流实际的生成器选择 Shell 代码。成功输出与对应生成器直接执行的 JSON 一致;缺少生成器时按预期失败。本地检查使用测试解释器替代 uv,托管 Actions 仍需单独验证。
  • 完整 CI 命令显式设置 RUNNER_NAME=local-review_00,对应 GitHub Actions 提供的环境变量:2,106 项通过,7 项跳过。未提供该变量时,无关的 H100 启动器测试在修改前后均失败;本次未修改启动器代码。
  • 24f2f4b521eee91c2b74a046edfceeb2128efbfc 上的托管 Tests 和 Lint、Zizmor 及 OperatorX 覆盖规划 均通过。规划器跳过了 GPU OperatorX 作业;未声称运行 GPU 基准测试或评测扫描。

用户要求修复后以管理员权限合并。由于 #3490 在本次审阅结束前已由其他会话合并,本 PR 承载其剩余修复。本次没有影响性能的配方或配置变更,不涉及基准扫描或产物复用;未声称获得人工 CODEOWNER 批准。

十个模型的审阅均已完成。GPT 6 Sol 和 GPT 5.6 Sol 还独立检查了后续修复。执行环境中没有 Codex,因此未声称完成 Codex 审阅。

AI 模型披露

GPT 6 Astra Fast 负责协调、实现、回归测试、验证和 PR 编写;运行时仅提供该标签,无法核实更细的标识。Claude Opus 5.5 审阅历史规划;Claude Sonnet 5.0 审阅工作流;Claude Fable 5 审阅子进程和导入隔离;GPT 6 Sol 审阅测试覆盖并独立验证工作流;GPT 5.6 Sol 审阅 OperatorX 并复核修复;GPT 5.6 Terra 检查调用方和已删除入口;GPT 5.6 Luna 审阅文档;Gemini 3.1 Pro 审阅打包行为;Gemini 3.7 Flash 审阅测试规范和测试发现;Grok 4.7 审阅 append-only 规划。

修复矩阵生成器导入隔离,保留旧修订的工作流和 OperatorX 入口,并补充行为回归测试与双语文档。
同步 main,保留生成器导入隔离与新加入的历史配方根目录设置,并合并双语文档。
@functionstackx
functionstackx merged commit c56182a into main Sep 26, 2026
7 checks passed
@functionstackx
functionstackx deleted the fix/pr3490-generator-isolation branch September 26, 2026 21:11

@claude claude Bot 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.

I reviewed this PR and didn't find any bugs. It's a moderate-sized fix touching subprocess environment isolation (PYTHONPATH construction and ordering) in infx/matrix/plan.py and experimental/operatorx/scripts/inferencex_testlist/matrix.py, plus two CI workflow files with new generator-selection branching, so a human look — including from the OperatorX code owner — would still be worthwhile.

What was reviewed: the module-vs-legacy-script detection and explicit PYTHONPATH construction in plan.py::generate_matrix and matrix.py::_run_inferencex_generator; the new/rewritten tests in test_inferencex_generator.py and test_process_changelog.py, confirming they exercise the real subprocess wrapper with a decoy foreign checkout on PYTHONPATH and PYTHONSAFEPATH toggled, asserting on real parsed output rather than pinning config or reimplementing logic; the two workflow YAML diffs, confirming the new branching fails loudly (exit 1) with no silent defaults; and the bilingual docs updates, confirming MODELS.md/_zh and architecture.md/_zh stay in sync with each other and with the code.

Extended reasoning...

The diff spans two CI workflows, two library modules (infx/matrix/plan.py and experimental/operatorx's matrix.py), docs, and tests, implementing subprocess PYTHONPATH isolation to fix a real cross-checkout import bug; no auth/crypto/permission surface is touched. Code review of the diff found the import-path ordering, legacy-vs-module detection, and workflow fallback logic all correct and consistent with the stated intent, and the new/rewritten tests genuinely exercise the real subprocess paths with concrete decoy-checkout and PYTHONSAFEPATH scenarios. Decisive factors for defer rather than approve: CODEOWNERS assigns experimental/operatorx/ to a specific named owner (@ hbarclay) and docs/MODELS.md to specific owners, and the change is non-trivial enough (subprocess env isolation across three call sites) that owner sign-off is appropriate rather than an automated approval.

This review covers commit 24f2f4b, which is no longer the latest commit on this pull request; later commits are not covered by it.

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

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant