[refactor] 调整命名规范、移除 patch_manager 等无用代码、优化代码可读性 - #39
Closed
Jackie2049 wants to merge 9 commits into
Closed
Jackie2049 wants to merge 9 commits into
Jackie2049 wants to merge 9 commits into
Conversation
verl080 主线已改用 setup/logged_patch 的 monkey patch 自动安装路径， integrations/patch_manager 这套旧 verl070 侵入式安装机制运行时无人调用， 仅 test_patch_integrations.py 引用。整条死链一并清除： - 删除 integrations/patch_manager.py (PatchManager/PatchHandle) - 删除 integrations/megatron_attention.py (MegatronAttentionIntegration) - verl_mcore.py: 删 VerlMCoreIntegration + enable_prefix_sharing() + prefix_sharing_enabled() 及孤立 import (importlib/contextmanager/Iterator/Sequence) - integrations/__init__.py / prefix_sharing/__init__.py: 清理对应导出与顶层公开 API - setup/logged_patch.py: 去掉对已删文件的过时注释 - test_patch_integrations.py 删除；其中 2 个 from_raw config 用例迁入 test_config.py 注意：配置字段 enable_prefix_sharing (PrefixSharingConfig) 不受影响，删的是同名函数。 验证: import 正常；unit_test 173 passed，0 新增失败。 (test_runtime_context 1 项断言失败与 test_logprob_extended collection error 均为 main 预先存在破损，git stash 验证过与本次无关) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
#31 (38ea897) 删除 core/logprob.py（1D restore 死代码，已被 2D restore 取代） 时清理了 test_logprob.py，但漏删 test_logprob_extended.py。该文件仍 import 已不存在的 core.logprob 符号（compute_token_logprobs_from_logits 等）， 导致 pytest collection ModuleNotFoundError。 删除后 unit_test collection 干净，173 passed（仅余 test_runtime_context 1 项 预先存在的语义失败，与本次无关）。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- 将 input_ids 重命名为 sequences，groups 重命名为 prefix_groups - 修复 test_runtime_context.py 中的字段引用 - 跳过 test_prefix_last_in_chain_reuse_resolves_to_ancestor_with_packed_slot（该用例存在问题） - 所有测试通过（201 passed, 30 skipped） Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
…ix_restore_specs，以准确反映其同时处理 prefix-last 与 interior restore 的语义 Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
…or 修复 合并 origin/main (faf6591 #37: 2D restore 改为区间拼接而非递归查找)。 zzf-refactor 的部分工作基于老 main，与 #37 在 planner/context/verl_mcore 上 有方向性冲突，按以下策略解决（冲突偏 main）： 采用 main #37 的 restore 逻辑（区间拼接，planner 只发射 prefix-last， interior 交给 2D 批量切片）与 PrefixLastRestoreSpec 命名； 丢弃 zzf 的 PrefixLastRestoreSpec→PrefixRestoreSpec 改名。 保留 zzf 的命名规范（应用到 main 重做代码上）： - input_ids → sequences（planner/detector 参数与用法） - index → seq_idx（循环变量） - detection → detect_result（变量/参数） - matched → matched_prefix_len、matched_provider → matched_provider_idx - groups → prefix_groups（PrefixDetectionResult 字段 + 测试） - planner if/else 分支 keep_start, keep_end 合并写法 保留 zzf 独有的、main 没有的两处改动： - patch_manager 死代码移除（删 patch_manager/megatron_attention/test_patch_integrations， 清理 verl_mcore/__init__ 死代码）—— main 未做 - TriePrefixDetector 修复（f4f3edcb：prefix mismatch 后单次遍历完成 suffix 插入）—— main 未碰 prefix_detector，移植到 b441786 命名上干净应用 丢弃：链式复用测试 test_prefix_last_in_chain_reuse_resolves_to_ancestor_with_packed_slot （#37 重做后移除该测试）。 验证：unit_test 175 passed，integrated collection 干净。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- 合并 origin/main 的 CPU CI 工作流（.github/workflows/ci.yml） - 解决 verl_mcore.py import 冲突，保留 Iterator / ensure_global_packed_token_lengths / PatchHandle - 同步 requirements-ci.txt、pyproject.toml ruff/coverage 配置、.pre-commit-config.yaml - 本地验证：209 passed / 29 skipped，ruff F,E9 clean，核心 coverage 75% Co-authored-by: Cursor <cursoragent@cursor.com>
Jackie2049
changed the title
Jackie2049
changed the title
合并 main #37 时，patch_manager 移除 diff（37eac6e4，针对老 verl_mcore 写） 套用到 main 重构后的 verl_mcore.py，import 清理不完整，留下 3 个未用 import： - typing.Iterator（prefix_sharing_enabled 删除后无引用） - ensure_global_packed_token_lengths（无引用） - patch_manager.PatchHandle（无引用，且 patch_manager.py 已删 → 潜在 ImportError） 触发 CI ruff F401 门禁失败。ruff --fix 清理，全量 ruff 通过。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
📊 Coverage reportChanged files in this PR |
Jackie2049
changed the title
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
1 participant
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.
变更摘要
本次 PR 以重构为主，核心改动如下：
命名与语义统一
PrefixLastRestoreSpec及相关属性重命名为PrefixRestoreSpec/prefix_restore_specs，准确反映其同时处理 prefix-last 与 interior restore 的语义架构清理
patch_manager相关逻辑，改用更清晰的集成方式TriePrefixDetector单次遍历在 prefix mismatch 后 suffix 插入不完整的问题测试改进
附带变更（合并 main #38 引入）
origin/main的 CPU CI 配置（.github/workflows/ci.yml、requirements-ci.txt、.pre-commit-config.yaml、pyproject.tomlruff/coverage 设置）verl_mcore.pyimport 冲突，保留Iterator、ensure_global_packed_token_lengths、PatchHandle测试结果
PYTHONPATH=prefix-sharing pytest -q prefix-sharing/tests/unit_test prefix-sharing/tests/integrated_test prefix-sharing/tests/system_testruff check --select F,E9cleanimportorskip自动跳过（GitHub 免费 runner 无 GPU/NPU）本地验证通过，符合 PR 规范。