Reviewing Pull Requests for Infinito.Nexus

Development
1

This document is the repository-wide guide for reviewing pull requests.

Use it together with the PR-specific template:

  • The PR template is for the author.

  • This guide is for the reviewer.

  • Each template asks whether the change is primarily a feature or a fix.

  • Each template also documents whether there is a security impact.

  • CI should enforce what can be automated.

Review Goals

A good review should answer these questions:

  • Does the change solve the stated problem?

  • Is the diff focused, understandable, and consistent with repository conventions?

  • Are the tests and validation appropriate for the risk of the change?

  • Are documentation, migration, and operational implications covered?

  • Does the change avoid unnecessary complexity, duplication, or security regressions?

Reviewer Workflow

Review in this order:

  1. Read the PR summary, template type, change type, validation notes, security impact notes, and review focus.

  2. Check whether the diff matches the stated purpose and stays reasonably scoped.

  3. Verify that automated checks and required tests are present and green.

  4. Review the highest-risk paths first, such as auth, data handling, networking, migrations, and shared abstractions.

  5. Confirm that documentation and follow-up work items were updated where needed.

Repository-Wide Checklist

Correctness and Scope

  • The behavior described in the PR matches the implementation.

  • The change is focused and does not include unrelated churn.

  • Shared logic is reused instead of duplicated where practical.

  • Naming, variable placement, and repository conventions remain consistent.

Testing and Validation

  • Local validation is credible and appropriate for the change.

  • CI is green or failures are clearly unrelated.

  • Touched *.py code has suitable unit and integration coverage.

  • web-* changes have suitable Playwright coverage, including login and logout.

Documentation and Traceability

  • Relevant README.md files or docs were updated.

  • The linked work item and PR reference each other when required.

  • Screenshots, logs, traces, or migration notes are attached when they help validate the change.

Security and Operations

  • No obvious secrets, permission mistakes, or unsafe defaults were introduced.

  • Auth, proxy, TLS, and network-exposed changes were reviewed carefully.

  • Idempotency, rollback, and failure handling were considered where relevant.

  • Data migration, persistence, backup, and restore implications are understood.

Maintainability

  • The implementation follows DRY, SPOT, and KISS.

  • Comments and docs explain intent where the code alone is not obvious.

  • The change leaves the touched area clearer or at least not worse than before.

Review Hints by Template

Server (web-*)

Focus on:

  • login and logout behavior

  • proxy, TLS, mail, auth, and storage integration

  • upstream image choice and version pinning

  • screenshots and local deployment validation

Workstation (desk-*)

Focus on:

  • distro-specific package handling

  • application launch and basic user workflow

  • desktop integration and user-visible regressions

System

Focus on:

  • idempotency

  • cross-distro behavior

  • service, timer, and package-management side effects

  • shared abstractions, architecture implications, and migration side effects

Pipeline (ci/cd)

Focus on:

  • workflow triggers, permissions, concurrency, and fork safety

  • release, build, test, and artifact behavior

  • flakiness, observability, and failure diagnostics

Agents (AGENTS.md, CLAUDE.md, GEMINI.md)

Focus on:

  • consistency between repository-wide and agent-specific guidance

  • contradictions, ambiguity, and enforcement gaps

  • safety guardrails around permissions, destructive actions, and escalation

  • clarity for contributors and agents who must follow the instructions

Documentation

Focus on:

  • technical accuracy against the current implementation

  • clear wording, audience fit, and discoverability

  • stale commands, broken links, and misleading examples

  • whether cross-references and related guides were updated together

Security Impact (Cross-Cutting)

Focus on:

  • threat model and trust-boundary assumptions

  • secrets, auth, TLS, permissions, and exposed surfaces

  • safe defaults, compatibility impact, and rollback path

Review Outcomes

Approve when:

  • the change is correct enough to merge

  • remaining follow-ups are non-blocking and clearly identified

Request changes when:

  • behavior is incorrect or too risky

  • required tests or docs are missing

  • the scope is unclear, misleading, or too broad to review safely

Prefer explicit findings over vague discomfort.

When possible, explain the concrete risk, expected behavior, or missing proof.