Merge pull request #45 from deepseek-ai/worktree-docs+todo-markers
docs(development): document FIXME/TODO/XXX markers
This commit is contained in:
@@ -104,6 +104,7 @@ Dev/test/demo run **unbuilt** via tsx + the `paths` map in the root `tsconfig.js
|
||||
- **Explicit > implicit at package seams**: interface/vocabulary types spell out every field a consumer must supply — no optional field that the implementation silently fills with a hidden `?? default`. Put defaulting in the owning implementation as an explicit step (a `resolve(request): Spec` method that turns the optional-field request into the required-field spec), not smuggled inside `run()`/`start()`. Example: `dsh-bash` splits `BashExecRequest` (optional `workdir`/`timeoutMs`, model-facing) from `BashExecSpec` (required, what `run`/`start` act on); the tool layer calls `ctx.bash.resolve()` between them. The reader of a `BashExecSpec` never has to wonder where the working directory came from.
|
||||
- **An empty `catch` must name what it swallows and why nothing else can hit it**: a bare `catch {}` hides bugs. When you deliberately ignore a throw, the comment must (a) name the single expected failure, (b) say why ignoring it is correct — usually because the useful state was already captured *before* the `try` — and (c) make clear nothing else of consequence can reach the catch (ideally the `try` wraps a single statement). Example: the error-body `response.json()` parse in `dsh-llm-deepseek`'s adapter sets `code` + HTTP `status` from the status line before the `try`, so a malformed provider body can only cost a richer message, never the real error.
|
||||
- **Symmetry is usually more correct**: when two related values play parallel roles (a test fixture and its expected output, a request shape and its response shape, a buggy input and the test that checks the fix), give them parallel form — both named consts, or both inline, not one each way. Asymmetry is a smell that usually points at a missed extraction.
|
||||
- **TODO markers**: use `FIXME`/`TODO`/`XXX` to flag known issues by urgency — see [docs/development.md](docs/development.md) for the semantics of each.
|
||||
- **Tests**: vitest, colocated under `packages/<name>/tests/*.spec.ts`. Every registry needs an HMR-safety test (dispose the contributing fiber, assert cleanup). **Excessive tests are welcome** — when in doubt, write the test; err on the side of covering edge cases, error paths, event ordering, and concurrency races even if they seem unlikely. Review findings get regression tests (see `packages/agent-loop/tests/review-fixes.spec.ts`).
|
||||
|
||||
## Defensive patterns (hard-won)
|
||||
|
||||
@@ -116,6 +116,16 @@ The coding-agent demo uses the real DeepSeek adapter and needs `DEEPSEEK_API_KEY
|
||||
pnpm run demo:coding
|
||||
```
|
||||
|
||||
## TODO markers
|
||||
|
||||
Use one of three comment tags to flag known issues in the code, ordered by urgency:
|
||||
|
||||
- `FIXME` — an issue that should block a new release. A release should not ship with an open `FIXME` unless reviewers explicitly agree the change can be merged anyway.
|
||||
- `TODO` — an issue that should be fixed soon, once we have the resources.
|
||||
- `XXX` — an issue that we may fix someday; lowest priority, no commitment.
|
||||
|
||||
Pick the tag that matches the urgency so anyone scanning the code can tell a release blocker from a someday-maybe.
|
||||
|
||||
## Architecture context
|
||||
|
||||
Read `docs/architecture.md` before changing anything under `packages/`. The codebase is built around Cordis plugins, event-sourced sessions, typed service seams, and explicit extension points.
|
||||
|
||||
Reference in New Issue
Block a user