Skip to content

Commit 2f1a76b

Browse files
authored
docs: clarify implementation and code review guidelines (#1721)
Define implementation boundaries and complete compatibility handling. Add review guidance that separates correctness defects from design notes.
1 parent f9370fb commit 2f1a76b

2 files changed

Lines changed: 62 additions & 25 deletions

File tree

‎AGENTS.md‎

Lines changed: 26 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -17,48 +17,49 @@
1717

1818
## Agent Skills
1919

20-
Use an available skill only when it matches the current task, and read its `SKILL.md` before applying it. Recommended
21-
skills are pinned in `skills-lock.json`; contributors can install them before a new Codex session with
22-
`make skills-install`.
20+
Use an available skill only when it matches the current task, and read its `SKILL.md` before applying it. Recommended skills are pinned in `skills-lock.json`; contributors can install them before a new Codex session with `make skills-install`.
2321

2422
## Coding Style & Naming Conventions
2523

2624
Target Python 3.11+. Use PEP 8 naming: modules and functions in `snake_case`, classes in `PascalCase`, constants in `UPPER_SNAKE_CASE`. Ruff enforces formatting and linting with a 120-character line length; run `uv run prek run -a` before committing if you do not use the installed hooks. Keep comments focused on non-obvious intent rather than restating code.
2725

26+
## Design and Implementation
27+
28+
- Start from the current contract and relevant RFC. Change public APIs only as required by the task and avoid unrelated refactors. Do not weaken behavior tests to accommodate an implementation shortcut.
29+
- Keep domain rules in the runtime or Artifact service that owns them. Transport and host adapters should translate their inputs and outputs without inventing different domain behavior.
30+
- Extract helpers for cohesive responsibilities, such as an invariant, algorithm, protocol encoding, or state transition. Share code when callers have the same contract and failure semantics. Similar syntax or a few repeated lines is not enough; avoid forwarding helpers and intermediate types that only scatter an operation across files.
31+
- Keep operation-specific validation, request construction, and error handling near their owning boundary. Do not add unrelated branches to a shared helper just to reuse it.
32+
- Make identity and state transitions explicit. Keep retries, persisted state, and recovery consistent with the promised idempotency and delivery behavior. A successful write must remain usable through its supported read path.
33+
- Choose compatibility deliberately. Unsupported formats may be rejected clearly before acceptance or writes; supported formats must work through serialization, persistence, readback, and reuse. Define the handling of existing records and version conflicts. Keep documentation and compatibility claims consistent with that choice.
34+
- Keep authoritative records distinct from derived projections. Rebuilding derived state must preserve exact identities and historical evidence rather than silently assigning today's interpretation to old data.
35+
- Account for work and storage costs at the expected scale. Identify whole-collection scans or rewrites on incremental operations, and measure their impact before claiming a performance improvement or acceptable cost.
36+
37+
Use [REVIEW.md](REVIEW.md) when reviewing changes.
38+
2839
## Generated API Contract
2940

30-
`openapi/powercontext.yaml` is the source of truth for the HTTP API contract. The Python sources in
31-
`src/powercontext/http/_generated/` are checked in; do not edit them by hand. After changing the contract, run
32-
`make api-generate` and `make contract-test`.
41+
`openapi/powercontext.yaml` is the source of truth for the HTTP API contract. The Python sources in `src/powercontext/http/_generated/` are checked in; do not edit them by hand. After changing the contract, run `make api-generate` and `make contract-test`.
3342

3443
## Testing Guidelines
3544

36-
Use `pytest`; name test files `test_*.py` and test functions `test_*`. Tests must protect observable behavior or a
37-
specific regression; complexity alone is not a reason to add one:
45+
Use `pytest`; name test files `test_*.py` and test functions `test_*`. Tests must protect observable behavior or a specific regression; complexity alone is not a reason to add one:
3846

3947
- Put focused behavior tests in `tests/` and assert through public interfaces.
4048
- Put cross-component acceptance scenarios in `tests/e2e/`; they should remain valid across internal rewrites.
41-
- Add a regression test when fixing a defect that is likely to recur. Reproduce the failure and preserve the
42-
externally observable behavior that was previously wrong.
43-
- Do not freeze implementation details such as import graphs, module ownership, dependency lists, buffer sizes,
44-
private call order, or call counts unless they express an external budget or idempotency guarantee.
45-
- Do not exhaustively mock and test internal errors that an abstraction deliberately normalizes. Cover an internal
46-
error separately only when it produces distinct observable behavior.
47-
- Do not test a straightforward script solely to increase coverage. If its correctness is evident from reading it
48-
end to end and it has no meaningful external behavior or known regression, extra tests add noise rather than
49-
confidence.
50-
51-
Include doctests when public examples are useful. For changes that affect supported Python versions, prefer `tox`
52-
before opening a PR. Tests may use plain `assert`; Ruff allows `S101` under `tests/`.
49+
- Add a regression test when fixing a defect that is likely to recur. Reproduce the failure and preserve the externally observable behavior that was previously wrong.
50+
- Do not freeze implementation details such as import graphs, module ownership, dependency lists, buffer sizes, private call order, or call counts unless they express an external budget or idempotency guarantee.
51+
- Do not exhaustively mock and test internal errors that an abstraction deliberately normalizes. Cover an internal error separately only when it produces distinct observable behavior.
52+
- Do not test a straightforward script solely to increase coverage. If its correctness is evident from reading it end to end and it has no meaningful external behavior or known regression, extra tests add noise rather than confidence.
53+
54+
Include doctests when public examples are useful. For changes that affect supported Python versions, prefer `tox` before opening a PR. Tests may use plain `assert`; Ruff allows `S101` under `tests/`.
55+
56+
Run focused checks first and expand based on affected behavior. For claims about an external backend or host, distinguish actual execution from mocks, emulators, and direct hook invocation. Simulated inputs verify local handling; they do not establish that a real service produces those inputs or that a host supports the complete workflow.
5357

5458
## Deliverables
5559

56-
Write page metadata and content as literal UTF-8 text. Use Chinese characters directly instead of Unicode escape
57-
sequences. Preserve non-ASCII text when serializing documentation metadata to JSON.
60+
Write page metadata and content as literal UTF-8 text. Use Chinese characters directly instead of Unicode escape sequences. Preserve non-ASCII text when serializing documentation metadata to JSON.
5861

59-
Write deliverables as self-contained, final-state artifacts. Incorporate requested feedback directly into the
60-
artifact. Do not refer to drafts, versions, review rounds, prior wording, superseded decisions, or the editing
61-
process unless the user explicitly requests a changelog, history, or decision record.
62+
Write deliverables as self-contained, final-state artifacts. Incorporate requested feedback directly into the artifact. Do not refer to drafts, versions, review rounds, prior wording, superseded decisions, or the editing process unless the user explicitly requests a changelog, history, or decision record.
6263

6364
## Commit & Pull Request Guidelines
6465

‎REVIEW.md‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
# Code review guidelines
2+
3+
Apply [AGENTS.md](AGENTS.md) and the affected contract. Focus on defects introduced by the change and decisions the author needs to make.
4+
5+
## Establish the behavior
6+
7+
- Read the issue or RFC, diff, and affected callers. Follow the public operation through its persistence or integration boundary; a changed helper alone rarely explains the outcome.
8+
- Record the reviewed commit. Check for updates and existing feedback before publishing; recheck findings affected by new commits rather than repeating resolved issues.
9+
- Support a finding with specific inputs or state, the expected behavior, and the actual result. Prefer a small public API reproduction. A clear code path can establish a defect; distinguish it from executed evidence and hypotheses.
10+
- Run relevant checks. Do not claim backend or host acceptance from a mock or a directly invoked script. Passing tests do not dismiss a demonstrated failure, and missing tests alone do not establish one.
11+
12+
## Separate defects from design choices
13+
14+
Request changes for demonstrated correctness failures, such as broken authorization, invalid persisted state, lost data, or a violated API or retry contract. State the required behavior without requiring a preferred implementation.
15+
16+
List alternative abstractions, versioning policies, and optimization strategies as `Design note (non-blocking)` unless they violate an established requirement. Explain the tradeoff. Quantify performance concerns and distinguish extra work from failure at a supported scale or a breached budget.
17+
18+
Legacy input may be rejected or supported. Neither policy is inherently a defect. Accepting input that the implementation then cannot read or reuse is a defect. Check the full chosen policy, including existing records and version conflicts; do not require backward compatibility by default.
19+
20+
## Check corresponding implementations
21+
22+
- When shared runtime or contract behavior changes, check the Python client, HTTP/MCP surfaces, storage backends, and host integrations that promise that behavior. Use [integrations/capabilities.toml](integrations/capabilities.toml) to establish host support; do not assume every surface exposes every feature.
23+
- When one public surface adds or changes an API, open its relevant counterparts and state whether an equivalent exists. If an expected equivalent is missing, identify the gap and suggest a follow-up issue.
24+
- For a bug fix, inspect the corresponding code path before asking for the same fix elsewhere. Name the affected path and concrete failure; do not infer a shared bug from similar code.
25+
- Respect intentional differences in backend and host support. Do not demand unsupported capabilities or identical implementations.
26+
- Do not raise parity notes for internal refactors, tests, or documentation changes with no shared public behavior.
27+
- Keep possible gaps brief and label them `Possible capability gap (non-blocking)`. A verified failure of an already supported contract is a correctness issue.
28+
29+
## Write useful comments
30+
31+
- Write concise English: failure, evidence, required outcome. Attach each finding to the smallest relevant changed range.
32+
- Use one thread per issue. Link equivalent affected paths instead of repeating the same comment on each implementation.
33+
- Put correctness findings first and label design notes separately. Avoid style preferences, generic checklists, praise, and speculative edge cases without a supported failure scenario.
34+
- Keep test pass counts and routine validation logs out of review comments. Include a failing example or material validation limit when it explains the finding.
35+
- Keep the review body short and use inline comments for details. Request changes when blocking findings remain; design notes alone do not justify that verdict.
36+
- Keep feedback local unless the user asks to publish it. When authorized, submit the review and inline comments against the reviewed commit, then verify their state and locations.

0 commit comments

Comments
 (0)