A community warehouse cooperative is preparing a bulk pickup-list export. The author has exercised an ordinary export, but the reviewer needs to answer four questions: what the local diff actually contains, whether the remote pull request still represents that revision, whether access has widened, and whether the implementation will remain maintainable. Rehearse the review with two fictional warehouses and synthetic pickup records. The final artifact is a set of findings that another developer can locate, reproduce and resolve.
Fix the review target and business contract
Record the repository, base branch, target commit, language and framework, then describe the behavior the change is meant to provide. In this example, an ordinary coordinator can export only their own warehouse. A platform maintainer has a separately approved cross-warehouse role. Existing scheduling software consumes the exported field contract. Confirm the role and field rules with the owner before turning them into assertions. Use synthetic accounts and records in isolation rather than attaching actual pickup lists.
Inspect staged, unstaged and untracked files and compare them with the requested scope. A staged-only review can omit a newly written test; treating the entire working tree as the pull request can include somebody else's experiment. Preserve a base and target for every review artifact. If the author commits during the review, identify which conclusions apply to the earlier revision and which portions need another pass. A finding without a reproducible version is difficult to evaluate later.
Ask the assistant to identify its reading plan and missing inputs. It needs the change description, diff, relevant callers, access and data contracts and test entry points. If remote access is unavailable, review the known local revision and mark the remote state as unverified. An inaccessible link must not become a claim that its contents were read. Separate covered paths, gaps and proposed checks so an absence of findings does not hide an absence of context.
Trace the local diff into behavior
Group changes by behavior. The export entry point handles request input and authorization, the query determines record membership, serialization defines the file contract, and a page control affects who sees the action. Follow each changed value to callers and consumers and inspect adjacent error handling and existing tests. A newly accepted warehouse parameter may interact with an older helper that substitutes it for session scope. Reading only added lines can miss that interpretation.
Prepare a row-set comparison. Warehouse A has two pending pickups and Warehouse B has one. A coordinator from A should receive only A's two records. Change the warehouse request parameter in the isolated test and observe whether account scope still constrains the result. The assistant can specify the expected check before an authorized maintainer executes it. Preserve request identifiers, roles, input and anonymous returned IDs. If the old version has the same flaw, state whether this change introduces, expands or exposes a pre-existing issue.
Review deletions and moves as carefully as additions. After extraction into a shared helper, verify how every caller interprets a missing warehouse, an empty result and a failure. Collapse formatting noise while reading if helpful, without losing genuine deletions. For generated files that must be committed, inspect their source and generation procedure. Ask for behavioral grouping when a huge mixed diff cannot be reviewed coherently and record unfinished groups rather than implying exhaustive coverage.
Produce bounded findings. Each should name a file and location, trigger, observed or inferred result, expected rule and consequence. A statement that a coordinator can change a request parameter and receive the other warehouse's rows can be assessed against code and a fixture. “Improve security” cannot. When execution did not happen, identify the source-based inference and proposed verification. Keep a concrete incorrect result distinct from a style preference and explain the priority through its effect.
Reconcile the remote pull request
Confirm the repository, target branch and head commit and compare them with the local record. Read the author's description of the trigger, resulting behavior and validation. Check whether linked discussion establishes a limitation or unresolved rule. A green pipeline represents the tasks configured for that commit; identify the tasks that ran and any that were skipped. A new head commit does not inherit proof from an older check run or finding automatically.
Locate each finding on the current diff. When code moves, inspect its callers again and decide whether the condition remains possible. Merge duplicate reports into one with an impact path and minimal reproduction. A resolved comment is not proof of a repair, while an outdated line anchor is not proof the bug remains. Replay a critical case against the present revision and update the report with that result.
Draft comments around conditions and consequences. For the access issue, state the role, parameter and affected records and propose preserving account scope. A plausible risk should remain labeled as such until supported. Maintainability advice should identify the next change it makes harder and distinguish required work from optional cleanup. Posting to GitHub is a separate external action: confirm that the task authorizes a comment on the particular pull request and inspect available repository permissions. A review-only assignment can finish with a local draft.
Before the final review, connect repaired findings to commits and regression output. Preserve counterexamples and withdraw conclusions that no longer hold. Name the owner of an unresolved business decision. Approval follows the project's merge procedure. An assistant's report, a comment draft and a passing check are distinct artifacts, none of which means the pull request has merged.
Follow security boundaries into consumers
Check authorization in the service. Hiding a button changes the interface but does not restrict an endpoint by itself. Compare unauthenticated, insufficient-role, same-warehouse and cross-warehouse inputs. Inspect whether a rejected request exposes the existence of another warehouse. Follow parameter binding and field selection through the project's established query mechanism. Keep synthetic identifiers and necessary fields in evidence rather than sending an entire real export to an external analysis service.
Check compatibility with consumers. A new column, null representation, date format, encoding or order can change the meaning of a file. Feed a new synthetic export through the existing consumer's parsing path and compare resulting pickup records. If the owner intends a contract change, document the migration version, transition and responsible consumer. A breaking change should not be hidden inside a formatting description. When the old consumer cannot run, mark the boundary uncovered and prepare an exact sample for its maintainer.
Exercise volume boundaries. Generate records across a pagination boundary and assert the complete row set without omission or duplication. Check deadlines, cancellation and repeated requests and inspect whether failed jobs have an explicit state. Concurrent writes can produce a file spanning multiple moments; establish whether the business needs a snapshot or accepts a stated consistency range before designing the test. A performance finding needs a scale and observation condition. A loop in a diff does not by itself demonstrate a production bottleneck.
Evaluate maintainability through future changes
Inspect whether role scope, selection and file formatting have understandable owners. If one function reads an account, constructs a query and writes a file, identify which error paths can be verified independently. Similar code is not necessarily the same business rule, so inspect callers before recommending consolidation. A shared helper also creates coupling. Explain the present maintenance problem and benefit instead of demanding abstractions for speculative future requirements.
Check whether tests detect the risk that motivated the review. A file-exists assertion misses cross-warehouse records, and a successful response misses an incompatible consumer. Require explicit row sets, fields and rejected roles, and demonstrate that an important regression fails against the faulty behavior. Use controllable clocks or documented terminal states for asynchronous work. Documentation should explain prerequisites and contracts, while comments preserve business reasoning that would otherwise be hard to recover.
Where the export button or results page changed, use Webapp Testing for the post-repair interface check. The verified listing describes Python and Playwright against a running local app, producing scripts, screenshots and logs. Supply startup instructions, synthetic accounts and expected controls. Inspect the rendered page before acting and prepare the browser dependencies and execution scope. This covers the chosen interface scenario. Service authorization, file compatibility and concurrency still require their own evidence.
Select capabilities by the review artifact
A local-diff method needs an identified base and target. A remote-PR method needs the current head and read access. A risk review needs business boundaries and negative cases, and a quality review needs callers and repository conventions. No suitable dedicated code-review entry was verified in the public SkillPI catalog for this guide, so the original source's nine names have not been replaced with guessed links. The browser skill above is restricted to interface regression. Existing inspection and test tools can perform the core review using the task card while the missing capability remains documented.
For discovery, first check whether SkillPI Skill Finder is installed. If absent, read the official repository instructions and enable it using the host's supported method. Ask for candidates matching the language, target, permissions and artifact, then inspect each actual package, revision, scripts and dependencies. Reject an inaccessible source, a collection introduction presented as a runnable skill or an environment mismatch with a concrete reason. As new entries become available, trial them on the same two-warehouse fixture before updating the relationship.
Run the complete rehearsal by preserving the base and head, checking local membership and authorization, and reconciling the remote revision and checks. After the author repairs a scope override, repeat the rejected cross-warehouse case, ordinary export and legacy consumer parse. Verify interface changes separately in the browser. Group results as demonstrated, pending verification and repaired, with locations, triggers, consequences and replay instructions. Another developer should be able to identify which conclusions support merge and which still need business or environment information.
Common questions
Is a diff enough for a complete review?
It supports an initial bounded pass. Access, compatibility and caller behavior often need surrounding context. List missing contracts and call sites and state the portion examined rather than treating a limited read as repository acceptance.
Can a pull request merge when no findings remain?
Check what was read, which tasks ran and whether all evidence refers to the present head. No findings can mean no issue was observed in that limited scope. The business contract, tests and project approval still govern merge.
Which findings should be handled first?
Verify concrete incorrect data, unauthorized access and broken consumers before optional style work. Explain each trigger and consequence and consolidate duplicates. Unsupported claims remain questions to test. Finding count is not a measure of review quality.
Will a skill edit code or post comments automatically?
That depends on its package, host tools and authorized task. Read actions and access before use. A review can produce only a local report. Recommendations, file changes, remote comments and merge are separate outcomes that should be checked separately.
What if there is no specialist for this language?
Keep the language, framework, test entry point and risk card and use existing search and execution tools for the verifiable portion. Compare a later candidate against the same fixture rather than linking an unverified name.