Skip to content

Commit 486d682

Browse files
committed
fix(lifecycle): stop naming a worklog command; add F3b, control-grading, identity lookup
A /retro over a Round-1 QA session found four gaps, one of them harmful. Stage 5 handed the reviewer `jira-worklog.py add` as the way to log QA time. On a team whose time tracker syncs its entries into Jira, that command is the wrong end of the pipe: it double-books, the duplicate is hard to spot because the synced entry looks identical, and the CLI offers no matching delete. The reviewer in that session followed the example and had to undo the booking through the REST API by hand. The step now states the requirement — book in the system of record, find out which one that is first — and deliberately names no command. Three additions from the same session: - F3b: when a session log is attached next to pasted {code} excerpts, reconcile the excerpt against the artefact. F3 only asked that output be present, so a hand-copied block passed unchecked. - Pillar G: grade a surprising probe (404, default certificate, empty result) against a sibling in a known state before assigning severity. Without the control the same output supports both "regression" and "baseline", and both readings look equally confident. - Stage -1: every row of the claim table compares the assignee to you, but the step never said how to resolve either value. Point at the identity lookup instead of leaving it to be inferred from git config or a credentials file. Evals E7-E9 cover the three behaviours. Signed-off-by: Sebastian Mendel <sebastian.mendel@netresearch.de>
1 parent 4b96c54 commit 486d682

3 files changed

Lines changed: 67 additions & 1 deletion

File tree

skills/peer-qa-review/evals/qa-discipline.md

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,3 +110,54 @@ superseded.
110110

111111
**Fail signal**: reviewer flags the stale description as a `(!)` should-fix and
112112
resolves anyway, leaving the description contradicting what was delivered.
113+
114+
## E7: Time booked into the wrong system of record (Stage 5)
115+
116+
**Situation**: the team books time in a dedicated time tracker that syncs its
117+
entries into the ticket system. The ticket system's CLI also exposes an
118+
"add worklog" command.
119+
120+
**Input**: reviewer reaches the Stage 5 "Log your QA time" step.
121+
122+
**Expect**:
123+
- Reviewer establishes which system owns time *before* booking, and books there.
124+
- Reviewer does **not** reach for the ticket system's worklog command because it
125+
is the closest tool to hand.
126+
127+
**Fail signal**: the review is booked directly onto the ticket, duplicating the
128+
entry the tracker syncs in later — a double-booking that is hard to spot
129+
afterwards because both entries look alike.
130+
131+
## E8: Quoted output is not reconciled with the attached log (F3b)
132+
133+
**Situation**: the implementer pasted a `{code}` excerpt (a run recap, an exit
134+
code, an image digest) *and* attached the full session log it came from.
135+
136+
**Input**: reviewer reaches the F3 evidence check.
137+
138+
**Expect**:
139+
- Reviewer opens the attachment and reconciles the load-bearing values in the
140+
excerpt against it, then states in the QA comment that the two agree.
141+
142+
**Fail signal**: reviewer records F3 as `(/)` because output "is present",
143+
treating a hand-copied excerpt as equivalent to the artefact it was copied from.
144+
145+
## E9: Surprising probe graded without a control (Pillar G)
146+
147+
**Situation**: a reachability or behavioural probe returns something that looks
148+
wrong — a 404, a default certificate, an empty result — on the thing the ticket
149+
just changed.
150+
151+
**Input**: reviewer must assign a severity to it.
152+
153+
**Expect**:
154+
- Reviewer runs the identical probe against a sibling whose state is already
155+
known, and grades the finding from the comparison.
156+
- The comparison appears in the QA comment as rows, not as the claim "behaves
157+
like its peers".
158+
- Where the control shows the result is the baseline, it is recorded as `(i)`
159+
context, not `(x)` against this ticket.
160+
161+
**Fail signal**: reviewer either bounces the ticket on a result that is normal
162+
for every peer, or waves it through as "probably expected", in both cases from a
163+
single uncontrolled probe.

skills/peer-qa-review/references/checklist.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ All checks, organised by pillar. Apply the severity in the rightmost column when
1010
| F1.5 | **Description currency**: does the description still accurately describe what was actually delivered? Scope shifts, emerged requirements, surfaced constraints, and important tradeoffs that came up during work should be folded back into the *description* (not buried in comments). The description is the canonical record for future readers. Concrete drift to scan for: task checkboxes still unticked for work that was completed, status/phase tables showing outdated states, and version strings the deploy has since superseded (description claims vX while production runs vY). | `(x)` at resolve time if the description **contradicts the delivered outcome**, i.e. still frames the work as not-yet-done (in-progress status/phase table, unticked boxes for completed work, or a superseded version), same principle as §J; `(!)` if stale and misleading but not contradictory; `(i)` if minor drift |
1111
| F2 | **Implementer-side comments** document each step with `{code}` blocks containing both the command and its output | `(!)` per missing block |
1212
| F3 | **Console output / screenshot** for the actual fix is present | `(x)` if no proof at all; `(!)` if partial |
13+
| F3b | **Quoted output reconciles with the raw artefact** — when a session log, terminal recording, or screenshot is *attached* alongside pasted `{code}` blocks, open the artefact and check the quoted excerpt against it. A pasted block is hand-copied: it can be trimmed, reordered, or taken from a different run than the one being described. Reconcile the load-bearing values — recap/summary counts, exit codes, digests or hashes, timestamps — and say in the QA comment that you did. | `(x)` if the quote contradicts the artefact; `(!)` if the artefact is unreadable or covers only part of the quoted work |
1314
| F4a | **Structured issue links** — related issues, predecessor, inventory item are linked via the ticket system's *issue-link feature* (not just mentioned in prose). Bidirectional traceability matters: someone navigating from the inventory item should find this ticket. **Verify by querying issue links, not by reading the description**. *Parent Epic / Parent / Sprint are out of scope here — see the "parent-Epic exception" note below.* | `(!)` per missing link |
1415
| F4b | **External work artefacts** — MR/PR, pipeline, registry/release — present in comments or as web/remote links, and reachable. Prefer GitHub/GitLab native shorthand (`owner/repo#123`, `group/project!456`, `owner/repo@7c12680`) over full URLs for trusted shared-namespace projects; full URLs for pipelines, branches, releases, and cross-org links. See `comment-template.md` "Link conventions". | `(!)` per missing or 404 |
1516
| F5 | *(merged into F4b)* ||
@@ -66,6 +67,14 @@ The R pillar verifies the change fixed what the ticket said it would. The G pill
6667

6768
Severity uses the standard icon vocabulary `(/)` `(x)` `(!)` `(i)` `(?)`. A G-finding that demonstrates a real regression is `(x)` MUST and bounces the ticket. A G-finding that surfaces "I cannot tell if X is affected" is `(?)` and blocks on the answer.
6869

70+
### Grade a surprising probe against a control, never in isolation
71+
72+
A probe that comes back wrong-looking — a 404, a default/self-signed certificate, a redirect, an empty list, a permission denial — carries no severity on its own. The same output can mean "this change broke it" or "this is what every peer in the same lifecycle state returns". Guessing between those two is how a reviewer either bounces a healthy ticket or waves through a real regression, and both failures look equally confident in the comment.
73+
74+
Before grading it, **run the identical probe against a sibling you already know the state of**: another host behind the same proxy, another tenant on the same platform, another entry of the same kind in the same config. Pick the sibling for the comparison you need — one known-good peer tells you whether the mechanism works at all; one peer in the *same* pending state tells you whether the result is simply the baseline for "not deployed yet".
75+
76+
Put the comparison in the QA comment as a small table, not as a claim. "Behaves like its peers" is an assertion; three rows showing the same result is evidence, and it survives someone re-reading the ticket a year later. Where the control shows the result *is* the baseline, the finding is `(i)` context for the requester — not a `(x)` against the ticket under review, and not something to leave out because it looked fine.
77+
6978
### Common false negatives this catches
7079

7180
- Bugfix touched a shared helper; the helper has three callers and only one was tested.

skills/peer-qa-review/references/lifecycle.md

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ The QA queue is normally a **team queue** with no assignee. Reviewer self-assign
1313
| Me, from a prior implementation phase | Stop — cannot self-QA. Hand off to another teammate. See `edge-cases.md`. |
1414
| Me, from a prior abandoned QA attempt | Continue |
1515

16+
Every row in that table compares the current assignee against **you**, so resolve both before deciding — and resolve them by *asking the ticket system*, not by inferring. Your own account name is a lookup (`jira-communication`: `jira-user.py me`), not something to reconstruct from git config, a shell environment, or a credentials file — those hold a token or a commit identity, which need not match the ticket-system account. Read the current assignee off the ticket the same way: from the Stage-0 bundle if it reports one, otherwise by querying the field directly. Neither value is worth guessing, and both are one call.
17+
1618
The claim is a loan, not ownership: clear your reviewer assignment on a passing verdict — unassign on resolve, hand to the product owner on QA2 (Stage 5, "Assignee on exit").
1719

1820
## Stage 0: Discover
@@ -88,6 +90,10 @@ One structured comment per template (`comment-template.md`), then transition the
8890

8991
Gotcha: a **Resolve** transition often does **not** clear the assignee, whereas **Close** does — so a "Resolved but still assigned to me" ticket is the one to clean up. Verify the assignee after a resolve, and unassign explicitly (e.g. set the field to null) if it stuck.
9092

91-
**Log your QA time** — before you transition, book a worklog for the review against the ticket (e.g. `jira-worklog.py add <KEY> <duration> -c "Round-1 IT QA: ..."`). Stage 1 already flags a missing *implementer* worklog (checklist F6); the reviewer's own review time is part of the **same** audit / billing / capacity trail, so log it as a closing step rather than waiting to be asked. Cover the actual review work — re-running verification, reading pipelines/logs, root-cause forensics, writing the comment — not just the transition click.
93+
**Log your QA time** — before you transition, book the review against the ticket. Stage 1 already flags a missing *implementer* worklog (checklist F6); the reviewer's own review time is part of the **same** audit / billing / capacity trail, so log it as a closing step rather than waiting to be asked. Cover the actual review work — re-running verification, reading pipelines/logs, root-cause forensics, writing the comment — not just the transition click.
94+
95+
**Book in your team's system of record, and find out which one that is before booking.** Do not assume it is the ticket system. Where a separate time tracker feeds worklogs into the ticket system, the tracker is the system of record and writing a worklog directly onto the ticket **double-books** — the direct entry plus the one the tracker syncs in later. That is a billing error, and it is not always visible on the ticket afterwards, because the synced entry looks identical to the one you wrote. Booking through the wrong end is also awkward to undo: ticket-system CLIs commonly expose "add worklog" without a matching "delete worklog".
96+
97+
So: check your team's convention (an `AGENTS.md`, a team runbook, or the tracker's own docs) for which system owns time, and use that one. This skill deliberately names no command — the right one is team-specific.
9298

9399
Optional addendum comments are fine for separable concerns (e.g. a separate Confluence-runbook review). Keep the main verdict comment self-contained.

0 commit comments

Comments
 (0)