diff --git a/dev/guidelines/backend/python.md b/dev/guidelines/backend/python.md index e674b24525..476ab30704 100644 --- a/dev/guidelines/backend/python.md +++ b/dev/guidelines/backend/python.md @@ -267,6 +267,22 @@ async def set(self, key: str, value: str, expires: KVTTL | int | None = None) -> To branch on or read from a typed object, use `isinstance` so the type checker can narrow it; reaching for `getattr(obj, "attr", default)` defeats type analysis. When guarding a schema object, cover the whole family that carries the attribute — `isinstance(schema, (NodeSchema, ProfileSchema, TemplateSchema))` — since profiles and templates inherit node behavior and a `NodeSchema`-only check silently drops them. +### `str` satisfies `Sequence` — exclude it before narrowing + +When a parameter accepts `T | Sequence[T] | ...` and `isinstance(data, Sequence)` (or `Iterable`/`Collection`) is how the code tells "one item" from "many", check `str` first whenever `T` includes `str`. A bare string satisfies `Sequence` in its own right, so without the carve-out it falls into the "many" branch and gets iterated character-by-character instead of treated as a single item — silently, if the single-item branch also accepts `str`. + +```python +# ❌ Bad - a bare id like "abc-123" satisfies Sequence and gets shredded into one item per character +if not isinstance(data, Sequence): + data = [data] + +# ✅ Good - str is excluded first, so a single id stays a single item +if isinstance(data, str) or not isinstance(data, Sequence): + data = [data] +``` + +This carve-out is easy to lose exactly when it matters most: widening a parameter from an invariant `list[T]` to a covariant `Sequence[T]` (e.g. to drop a call-site `# type: ignore[arg-type]`, see [When a wrong-type bug slips through](#when-a-wrong-type-bug-slips-through)) means widening this runtime check in step — an annotation that newly accepts `str` as a `Sequence` while the `isinstance` check still assumes only `list` reaches it will misroute every bare string. Add a test for both the bare-`str` case and the newly-accepted non-`list` sequence (e.g. a `tuple`). + ### Deterministic serialization for hashes and cache keys When a JSON string feeds a hash, fingerprint, or cache key, its output must be deterministic. Do **not** pass `default=str` to `json.dumps` there: it silently serializes unexpected types via `str()`, which can embed run-specific data (memory addresses) and break determinism. Serialize an explicit, canonical shape (sorted keys, known field types) and let unknown types raise instead of being coerced. diff --git a/dev/guidelines/changelog.md b/dev/guidelines/changelog.md index e174049d4d..c9d25c5e44 100644 --- a/dev/guidelines/changelog.md +++ b/dev/guidelines/changelog.md @@ -50,6 +50,14 @@ uv run towncrier create -c "Added breadcrumb navigation for hierarchical schemas uv run towncrier create -c "Updated dependencies to latest versions" +deps-update.housekeeping.md ``` +## When to Skip + +Skip the fragment entirely when the change has **no user-facing effect** — an internal +type-annotation correction, a refactor with no behavior change, cleanup of internal docs or +spec-kit scaffolding. `housekeeping` is for internal changes a user could still plausibly notice +(a dependency bump, a tooling change) — it is not a catch-all for anything code-adjacent. If it's +unclear whether a change is user-facing, ask rather than defaulting to adding a fragment. + ## Writing Good Changelog Messages - Write from the user's perspective diff --git a/dev/guidelines/git-workflow.md b/dev/guidelines/git-workflow.md index 65056f532c..c1aba1d8ce 100644 --- a/dev/guidelines/git-workflow.md +++ b/dev/guidelines/git-workflow.md @@ -110,6 +110,13 @@ Add changelog fragments to `changelog/` using Towncrier. See [Changelog Guidelin - Any notable implementation details - Testing performed +**Scope to match the change:** for a trivial, code-only fix with no behavior change (e.g. a +type-annotation correction), ship just the code diff. Don't carry the spec-kit design record +(`dev/specs//`, see [Repository Organization](repository-organization.md)) or a +changelog fragment with no user-facing effect (see [When to Skip](changelog.md#when-to-skip)) +into the PR — trim them before opening it if the workflow that produced the change generated them +by default. + ## Critical Rules - Never force push to `stable` or `develop` diff --git a/dev/guidelines/repository-organization.md b/dev/guidelines/repository-organization.md index 43edf40b4e..7893c95a74 100644 --- a/dev/guidelines/repository-organization.md +++ b/dev/guidelines/repository-organization.md @@ -85,6 +85,12 @@ Each directory in `dev/` serves a specific purpose and follows a content lifecyc - Moved to `knowledge/` (if describing how something works) - Moved to `guidelines/` (if describing how to use something) +**Proportionality**: Not every change needs a spec. If the actual code change is small enough to +be self-explanatory (e.g. an annotation-only fix, a one-line correction with no behavior change), +skip the spec-kit scaffolding entirely or trim `dev/specs//` from the PR before it goes +to review — the design record should be proportional to the change it documents, not a fixed-cost +byproduct of running the workflow. See [Git Workflow → Pull Requests](git-workflow.md). + ### guidelines/ **Purpose**: "What rules should I follow?"