Repository navigation
关于 Commit/Range Review 输入一致性与 Resume 重复 Diff 计算的两个疑惑问题,衷心希望能解答我的困惑 #1658
Unanswered
Redeemz473
asked this question in
Q&A
Replies: 1 comment
|
我不是维护者,但对照当前 main( 1. 普通 Commit / Range Review 没有固定输入
从代码看,sealing 是为 Resume 准入引入的,还没有推广到普通路径,不像是刻意保留 ref 的动态性。 修复前可以直接传 SHA 规避(merge-base 结果不变): ocr review --from "$(git rev-parse main)" --to "$(git rev-parse feature)"
ocr review --commit "$(git rev-parse HEAD)"2. Resume 时
|
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
您好,我最近在阅读 OpenCodeReview 的 Review / Session / Resume 相关源码,有两个设计问题想向您询问一下,不确定我的理解是否正确,很希望您能够帮我解答
我看到当前 SealedInput 只在 Resume 路径中使用。Resume 会在正式 Review 前把 HEAD、main、feature 等可移动 Git Ref 解析成固定 Commit SHA,后面的 Diff、NewFileContent 和 file_read 都基于固定 SHA。但是普通非 Resume Review 中,Commit / Range Provider 好像还是保存并使用用户传入的原始 Ref。这样是否会导致出现下面这种情况:
例如:ocr review --from main --to feature
假设最开始:main → A feature → B
首先生成 Review Diff:git merge-base main feature→ 得到 Base,例如 A
git diff A feature→ 此时 feature = B,此时 d.Diff = A → B 的代码变化
解析 Diff 后,还调用了ParseDiffText(),从而会为了构造每个 model.Diff 的:NewFileContent
再次执行类似:git show feature:,这里仍然使用的是字符串 feature,而不是生成 Diff 时对应的固定 Commit B。
如果此时用户又提交了一次:feature:B → C
那么第二次读取就会变成:git show feature:a.go
同一个 model.Diff 中可能出现:d.Diff→ 来自 A → B d.NewFileContent→ 来自 Commit C 这样的一致性问题
并且在这个过程中,如果模型调用:file_read("a.go"),普通Review 的 fileReadRef() 在没有 SealedInput 时似乎仍然返回原始 Ref:feature,如果Agent运行期间调用file_read,feature仍然指向C,甚至可能已经进一步移动,file_read可能是 C或更后版本的文件
这样是否会导致出现下面的不一致情况:
Main Prompt告诉模型:“正在Review A → B”
NewFileContent的内容来自C,file_read("a.go")也可能来自C
也就是导致模型可能在审查 B 引入的代码变化,却拿 C 版本的完整代码作为上下文进行推理。
LLM 根据旧 Diff + 新文件上下文进行跨文件推理时,结论可能不一致。
另外,当前 loadDiffs() 中似乎是先 GetDiff(),随后再 ResolveInput()
如果 Ref 恰好在这两个阶段之间移动,还可能产生另一种不一致:真正产生Review Diff时:feature = B,实际Diff = A → B
后面ResolveInput时:feature = C 这就导致Manifest里面ResolvedHead = C
于是可能出现:真正Review的代码:A → B Manifest记录:A → C
但是如果用户直接传完整 Commit SHA,就不存在这个问题了
我看到了 fileReadRef() 附近的源码注释本身也提到了类似风险:如果 reader 继续使用 moving ref,可能出现模型“review one version of the diff, but read another version of the file”的情况;不过目前这层保护似乎主要在 Resume 的 SealedInput 路径生效。
所以想确认:普通 Commit / Range Review 不在运行开始阶段统一把用户 Ref 固定成 SHA,是有意保留 Ref 动态性的设计吗?还是后续也考虑把 Resume 中的 input sealing 泛化到普通 Commit / Range Review,使 Diff、NewFileContent、file_read 和 Manifest 始终基于同一组 Commit SHA?
我看到 Resume 预检阶段的
ResolveIdentity()为了计算 RunIdentity,已经执行:loadDiffs()和selectFiles()我理解当前这样设计的主要原因是:Resume 校验必须发生在
agent.New()前,因为agent.New()会立即调用 session.New();而项目又要求 rejected resume 不留下新的 Session。我在想从实现上看,是否可以考虑把预检得到的 Diff / Selection 作为一种 PreparedReviewInput 传给正式 Agent,在一次loadDiffs()和selectFiles()之后计算并校验 RunIdentity,不通过则直接退出,不创建 Session,通过则调用agent.New(),从而能够复用之前已经准备好的 Diff / Selection,不用再进行两次的Diff 和 Selection这样似乎仍然可以保证:Rejected Resume 不创建 Session,同时避免 Resume 对同一组固定 Commit 重复执行 Diff 解析、完整文件读取和文件筛选。
所以想请教:这里重复执行
loadDiffs + selectFiles是有意用额外计算换取 preflight 与正式 Agent 解耦吗?后续是否有考虑让两阶段共享已经准备好的 Review Input?All reactions