{"metabox":"1","type":"annotation","subject":"src/annotation.rs","issuer":"mailto:claude-review@anthropic.com","issuer_type":"ai","created_at":"2026-05-06T21:04:22.022567Z","id":"b4a15cbd6ff7f606479d06ce62805cf91f55b141a8de22b9fa3d9d17d2b9a6ca","body":{"detail":"serde_json serializes struct fields in declaration order. AnnotationBody fields are declared alphabetically (detail, kind, ref, references, span, suggested_fix, summary, supersedes, tags), and that ordering IS the Metabox canonical body form per SPEC §canonical-form. The doc comment on line 244 says 'Field order is alphabetical (MCF canonical form)' but nothing prevents a future contributor from grouping fields semantically (e.g., putting 'kind' first). Such a reorder would silently change every newly-minted record's id and invalidate every existing record's id. EpochBody and DependencyBody have the same fragility.","kind":"suggestion","span":{"start":{"line":243},"end":{"line":262},"content_hash":"8e674cc061528ba83df5271a15242c8d42009b368805a8a09e6907a599f982c9"},"suggested_fix":"Add a unit test that pins the canonical id for a fixed AnnotationBody/EpochBody/DependencyBody triple (with all fields populated) — analogous to the dependency golden id in tests/integration.rs but covering each body type. Any field reorder will then break the test loudly. Alternative: serialize bodies via a BTreeMap or a hand-written Serialize impl that doesn't rely on declaration order, decoupling the id from struct layout.","summary":"AnnotationBody field declaration order silently determines MCF canonical IDs — make the invariant load-bearing in code or in a doc-test","tags":["review","invariant","canonical-form"]}}
{"metabox":"1","type":"annotation","subject":"src/qual_file.rs","issuer":"mailto:claude-review@anthropic.com","issuer_type":"ai","created_at":"2026-05-06T21:04:52.981430Z","id":"51596621d397cf8d6f0ec960a74945b637a03a419daae0212de4fc6d94ed8ccb","body":{"detail":"subject_name() uses to_string_lossy() and then checks 'stripped.ends_with('/') || stripped.ends_with(MAIN_SEPARATOR)'. On Windows, MAIN_SEPARATOR is '\\\\', so a path like 'src\\\\.qual' yields a subject of 'src\\\\' rather than 'src/'. Tests at qual_file.rs:332 and the recommended directory layout in agents/concepts.md ('src/.qual -> src/') treat the trailing '/' as canonical. A subject keyed by 'src\\\\' would not match any annotation lookup keyed by 'src/' (and vice versa). This is a portability defect, since other lookups (find_annotations_for, by_subject in ls.rs) compare subjects by exact string equality.","kind":"concern","span":{"start":{"line":228},"end":{"line":245},"content_hash":"6d25802318273ee4d04ce1b55087ac6b3ebd0d2a1dbd0240ebaf82a8e2500691"},"suggested_fix":"Normalize the separator before comparing/forming the subject — replace backslashes with forward slashes (via .replace('\\\\', \"/\") on the lossy string, or by walking Path components and rejoining with '/'). Apply consistently in subject_name(), resolve_qual_path(), and find_qual_file_for(). Add a Windows-targeted test that exercises 'src\\\\.qual' directly.","summary":"subject_name() yields backslash-suffixed subjects on Windows, breaking the trailing-'/' convention","tags":["review","portability"]}}