review-implementation

Name: review-implementation
Author: gtrabanco

35installs2forks

How to install review-implementation?

gtrabanco/agentic-workflow/review-implementation

$npx -y skills add gtrabanco/agentic-workflow --skill review-implementation

Installs into the current project.

Summary

Internal findings engine composed by review-change (and reused by the audit skills): two-phase find → classify pass ending in a classified decision table (fix-now / postpone / ignore / intentional-tradeoff). Findings only — never refactors.

Files · 1

View on GitHub

SKILL.md

1	# Review Implementation (internal engine)
2
3	The findings engine the review/audit skills compose: it **produces findings and a
4	decision table, and stops — never refactors or edits code. It owns the review
5	axes (Phase 1) and the classification rubric** (Phase 2) that `review-change`,
6	`audit-pr`, and `product-audit` reference instead of restating.
7
8	## When to use
9
10	- Invoked by `review-change` (the user-facing review entry) as its engine; the
11	audit skills reference its rubric.
12	- Run directly only when you want the raw classified pass without the
13	platform-adaptive orchestration `review-change` adds.
14
15	## Scope
16
17	Default target is the current change (branch diff vs. the default branch);
18	accept an explicit path/glob to widen or narrow it. State the scope at the top of
19	the report so the reader knows what was and wasn't reviewed.
20
21	## Step 0 — Discover the project (always first)
22
23	Per the agent guide's Workflow conventions + documentation map, then read
24	what THIS skill needs: the architecture/layering rules, the testing philosophy,
25	and any runtime/platform, security, money, i18n/SEO/a11y and bundle rules. Pull
26	the project's specific risk axes from its guardrail skills where present
27	(architecture-pattern, runtime/platform, domain-rules). The axis list below is the
28	default; the project's docs refine it.
29
30	## Context budget (hard rule)
31
32	The scope is the diff. Beyond it, read at most 10 non-diff files in full
33	for surrounding context (callers, contracts, tests); targeted reads (≤ 50
34	lines of a named range) and grep/glob results don't count. Record each
35	finding as its table row immediately (id, `file:line`, axis, one-line
36	evidence) and drop the raw file content — Phase 2 classifies the table, never
37	the sources. Never quote whole files into the report.
38
39	## Phase 1 — Find (no refactor)
40
41	Assume the diff is WRONG — your job is to prove it does not work. Scan the
42	scope adversarially and record every finding across these axes. Fix nothing; the
43	classification in Phase 2 decides what matters.
44
45	\| # \| Axis \| Looks for \|
46	\|---\|---\|---\|
47	\| 1 \| Bug / correctness \| Logic errors, wrong edge-case handling, races, unhandled rejections, imprecise numeric handling \|
48	\| 2 \| Architecture violation \| Broken dependency direction, business logic in the wrong layer, abstraction bypass, cross-layer shortcut (per the architecture doc) \|
49	\| 3 \| Overengineering / premature optimization \| Unnecessary abstractions, single-caller indirection, speculative generality, micro-opt without a measured bottleneck \|
50	\| 4 \| Removable / dead code \| Unused exports, unreachable branches, commented-out blocks, obsolete files — see exception below \|
51	\| 5 \| Security / cybersecurity \| Secrets in code, injection, missing authz, unsafe deserialization, PII exposure, weak crypto, SSRF, over-broad CORS, leaking errors \|
52	\| 6 \| Platform / runtime incompatibility \| APIs unavailable on the target runtime, unsupported in-memory state assumptions, runtime-incompatible deps, blocking external calls in the request path \|
53	\| 7 \| Bundle-size risk \| Heavy/duplicate deps, accidental large imports, non-tree-shakeable patterns \|
54	\| 8 \| Tests — failing/weak \| Flaky/over-mocked/snapshot-heavy tests, tests asserting nothing meaningful \|
55	\| 9 \| Tests — missing \| Uncovered branches, new use-cases/adapters without tests, SPEC dev-scenario failure modes not exercised \|
56	\| 10 \| Project-rule violations \| Whatever the project's docs mandate (e.g. domain value-object rules, no hardcoded UI strings, don't hide user-facing limitations, naming conventions) \|
57
58	### Dead-code exception (important)
59
60	Do not flag code as removable if it is **intentionally staged for an
61	in-progress or planned feature**. Before reporting axis 4, cross-check the
62	roadmap, feature SPECs/`TASKS.md`, and `known-issues.md`: if the code is wired
63	into a planned phase or another feature, classify it intentional / in-progress,
64	not dead. When unsure, mark it verify and ask — never assert "dead" on a
65	guess.
66
67	### Finding format
68
69	Each finding: a stable id (`F-1`, `F-2`, …), `file:line`, axis, a one-line
70	description, and the evidence (the code/why it qualifies). No remedy code yet.
71
72	## Phase 2 — Classify (no refactor)
73
74	Turn findings into a decision table. Classify each into exactly one of:
75
76	- fix-now — correctness/security risk, blocks the merge, is cheap to fix,
77	or is in-scope of the unit under review (see the mandatory checks below).
78	- postpone — real but deferrabl

People who liked this also viewed

6 picks

Type

review-implementation

35installs2forks

View on GitHub

How to install review-implementation?

gtrabanco/agentic-workflow/review-implementation

$npx -y skills add gtrabanco/agentic-workflow --skill review-implementation

Installs into the current project.

Summary

Files · 1

View on GitHub

SKILL.md

1	# Review Implementation (internal engine)
2
3	The findings engine the review/audit skills compose: it **produces findings and a
4	decision table, and stops — never refactors or edits code. It owns the review
5	axes (Phase 1) and the classification rubric** (Phase 2) that `review-change`,
6	`audit-pr`, and `product-audit` reference instead of restating.
7
8	## When to use
9
10	- Invoked by `review-change` (the user-facing review entry) as its engine; the
11	audit skills reference its rubric.
12	- Run directly only when you want the raw classified pass without the
13	platform-adaptive orchestration `review-change` adds.
14
15	## Scope
16
17	Default target is the current change (branch diff vs. the default branch);
18	accept an explicit path/glob to widen or narrow it. State the scope at the top of
19	the report so the reader knows what was and wasn't reviewed.
20
21	## Step 0 — Discover the project (always first)
22
23	Per the agent guide's Workflow conventions + documentation map, then read
24	what THIS skill needs: the architecture/layering rules, the testing philosophy,
25	and any runtime/platform, security, money, i18n/SEO/a11y and bundle rules. Pull
26	the project's specific risk axes from its guardrail skills where present
27	(architecture-pattern, runtime/platform, domain-rules). The axis list below is the
28	default; the project's docs refine it.
29
30	## Context budget (hard rule)
31
32	The scope is the diff. Beyond it, read at most 10 non-diff files in full
33	for surrounding context (callers, contracts, tests); targeted reads (≤ 50
34	lines of a named range) and grep/glob results don't count. Record each
35	finding as its table row immediately (id, `file:line`, axis, one-line
36	evidence) and drop the raw file content — Phase 2 classifies the table, never
37	the sources. Never quote whole files into the report.
38
39	## Phase 1 — Find (no refactor)
40
41	Assume the diff is WRONG — your job is to prove it does not work. Scan the
42	scope adversarially and record every finding across these axes. Fix nothing; the
43	classification in Phase 2 decides what matters.
44
45	\| # \| Axis \| Looks for \|
46	\|---\|---\|---\|
47	\| 1 \| Bug / correctness \| Logic errors, wrong edge-case handling, races, unhandled rejections, imprecise numeric handling \|
48	\| 2 \| Architecture violation \| Broken dependency direction, business logic in the wrong layer, abstraction bypass, cross-layer shortcut (per the architecture doc) \|
49	\| 3 \| Overengineering / premature optimization \| Unnecessary abstractions, single-caller indirection, speculative generality, micro-opt without a measured bottleneck \|
50	\| 4 \| Removable / dead code \| Unused exports, unreachable branches, commented-out blocks, obsolete files — see exception below \|
51	\| 5 \| Security / cybersecurity \| Secrets in code, injection, missing authz, unsafe deserialization, PII exposure, weak crypto, SSRF, over-broad CORS, leaking errors \|
52	\| 6 \| Platform / runtime incompatibility \| APIs unavailable on the target runtime, unsupported in-memory state assumptions, runtime-incompatible deps, blocking external calls in the request path \|
53	\| 7 \| Bundle-size risk \| Heavy/duplicate deps, accidental large imports, non-tree-shakeable patterns \|
54	\| 8 \| Tests — failing/weak \| Flaky/over-mocked/snapshot-heavy tests, tests asserting nothing meaningful \|
55	\| 9 \| Tests — missing \| Uncovered branches, new use-cases/adapters without tests, SPEC dev-scenario failure modes not exercised \|
56	\| 10 \| Project-rule violations \| Whatever the project's docs mandate (e.g. domain value-object rules, no hardcoded UI strings, don't hide user-facing limitations, naming conventions) \|
57
58	### Dead-code exception (important)
59
60	Do not flag code as removable if it is **intentionally staged for an
61	in-progress or planned feature**. Before reporting axis 4, cross-check the
62	roadmap, feature SPECs/`TASKS.md`, and `known-issues.md`: if the code is wired
63	into a planned phase or another feature, classify it intentional / in-progress,
64	not dead. When unsure, mark it verify and ask — never assert "dead" on a
65	guess.
66
67	### Finding format
68
69	Each finding: a stable id (`F-1`, `F-2`, …), `file:line`, axis, a one-line
70	description, and the evidence (the code/why it qualifies). No remedy code yet.
71
72	## Phase 2 — Classify (no refactor)
73
74	Turn findings into a decision table. Classify each into exactly one of:
75
76	- fix-now — correctness/security risk, blocks the merge, is cheap to fix,
77	or is in-scope of the unit under review (see the mandatory checks below).
78	- postpone — real but deferrabl

People who liked this also viewed

6 picks

Type