feat: resolve labeled GitLab review discussions - #120
Conversation
Publish GitLab review findings as replyable discussions even when a diff anchor cannot be created, while keeping the aggregate review summary as a plain note. Expose provider-neutral resolution state and resolve a discussion only after a valid ccr:label reply lands. Resolution remains best-effort so a successful label is not duplicated when GitLab cannot close the thread. Update the label-review workflow, regression coverage, and plugin version to 0.2.12.
| return 1 | ||
| print(f"replied to {ns.comment_id} on {pr_label(forge.provider, number)}") | ||
| suffix = "" | ||
| if review_feedback.label_verdict(ns.body) and target.resolvable and not target.resolved: |
There was a problem hiding this comment.
🤖 devloop code-review · seed-2.1-pro
label_verdict() returns a str, not bool. While this works correctly here because empty strings are falsy and valid verdicts are truthy, it's an implicit coercion that could mislead future readers. The function is used as a boolean predicate but its return type is str. Consider extracting to a local variable for clarity — e.g., verdict = review_feedback.label_verdict(ns.body) then if verdict and target.resolvable ... — which also avoids calling label_verdict twice if the verdict is needed later.
ccr:fp=4f43378fb247
There was a problem hiding this comment.
ccr:label=debatable — label_verdict 的返回值契约就是合法 verdict 或空串，此处只调用一次且只需真假判断；提取局部变量属于风格偏好。
| def reply(self, number, target, body): | ||
| self.replied.append((number, target.id, body)) | ||
| def resolve_thread(self, number, target): | ||
| self.resolved.append((number, target.thread_id)) |
There was a problem hiding this comment.
🤖 devloop code-review · seed-2.1-pro
The _F._FakeForge.resolve_thread stub does not mutate the comment's resolved field, so the second assertion (assert fake.resolved == [(5, "30")]) cannot distinguish between "the guard correctly skipped re-resolution" and "the guard is missing but the fake happens to only record one call." Consider also updating a _resolved tracking dict keyed by comment id so comments() returns the updated resolved state, making the gate in cmd_reply truly exercised.
ccr:fp=0d500987fff2
There was a problem hiding this comment.
ccr:label=wrong — 第二次调用拿到的 target 仍明确是 unresolved；若缺少合法 label 条件，普通回复会再次调用 resolve_thread 并让列表出现第二项，现有断言能检出。该用例本就不负责测试 resolved 分支。 #textbook
There was a problem hiding this comment.
🤖 devloop code-review · deepseek-v4-pro
The _FakeForge base class in _testkit.py does not override thread_comment, so it inherits Forge.thread_commit which raises ForgeError. The Refusing test case at line 418-423 implicitly depends on this inherited behavior to keep n2 == 0. If someone later adds a thread_comment override to _FakeForge, the Refusing test would break because thread_comment would now succeed for the 3rd comment ("no path at all") even though diff_comment still fails. Consider making Refusing also explicitly override thread_comment to raise ForgeError, or adding a comment noting the dependency.
ccr:fp=3c419ad54d8f
There was a problem hiding this comment.
ccr:label=debatable — Refusing 有意组合“diff anchor 拒绝 + Forge 默认不支持无锚 thread”以覆盖最终回落；未来若公共 FakeForge 新增 thread 支持，测试期望也应随能力变化。显式重写只是隔离测试替身的防御性选择。 #padding
|
🤖 devloop code-review · 3 finding(s)（3 条已锚到 diff） |
2 participants
Publish GitLab review findings as replyable discussions even when a diff
anchor cannot be created, while keeping the aggregate review summary as a
plain note.
Expose provider-neutral resolution state and resolve a discussion only after
a valid ccr:label reply lands. Resolution remains best-effort so a successful
label is not duplicated when GitLab cannot close the thread.
Update the label-review workflow, regression coverage, and plugin version to
0.2.12.