Study guide: Part 16. Teamwork and Code Review

Lesson 2 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.

Topic: Part 16. Teamwork and Code Review

Difficulty level: Medium

Estimated study time: 4-6 hours (theory: 2 hours, practice: 2-4 hours)

Prerequisites: Parts 1-15 of the SDD course (understanding the specification: requirements.md, plan.md, validation.md)

Basic Git knowledge and branch workflow

Experience working with pull requests in GitHub/GitLab

Understanding the role of Qwen Code as a development agent

Learning objectives: Compose and use four-layer review (requirements → plan → facts → code) for checking merge requests in an SDD project

Apply a merge request template to demonstrate the connection between specification and code changes

Determine when to edit specification in a feature branch and when a separate replanning branch is required

Use Qwen Code as a second reader during review, preventing automatic code correction by the agent

Identify changes that cannot be handed to the agent on autopilot and ensure human control over critical decisions

Overview: This part of the course transitions from individual development to teamwork within Specification-Driven Development (SDD). Key insight: in a team, the specification becomes not just a hint 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 — and it will look like success until hidden problems are discovered. The main rule: in a merge request, review not only the code, but the bundle "requirements → plan → verification facts → changes". The section covers the full workflow from feature branch to merge, roles of author and reviewer, PR templates, managing conflicts in roadmap.md, and boundaries of automation with Qwen Code.

Key concepts: Main rule of team SDD: In a merge request, review not only the code. Review the bundle: requirements → plan → verification facts → changes. This prevents the situation where neat code implements an incorrect specification.

Workflow: Feature branch → specification (requirements.md, plan.md, validation.md) → first review of boundaries and facts → decision "can we implement?" → implementation → check against validation.md → pull request → code and specification review → merge and replanning. First review in a small team can be verbal, in a large team — a short comment in PR.

Four questions for the author before opening a PR: 1) Does the feature have a separate folder in specs/? 2) Did the agent change files outside the feature boundaries? 3) Are all mandatory facts from validation.md verified? 4) If facts changed during work, is it clear why they changed? The last question is critical: a bad sign — the agent changes validation.md after test failure to make verification easier; a good sign — the author explicitly explains the reason for changing facts.

Four layers of the reviewer: Requirements (are boundaries clear, who is the feature for, what is out of scope), Plan (do task groups match requirements, is there hidden refactoring), Facts (do they verify actual behavior rather than intent; are there machine-verifiable facts), Code (does implementation match plan and facts, are there "while we're at it" changes). If the reviewer looks only at code, SDD degrades to ordinary review with extra Markdown files.

Merge request template: The minimal template is placed in appendix-c-checklists.md and copied to .github/pull_request_template.md. The purpose — to force the author to show the connection between specification and changes: specification folder, passed facts, deferred facts with reasons, files outside expected area, especially careful checks for the reviewer. For most features, 6–8 items are sufficient.

Editing specification in a feature branch: Allowed for clarifications related to the current feature and not changing project strategy: clarify HTTP status, add manual fact for UI verification, fix route name, mark fact as deferred, add a decision made during implementation. Bad edit: rewriting feature boundaries to match accidentally written code.

Replanning branch: Needed for changes affecting the project constitution: stack change, phase order, new architectural boundary, MVP change, rule for several future features, rewriting QWEN.md or team skill. Separates feature implementation from process/roadmap changes.

Conflicts in roadmap.md: Bottleneck: two people may simultaneously change statuses of different phases. Rule: in a feature branch you can read and reference, changing statuses is better before merge or in a short replanning branch. In a large team — rule of roadmap owner.

Qwen Code in review: Useful as a second reader, not as a replacement for the reviewer. Good request: read QWEN.md, branch specification, git diff; compare implementation with three specification files; compose a list of discrepancies; do not change files. Bad request: "check the PR and fix everything" — the agent mixes review, correction, and decision-making, which is dangerous in teamwork.

Autopilot prohibitions for the agent: Without human decision, cannot hand over: expanding feature boundaries, stack change, deleting facts from validation.md, changing security policy, updating roadmap.md for multiple phases, merging PR, rewriting Git history, irreversible migrations/data deletion. The agent may propose, the human decides.

Practice exercises: Name: Exercise 1: Audit PR readiness using four questions

Problem: You are given a feature branch "adding patient filtering by diagnosis" in the AgentClinic project. The author opened a PR. Upon inspection you find: folder specs/patient-filter/ exists; in git diff there are changes to specs/patient-filter/requirements.md, specs/patient-filter/plan.md, specs/patient-filter/validation.md, src/services/patient_service.py, tests/test_patient_filter.py; but also changed is file src/models/base_model.py (added field is_archived). In validation.md the last fact was changed from "Filter returns only active patients" to "Filter returns patients where is_archived = false", and in commit history this change was made after test test_active_patients_only failed. Write which of the four questions the author failed to answer satisfactorily, and formulate a reviewer comment.

Solution: Step 1: Check question 1 — folder specs/patient-filter/ exists, answer "yes". Step 2: Check question 2 — src/models/base_model.py was changed outside feature boundaries, answer "no, it was changed". Step 3: Check question 3 — impossible to verify unambiguously, but assume facts were checked. Step 4: Check question 4 — fact was changed after test failure, reason not explicitly explained, appears to be weakening the check. Reviewer comment: "Question 2: change to base_model.py goes beyond feature boundaries. Either extract to a separate feature, or justify in PR why it is necessary for filtering. Question 4: fact about active patients was changed after test failure without explaining the reason. This looks like adapting specification to code, not clarifying requirements. Please restore the original fact or explicitly explain the business reason for the change and add a new fact confirming correctness of the new behavior."

Complexity: intermediate

Name: Exercise 2: Separating edits into feature branch and replanning branch

Problem: In the AgentClinic project, in the feature branch "PDF report export" the developer wants to make the following changes. Determine which can be done in the feature branch and which require a separate replanning branch: (A) add to validation.md manual fact "PDF opens in Acrobat Reader without validation errors"; (B) replace PDF generation library from WeasyPrint to Playwright + Chromium; (C) add to requirements.md clarification that export supports only Latin and Cyrillic; (D) move report generation from services/ module to new module reporting/; (E) update roadmap.md, marking phase "Reports" as completed and adding phase "Report Analytics"; (F) change QWEN.md, adding rule "all new modules are created with agentclinic_ prefix".

Solution: Step 1: Analyze each change by criterion — does it affect project constitution or only concern the current feature. (A) Manual fact in validation.md — clarification of current feature, feature branch. (B) Stack library change — changes technology choice, affects other features and project future, separate replanning branch. (C) Clarification of supported characters — boundary of current feature, feature branch. (D) New architectural boundary of module reporting/ — affects project structure and future features, separate replanning branch. (E) Updating roadmap.md with new phase — changes roadmap of multiple phases, separate replanning branch (or short replanning branch). (F) Changing QWEN.md — changes team skill, separate replanning branch. Result: feature branch — A, C; separate replanning branch — B, D, E, F.

Complexity: intermediate

Name: Exercise 3: Composing a PR template for a specific feature

Problem: A developer completed the feature "doctor notification on critical patient indicator change" in AgentClinic. Specification is in specs/critical-alerts/. From validation.md, 4 of 5 facts were verified: fact "Notification delivered in < 30 seconds" is deferred, as it requires load testing planned for next iteration. In git diff changed: specs/critical-alerts/validation.md (marking deferred fact), src/services/alert_service.py, src/notifications/push_sender.py, tests/test_critical_alerts.py. Unexpectedly: also changed src/config/settings.py — added environment variable ALERT_TIMEOUT_MS. Compose a PR description using the template from Appendix C, 6–8 items.

Solution: Step 1: Specify specification folder. Step 2: List verified facts. Step 3: Mark deferred fact with reason. Step 4: Explain change outside expected area. Step 5: Indicate what the reviewer should check especially carefully. Step 6: Give overall readiness assessment. Example result: 1. Specification: specs/critical-alerts/ (requirements.md, plan.md, validation.md). 2. Verified facts: notification is created on critical change; contains patient ID and indicator; sent to assigned doctor; logged in audit trail. 3. Deferred: fact "delivery < 30 sec" — requires load testing, planned for sprint 14. 4. Change outside boundaries: src/config/settings.py — added ALERT_TIMEOUT_MS for unified notification timeout management. Justification: avoid hardcoding in alert_service.py. 5. Check carefully: correctness of connection between alert_service.py and push_sender.py; whether ALERT_TIMEOUT_MS creates conflicts with other timeouts in settings.py. 6. Readiness: feature is functional, deferred fact does not block MVP.

Complexity: intermediate

Name: Exercise 4: Formulating a request to Qwen Code for review

Problem: You are a reviewer in an SDD project. The PR author asks "let Qwen check and fix the code, then you look". Explain why this is bad practice, and compose a correct request to Qwen Code that you as reviewer would give the agent to assist in review, preventing automatic correction.

Solution: Step 1: Explain the problem: request "check and fix" mixes three roles — review (identifying discrepancies), correction (changing code), and decision-making (what to fix and what not to). In teamwork this is dangerous: the agent may make decisions that should be made by a human (for example, weaken a fact or expand boundaries). Review should first create a clear list of discrepancies, and the author decides what to fix. Step 2: Compose correct request. Result: "/clear. Read @QWEN.md, branch specification specs/critical-alerts/ (requirements.md, plan.md, validation.md) and git diff of this branch relative to main. Compare implementation with each of the three specification files. Compose a list of discrepancies: where code does not match requirements, where implementation differs from plan, where validation.md facts are not verified or are incorrectly verified. Do not change files. Format result as a numbered list with file and line reference."

Complexity: intermediate

Case studies: Name: Case: SDD degradation to "Markdown review" at startup MedFlow

Scenario: MedFlow — a startup of 4 developers who adopted SDD after the course. First two months they worked by the rule "review the requirements-plan-facts-code bundle". As load grew, the senior developer suggested "speeding up": the reviewer started looking only at git diff and validation.md, skipping requirements.md and plan.md. PR authors began minimizing specification, and Qwen Code was used for auto-filling PR templates.

Challenge: After 6 weeks the team discovered that a feature "laboratory integration" was implemented that didn't actually do what the business required: the code neatly transfers data to the laboratory API, but requirements.md had vague boundaries ("integration" without clarifying direction). The plan assumed two-way exchange, but implementation only did order submission without receiving results. validation.md verified correct HTTP statuses, but didn't verify existence of the callback endpoint. The feature passed review because code matched the weak specification. The business lost 3 weeks on rework.

Solution: The team introduced a strict rule: PR without explicit reference to requirements.md and plan.md is automatically returned. The reviewer is obligated to ask at least one question for each of the four layers. Qwen Code was switched to "discrepancy list only" mode with prohibition on auto-correction. A roadmap.md owner was assigned with weekly rotation. The PR template was reduced to 6 mandatory items, but made them strict.

Result: Average review time grew from 15 to 40 minutes, but number of reworks dropped by 70%. The next integration feature (with payment gateway) passed with one review iteration instead of previous four. The CTO noted that "slowing down" on review paid off with acceleration at business acceptance stage.

Lessons learned: Skipping review layers (especially requirements and plan) makes SDD a formality and fails to protect against implementing incorrect specification

Auto-filling templates by agent destroys their purpose — the template should make the author think, not save time on copying

Template brevity matters more than completeness: 6 strict items work better than 15 optional ones

Related concepts: Main rule of team SDD

Four layers of the reviewer

Merge request template

Qwen Code in review

Name: Case: Conflict in roadmap.md and introducing roadmap owner

Scenario: AgentClinic — a project of 8 developers working with SDD. The roadmap in roadmap.md contained 5 phases. Two developers, Alexey and Maria, worked in parallel on phases 3 ("Sensor integration") and 4 ("Indicator analytics"). Both changed the status of their phases in roadmap.md in their feature branches.

Challenge: When merging the first PR, phase 3 status changed to "completed", and phase 4 — to "in development". When merging the second PR, a conflict occurred: Maria changed phase 4 status to "in development" in her branch, but by then Alexey had already added phase 5 "Predictive analytics" to main. Git could not automatically merge roadmap.md changes. Conflict resolution took 2 hours, and the status of phase 2 ("Patient registration") was erroneously lost, which someone had corrected earlier.

Solution: The team introduced a rule: in a feature branch you can read roadmap.md and reference a phase, but change statuses only in a short replanning branch or directly before merge with explicit agreement. A roadmap owner was assigned — role rotates weekly, responsibility: verify that phase statuses match reality, and be the only one who commits roadmap.md changes to main.

Result: Conflicts in roadmap.md disappeared. Sprint planning time reduced from 4 hours to 45 minutes. The roadmap owner became a natural synchronization point between developers, which unexpectedly improved team communication.

Lessons learned: roadmap.md — a bottleneck in parallel work, requiring explicit ownership rule

Rotation of roadmap owner role prevents bus factor and involves everyone in understanding strategy

Separating status changes from feature branches reduces conflicts and makes history understandable

Related concepts: Conflicts in roadmap.md

Replanning branch

Workflow

Study tips: Go through the material in parallel with a real or training project: try to open any completed PR as a "mental review" using four layers — this turns abstract rules into a skill

Print or keep at hand the checklist of four author questions and four reviewer layers — use them literally for the first 5 PRs until it becomes automatic

Practice formulating "good" and "bad" requests to Qwen Code: write 3 good and 3 bad, explain the difference to a colleague or in a study chat — teaching others strengthens understanding

Create a training repository with several feature branches and intentionally introduce typical errors in one of them (change outside boundaries, weakening fact after test failure, missing specs folder) — practice identifying them using the checklist

Study related parts 17, 18, 20 not sequentially, but in parallel: review automation (17), review security (18), and antipatterns (20) — these are three facets of one process, and understanding their interconnection is deeper than isolated study

Keep a "reviewer diary": record which questions you ask on real PRs, which find problems, which turn out to be false alarms — in a month you will have a personalized checklist

For visual learning style: draw the workflow from feature branch to merge on paper, mark where exactly each check happens and who is responsible for it

Additional resources: Appendix-c-checklists.md: Original merge request template from the course — basis for .github/pull_request_template.md

Part 17. Review automation checks: Which hooks help make part of review checks automatic — continuation of teamwork topic

Part 18. Security during review: What to look for in diff: secret leaks, new MCP servers, hook changes — critical for real projects

Part 20. Antipatterns: Situations for immediate PR rejection: specification without facts, facts weakened during work, implementation of future phase

GitHub Docs: About pull request templates: https://docs.github.com/en/communities/using-templates-to-encourage-useful-issues-and-pull-requests/creating-a-pull-request-template-for-your-repository — technical instruction for template implementation

"Code Review Best Practices" by Palantir: https://blog.palantir.com/code-review-best-practices-19e02770015f — classic article complementing the SDD-specific approach with general review principles

Book "Working Effectively with Legacy Code" by Michael Feathers: Chapter about "seams" and change boundaries — useful for understanding why it's important not to go beyond feature boundaries

Summary: Teamwork in SDD requires rethinking the role of specification: it becomes a contract between people, not just a hint for the agent. The main rule — review the bundle "requirements → plan → facts → code", not code in isolation. Successful application requires author discipline (four questions before PR), reviewer systematicity (four layers of checking), clarity in change boundaries (feature branch vs replanning branch), managing bottlenecks (roadmap.md owner), and conscious use of automation (Qwen Code as second reader, not replacement for reviewer). The PR template is a communication tool, not bureaucracy. Autopilot prohibitions for the agent protect critical decisions from thoughtless automation. Adherence to these principles prevents SDD degradation to formal review with extra Markdown files and ensures predictability of team development.

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