Reading: Part 16. Teamwork and Code Review

Lesson 1 of 5 in module «Part 16. Teamwork and Code Review»
You are viewing the lesson without signing in. Sign in to save progress and take tests.

Part 16. Team Collaboration and Code Review

So far the tutorial has guided a single developer through the full SDD cycle. In a team, a second layer of complexity appears: the specification becomes not just a prompt for the agent, but a contract between people. If this contract is not reviewed, the agent may implement neat code based on a weak or incomplete description.

The main rule of team SDD:

In a merge request, not only the code is reviewed.
The bundle is reviewed: requirements -> plan -> validation facts -> changes.

What the workflow looks like

flowchart TD
  A["Feature branch"] --> B["Specification<br/>requirements.md<br/>plan.md<br/>validation.md"]
  B --> C["First review<br/>are the boundaries and facts clear"]
  C --> D{"Can we implement?"}
  D -- "no" --> B
  D -- "yes" --> E["Implementation"]
  E --> F["Check against validation.md"]
  F --> G["Pull request"]
  G --> H["Code and specification review"]
  H --> I{"Ready to merge?"}
  I -- "no" --> B
  I -- "yes" --> J["Merge and replanning"]

In a small team, the first review can be done verbally. In a large team, it is better to leave a short comment in the merge request: "boundaries are clear, validation facts are sufficient, implementation can begin." This saves time during the final review.

What the author checks before opening a merge request

Before opening a merge request, the author must answer four questions.

  1. Does the feature have a separate folder in specs/?
  2. Did the agent change files outside the feature boundaries?
  3. Have all mandatory facts from validation.md been checked?
  4. If facts changed during the work, is it clear why they changed?

The last question is especially important. A bad sign: the agent changes validation.md after a test failure to make the check easier. A good sign: the author explicitly writes in the PR that one fact turned out to be wrong, explains the reason, and adds a new fact.

What the reviewer checks

In an SDD project, the reviewer looks at four layers.

Requirements. Is it clear who the feature is for, what is within the boundaries, what is not, and what decisions have already been made?

Plan. Do the task groups correspond to the requirements? Is there a hidden large refactoring unrelated to the feature?

Facts. Do the facts check actual behavior, not intent? Is there at least a portion of machine-checkable facts?

Code. Does the implementation match the plan and the facts? Are there changes the agent made "while at it"?

If the reviewer looks only at the code, SDD degrades into ordinary review with a lot of Markdown files.

Merge request template

The minimal template is moved to the appendix: appendix-c-checklists.md. It can be copied to .github/pull_request_template.md.

The point of the template is not bureaucracy. It forces the author to show the connection between the specification and the changes:

  • which specification folder belongs to the branch;
  • which validation facts passed;
  • which facts are postponed and why;
  • which files were changed outside the expected area;
  • what the reviewer should pay special attention to.

If the template becomes longer than the merge request itself, shorten it. For most features, 6–8 items are enough.

When to fix the specification in the same branch

The specification can be fixed in the same branch if the clarification is related to the current feature and does not change the project strategy.

Examples of normal edits:

  • clarify the expected HTTP status;
  • add a manual UI validation fact;
  • fix an incorrect route name;
  • mark a fact as postponed with a reason;
  • add a decision made during implementation.

A bad edit: rewriting the feature boundaries to match code that was accidentally written. This is not a clarification, but a hidden change of task.

When a separate replanning branch is needed

A separate replanning branch is needed if the change affects the project constitution:

  • the stack changes;
  • the phase order changes;
  • a new architectural boundary appears;
  • the MVP definition changes;
  • several future features must account for a new rule;
  • QWEN.md or the team skill needs to be rewritten.

This approach reduces conflicts and makes history clear: the feature branch implements the feature, the replanning branch changes the process or roadmap.

Conflicts in roadmap.md

roadmap.md often becomes a bottleneck. Two people may simultaneously start different phases and both change the roadmap status.

A practical rule:

In a feature branch, you can read roadmap.md and reference a phase.
It is better to change roadmap.md statuses before merging or in a short replanning branch.

If the team is large, establish a roadmap owner rule. This is not a process manager, but a person who ensures that phase statuses do not diverge from reality.

How to use Qwen Code in review

Qwen Code is useful as a second reader, but not as a replacement for a reviewer.

A good prompt:

/clear
Read @QWEN.md, the specification of this branch, and git diff.
Compare the implementation with requirements.md, plan.md, and validation.md.
Make a list of discrepancies.
Do not change files.

A bad prompt:

Review the merge request and fix everything you find.

In the second case, the agent mixes review, fixes, and decision-making. In team work this is dangerous: review should first create a clear list of discrepancies, and only then the author decides what to fix.

What should not be given to the agent on autopilot

Do not give the agent without human decision:

  • expanding feature boundaries;
  • changing the technology stack;
  • removing facts from validation.md;
  • changing the security policy;
  • updating the roadmap of multiple phases;
  • merging a merge request;
  • rewriting Git history;
  • deleting data or migrations that cannot be repeated.

The agent may suggest changes. The human is responsible for the decision.

Practical exercise

Take any already implemented AgentClinic feature and open it as a mental merge request.

  1. Find the specification folder.
  2. Write out the three main facts from validation.md.
  3. Compare them with the changed files.
  4. Find at least one question a reviewer would ask.
  5. Formulate a short merge request description using the template from Appendix C.

If the merge request description cannot be written without guesswork, the specification is not portable enough.

Related parts

  • Part 18 covers security risks during review: what to look for in a diff regarding secret leaks, new MCP servers, hook changes.
  • Part 20 (antipatterns) lists situations the reviewer should immediately reject: specification without facts, facts weakened during work, implementation of a future phase.
  • Part 17 describes which hooks help automate part of the review checks.
My notes
0 / 10000

Notes are saved in this browser. They will not appear on another device.

Course menu

Course

Specification-Driven Development with Qwen Code CLI
Progress 0 / 135