Skip to content

Commit 087227a

Browse files
committed
Merge branch '001-consistent-inout'
* 001-consistent-inout: _format: foundational package + tests for format-preserving I/O (T001–T013) spec: design json-five-via-adapter format-preserving I/O (001-consistent-inout) research: empirical evidence for json-five-via-adapter (001-consistent-inout) [DATALAD RUNCMD] yolo '/speckit.specify Formatting preser... spec: format-preserving I/O for JSON and other BIDS file formats
2 parents 25270cb + d4ba093 commit 087227a

26 files changed

Lines changed: 4132 additions & 0 deletions
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
# Specification Quality Checklist: Format-Preserving Input/Output for BIDS Files
2+
3+
**Purpose**: Validate specification completeness and quality before proceeding to planning
4+
**Created**: 2026-04-21
5+
**Feature**: [spec.md](../spec.md)
6+
7+
## Content Quality
8+
9+
- [x] No implementation details (languages, frameworks, APIs)
10+
- [x] Focused on user value and business needs
11+
- [x] Written for non-technical stakeholders
12+
- [x] All mandatory sections completed
13+
14+
## Requirement Completeness
15+
16+
- [x] No [NEEDS CLARIFICATION] markers remain
17+
- [x] Requirements are testable and unambiguous
18+
- [x] Success criteria are measurable
19+
- [x] Success criteria are technology-agnostic (no implementation details)
20+
- [x] All acceptance scenarios are defined
21+
- [x] Edge cases are identified
22+
- [x] Scope is clearly bounded
23+
- [x] Dependencies and assumptions identified
24+
25+
## Feature Readiness
26+
27+
- [x] All functional requirements have clear acceptance criteria
28+
- [x] User scenarios cover primary flows
29+
- [x] Feature meets measurable outcomes defined in Success Criteria
30+
- [x] No implementation details leak into specification
31+
32+
## Notes
33+
34+
- Validation pass 1 (2026-04-21): all items pass. No [NEEDS CLARIFICATION] markers
35+
were introduced because every ambiguous aspect had a reasonable default
36+
(documented in the Assumptions section) — most notably the formatting used
37+
for brand-new files (FR-006), handling of mixed indentation (edge case),
38+
BOM policy (FR-008), and no-op write detection semantics (FR-007).
39+
- Items marked incomplete require spec updates before `/speckit.clarify` or `/speckit.plan`.
Lines changed: 310 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,310 @@
1+
# Internal I/O Contract — `bids_utils._format` and friends
2+
3+
bids-utils is a CLI/library, not a network service. The "interfaces" this
4+
feature exposes are (a) the internal Python API used by every other
5+
module that touches a file, and (b) the **observable behavioral contract**
6+
on the resulting on-disk bytes. Both are specified below as the
7+
acceptance surface for tests.
8+
9+
> **Status (FR-014)**: This contract is **internal**. No public API
10+
> guarantee is implied; semver covers only the documented public surface
11+
> in `bids_utils.__init__`. The contract here is the load-bearing
12+
> agreement between modules in this repo.
13+
14+
---
15+
16+
## Part A — Python API contract
17+
18+
### `bids_utils._format` (package — re-exports from sub-modules)
19+
20+
```python
21+
from collections.abc import MutableMapping
22+
from dataclasses import dataclass
23+
from pathlib import Path
24+
from typing import Any, Protocol
25+
26+
# ── Dataclasses (from _format/_profile.py) ────────────────────────────
27+
28+
@dataclass(frozen=True)
29+
class FormattingProfile:
30+
indent: str # "" | " " | " " | "\t" | ...
31+
separators: tuple[str, str] # (item_sep, key_sep)
32+
line_ending: str # "\n" | "\r\n" | "\r"
33+
trailing_newline: bool
34+
bom: bool
35+
36+
@dataclass(frozen=True)
37+
class TSVFormattingProfile:
38+
fieldnames: tuple[str, ...]
39+
line_ending: str
40+
trailing_newline: bool
41+
bom: bool
42+
original_lines: tuple[bytes, ...] # row bytes, header excluded
43+
quoting: int # csv.QUOTE_*
44+
45+
@dataclass(frozen=True)
46+
class TextFormattingProfile:
47+
line_ending: str
48+
trailing_newline: bool
49+
bom: bool
50+
51+
DEFAULT_JSON_PROFILE: FormattingProfile # FR-006
52+
DEFAULT_TSV_PROFILE: TSVFormattingProfile
53+
DEFAULT_TEXT_PROFILE: TextFormattingProfile
54+
55+
# ── Backend Protocol (from _format/_json_backend.py) ──────────────────
56+
57+
class JSONBackend(Protocol):
58+
def parse(self, source: bytes) -> Any: ...
59+
def serialize(self, model: Any, profile: FormattingProfile) -> bytes: ...
60+
def detect_profile(self, source: bytes) -> FormattingProfile: ...
61+
def get_keys(self, model: Any) -> list[str]: ...
62+
def get_value(self, model: Any, key: str) -> Any: ...
63+
def set_value(self, model: Any, key: str, value: Any) -> None: ...
64+
def append_key(self, model: Any, key: str, value: Any) -> None: ...
65+
def delete_key(self, model: Any, key: str) -> None: ...
66+
67+
# ── Format-preserving JSON document (from _format/_json.py) ───────────
68+
69+
class JSONDocument(MutableMapping[str, Any]):
70+
"""Format-preserving JSON document. Use like a dict.
71+
72+
- __getitem__ returns Python values (lists, plain dicts for nested
73+
objects, strings, numbers, bools, None).
74+
- __setitem__ append-if-new, replace-if-exists.
75+
- __delitem__ removes the key.
76+
- Nested object views are plain dicts; mutate-and-assign-back to
77+
propagate (see quickstart.md).
78+
"""
79+
profile: FormattingProfile
80+
81+
def to_dict(self) -> dict: ...
82+
83+
def parse_json(source: bytes) -> tuple[JSONDocument, FormattingProfile]: ...
84+
def serialize_json(
85+
data: JSONDocument | dict,
86+
profile: FormattingProfile,
87+
) -> bytes:
88+
"""Polymorphic on input type:
89+
- JSONDocument: byte-perfect round-trip via the backend.
90+
- dict: stdlib json.dumps parameterized by profile (FR-006 path).
91+
"""
92+
93+
# ── TSV (from _format/_tsv.py) ────────────────────────────────────────
94+
95+
def detect_tsv_profile(source: bytes) -> TSVFormattingProfile: ...
96+
def serialize_tsv(
97+
rows: list[dict[str, str]],
98+
profile: TSVFormattingProfile,
99+
changed_indices: set[int] | None = None,
100+
) -> bytes: ...
101+
102+
# ── Text (from _format/_text.py) ──────────────────────────────────────
103+
104+
def detect_text_profile(source: bytes) -> TextFormattingProfile: ...
105+
def serialize_text(text: str, profile: TextFormattingProfile) -> bytes: ...
106+
107+
# ── Write helper (from _format/_write.py) ─────────────────────────────
108+
109+
def write_if_changed(path: Path, new_bytes: bytes) -> bool: ...
110+
```
111+
112+
### `bids_utils._io` (additions)
113+
114+
```python
115+
from bids_utils._format import (
116+
FormattingProfile,
117+
JSONDocument,
118+
TextFormattingProfile,
119+
)
120+
121+
def read_json_with_profile(
122+
path: Path,
123+
vcs: VCSBackend | None,
124+
mode: AnnexedMode = AnnexedMode.ERROR,
125+
) -> tuple[JSONDocument | None, FormattingProfile | None]:
126+
"""Read JSON and return (document, profile).
127+
profile is None iff document is None.
128+
The returned JSONDocument carries `profile` on `.profile`; the
129+
tuple shape is symmetric with `read_tsv_with_profile`."""
130+
131+
def write_json(
132+
path: Path,
133+
data: JSONDocument | dict,
134+
vcs: VCSBackend,
135+
profile: FormattingProfile | None = None,
136+
) -> bool:
137+
"""Serialize *data* using *profile* (default if None) and write iff
138+
bytes differ from disk. Returns True iff a write occurred.
139+
140+
- If *data* is a JSONDocument: profile defaults to data.profile
141+
(the json-five-backed path). An explicit profile= overrides.
142+
- If *data* is a dict: profile defaults to DEFAULT_JSON_PROFILE
143+
(the stdlib path; FR-006 new-file behavior)."""
144+
145+
def read_text_with_profile(
146+
path: Path,
147+
vcs: VCSBackend | None,
148+
mode: AnnexedMode = AnnexedMode.ERROR,
149+
) -> tuple[str | None, TextFormattingProfile | None]: ...
150+
151+
def write_text(
152+
path: Path,
153+
text: str,
154+
vcs: VCSBackend,
155+
profile: TextFormattingProfile | None = None,
156+
) -> bool: ...
157+
```
158+
159+
### `bids_utils._tsv` (additions)
160+
161+
```python
162+
from bids_utils._format import TSVFormattingProfile
163+
164+
def read_tsv_with_profile(
165+
path: Path,
166+
vcs: VCSBackend | None = None,
167+
annexed_mode: AnnexedMode | None = None,
168+
) -> tuple[list[dict[str, str]], TSVFormattingProfile]: ...
169+
170+
def write_tsv(
171+
path: Path,
172+
rows: list[dict[str, str]],
173+
vcs: VCSBackend | None = None,
174+
profile: TSVFormattingProfile | None = None,
175+
changed_indices: set[int] | None = None,
176+
) -> bool: ...
177+
```
178+
179+
### Backward compatibility
180+
181+
The pre-existing signatures `read_json(path, vcs, mode) -> dict | None`
182+
and `write_json(path, data, vcs) -> None`, `read_tsv(path, vcs,
183+
annexed_mode) -> list[dict]`, `write_tsv(path, rows, vcs)` remain
184+
importable and behaviorally equivalent. Per Principle X:
185+
186+
- Legacy `read_json` internally calls `read_json_with_profile` and
187+
returns `doc.to_dict()` (dropping the profile). Callers that only
188+
read continue to work without changes.
189+
- Legacy `write_json(path, dict, vcs)` is the stdlib new-file path;
190+
it internally calls the profile-aware path with
191+
`profile=DEFAULT_JSON_PROFILE` and discards the returned bool.
192+
- TSV legacy signatures are analogous.
193+
194+
### Adapter contract (new)
195+
196+
The `JSONBackend` Protocol IS a load-bearing interface contract. Any
197+
future implementation must conform to its signatures AND pass
198+
`tests/test_format_json_backend.py`, which encodes:
199+
- the six R1.1 defenses (state-bug, mutation symmetry, etc.) as
200+
positive tests on the backend;
201+
- the round-trip identity expectation on the bids-examples corpus
202+
(delegated test, optionally skipped in fast CI).
203+
204+
A new backend module shipping alongside `_json_backend.py` (e.g.,
205+
`_json_backend_stdlib.py`) plus a one-line change to
206+
`_format/_json.py` that selects between them is the supported swap
207+
mechanism. No call site changes are required for a swap.
208+
209+
---
210+
211+
## Part B — Behavioral contract on output bytes
212+
213+
For every JSON, TSV, or plain-text file written by bids-utils, the
214+
following MUST hold. These are the test assertions for
215+
`tests/integration/test_format_preservation.py`.
216+
217+
### B.1 — Round-trip identity (FR-007 / SC-001)
218+
219+
For any file `f` read by bids-utils and written back without any
220+
operation modifying `data`:
221+
```
222+
read_bytes_before == read_bytes_after
223+
```
224+
i.e., the file's bytes on disk are unchanged. No write happens; VCS
225+
reports the file as clean.
226+
227+
### B.2 — Format preservation under semantic edit (FR-001..FR-004, FR-008..FR-010)
228+
229+
For any JSON file `f` with profile `P` whose data dict is modified by
230+
changing exactly one existing key's value:
231+
- `detect_json_profile(new_bytes) == P` (style preserved)
232+
- The unified diff against the source has ≤ 5 changed lines in 95%+ of
233+
bids-examples sidecars (SC-002).
234+
235+
For any TSV file `f` with profile `T` whose rows are modified by
236+
changing exactly one existing row's value:
237+
- `detect_tsv_profile(new_bytes).line_ending == T.line_ending`
238+
- `detect_tsv_profile(new_bytes).fieldnames == T.fieldnames`
239+
- The unified diff against the source has ≤ 3 changed lines in 100% of
240+
cases (SC-003), regardless of LF vs CRLF.
241+
242+
### B.3 — Adding a new key (FR-005)
243+
244+
For any JSON file with a top-level (or nested) object `O = {k1, k2, ..., kn}`,
245+
after `O[k_new] = v`:
246+
- `list(O.keys()) == [k1, k2, ..., kn, k_new]` in serialized output.
247+
- The diff is bounded to lines around the insertion point only; pre-existing
248+
keys are not reordered.
249+
250+
### B.4 — New file from scratch (FR-006)
251+
252+
For any file path that did not exist on disk before the write, the bytes
253+
match `serialize_*(data, DEFAULT_*_PROFILE)`. The default is defined in
254+
exactly one place (`_format.DEFAULT_*_PROFILE`).
255+
256+
### B.5 — Uniformity (FR-011)
257+
258+
There exists no code path in `src/bids_utils/` that writes a JSON sidecar
259+
or TSV file by means other than `_io.write_json` / `_tsv.write_tsv` (or
260+
equivalents that route through `_format`). Enforced by a meta-test that
261+
greps `src/bids_utils/` for `json.dumps(...)` and `csv.DictWriter(...)`
262+
calls outside `_format.py` / `_io.py` / `_tsv.py` and asserts none target
263+
on-disk files. (CLI stdout writes are exempt and explicitly out of
264+
scope.)
265+
266+
### B.6 — Correctness wins over preservation (FR-012)
267+
268+
If a preservation choice would alter semantic content (e.g., a TSV
269+
quoting style would render a cell ambiguous), the writer drops the
270+
preservation aspect and emits a `warnings.warn` with category
271+
`UserWarning`. The output is then valid even if non-byte-identical to
272+
what naive preservation would have produced.
273+
274+
### B.7 — Dry-run consistency (spec edge case)
275+
276+
For any operation invoked with `--dry-run`, the diff text rendered to the
277+
user is identical (line-for-line) to the diff a real run would have
278+
produced against disk. Implementation: dry-run synthesizes the would-be
279+
new bytes with the same `serialize_*` call and diffs against the read
280+
bytes.
281+
282+
### B.8 — Performance (SC-006)
283+
284+
The full `bids-examples` integration sweep runs in ≤ 1.20× the wall-clock
285+
time of a baseline measured on `main` immediately before this feature
286+
lands. Recorded as a benchmark in CI logs (informational, not a hard
287+
gate).
288+
289+
---
290+
291+
## Test surface mapping
292+
293+
| Contract clause | Test |
294+
|---|---|
295+
| A (signatures) | `tests/test_format.py::test_signatures_present` |
296+
| A (Protocol) | `tests/test_format_json_backend.py::test_default_backend_conforms_to_protocol` |
297+
| A (adapter defenses R1.1.1) | `tests/test_format_json_backend.py::test_serialize_is_stateless_across_calls` |
298+
| A (adapter defenses R1.1.2) | `tests/test_format_json_backend.py::test_set_value_routes_through_canonical_lists` |
299+
| A (adapter defenses R1.1.3) | `tests/test_format_json_backend.py::test_serialize_asserts_keys_values_length` |
300+
| A (adapter defenses R1.1.5) | `tests/test_format_json_backend.py::test_append_key_mirrors_whitespace` |
301+
| A (adapter defenses R1.1.6) | `tests/test_format_json_backend.py::test_parse_falls_back_to_stdlib_on_json5_failure` |
302+
| B.1 | `tests/integration/test_format_preservation.py::test_no_op_byte_identity` |
303+
| B.2 (JSON) | `tests/integration/test_format_preservation.py::test_one_field_edit_diff_size` |
304+
| B.2 (TSV) | `tests/integration/test_format_preservation.py::test_one_row_edit_diff_size` |
305+
| B.3 | `tests/test_format.py::test_new_key_appended_at_end` |
306+
| B.4 | `tests/test_format.py::test_default_profile_for_new_file` |
307+
| B.5 | `tests/test_format.py::test_no_bypass_writers_in_repo` (meta-test; also asserts only `_json_backend.py` imports `json5`) |
308+
| B.6 | `tests/test_format.py::test_correctness_wins_over_preservation` |
309+
| B.7 | `tests/test_dry_run.py::test_dry_run_diff_matches_real_run` (extension) |
310+
| B.8 | `tests/integration/test_format_preservation.py::test_perf_within_budget` |

0 commit comments

Comments
 (0)