Skip to content

Add post-implement verification that planned work was actually completed#298

Merged
bamdadd merged 1 commit into
mainfrom
lintel/feat/bdaffd2e-after-the-implement-stage-finishes-add-a
Apr 7, 2026
Merged

Add post-implement verification that planned work was actually completed#298
bamdadd merged 1 commit into
mainfrom
lintel/feat/bdaffd2e-after-the-implement-stage-finishes-add-a

Conversation

@bamdadd

@bamdadd bamdadd commented Apr 7, 2026

Copy link
Copy Markdown
Owner

Summary

Insert a standalone VerifyImplementationNode between the implement and review stages that cross-references plan tasks against sandbox-verified modified files, routes back to implement on failure (up to a configurable retry cap), and emits structured audit events for completeness scoring.

Changes

  • Add VerificationResult type and new events to contracts
  • Extend WorkflowState with verification and loop-guard fields
  • Implement the VerifyImplementationNode
  • Wire the new node into the LangGraph graph with conditional edges
  • Update projections to handle new verification events
  • Unit tests for VerifyImplementationNode
  • Update graph topology tests and integration tests

Context

After the implement stage finishes, add a verification step (either as part of review or as a separate check) that compares the plan output against what was actually implemented.

Currently the implement stage can finish without completing all planned tasks, and the review stage only checks code quality — not plan completeness.

Acceptance criteria:

  • Compare plan tasks/changes against actual file modifications in the sandbox
  • Flag any planned tasks that have no corresponding code changes
  • If significant planned work is missing, either fail the stage or loop back to implement
  • Log the verification result in the stage logs for visibility

Review

✅ Automated review passed.

Tests

✅ All tests passing.


Raised by Lintel 🤖

@bamdadd

bamdadd commented Apr 7, 2026

Copy link
Copy Markdown
Owner Author

Review

VERDICT: APPROVE

Summary

This diff adds a verify_implementation workflow node that cross-references plan tasks against sandbox-modified files, routing back to implement if completeness is below threshold (with a retry cap). The changes are well-structured and follow project conventions.

Findings

Correctness:

  1. route_after_verification reads stale implement_attempt_count: The routing function reads state.get("implement_attempt_count", 0), but the incremented value (attempt_count + 1) is only written to state in the execute return dict. Since LangGraph applies state updates after the node completes, route_after_verification will see the already-incremented value from the previous iteration — which is actually correct behavior. However, the logic in execute checks attempt_count >= MAX_IMPLEMENT_ATTEMPTS (pre-increment) while route_after_verification also checks attempt_count >= MAX_IMPLEMENT_ATTEMPTS (post-increment from previous round). These are consistent, so no bug here.

  2. _emit_event uses asyncio.get_event_loop(): This is deprecated in Python 3.10+ and can raise a DeprecationWarning or fail in some contexts. Since the node is already async, asyncio.get_running_loop() would be more correct. However, since this is best-effort fire-and-forget and failures are silently caught, this is a minor issue that won't break functionality.

  3. _task_addressed same-stem matching is overly broad: Matching on mod_path.stem == task_stem without considering directory could produce false positives (e.g., tests/utils.py matching src/utils.py). This could inflate the completeness score. Given the fallback of "same parent directory" also being broad, the verification may be lenient — but this is a design choice, not a bug.

  4. git diff HEAD~100 HEAD: The hardcoded HEAD~100 is a heuristic that could fail on shallow clones or repos with fewer than 100 commits. The fallback chain (|| git diff --cached || git diff) handles this gracefully.

Security: No issues. No user input is interpolated into shell commands (sandbox_id and workspace_path come from trusted state, not user input).

Quality: Code is clean, well-documented, and follows project patterns. The VerificationResult dataclass is properly frozen and registered. State fields are correctly typed with TYPE_CHECKING guard.

@bamdadd
bamdadd force-pushed the lintel/feat/bdaffd2e-after-the-implement-stage-finishes-add-a branch 2 times, most recently from 09141ed to d44ff5f Compare April 7, 2026 20:40
DM events and Enterprise Grid events don't always include the `team`
field, causing `translate_message_event` to return None. Now falls back
to `user_team` field, then `connection_id`, then "unknown".

Also adds debug logging when messages are skipped for easier diagnosis.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@bamdadd
bamdadd force-pushed the lintel/feat/bdaffd2e-after-the-implement-stage-finishes-add-a branch from d44ff5f to d965ba0 Compare April 7, 2026 20:56
@bamdadd
bamdadd merged commit 671b908 into main Apr 7, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant