Skip to content

Edits polish: residuals from PR #11's adversarial review #12

Closed

Description

@thorwhalen

Discovered from #5 / PR #11 (three review rounds). None corrupt silently in a plausible flow; all graded ride-along. Collected so they are not re-derived:

  • Optional span ids (additive SourceSpan.id) — the real cure for the one residual mis-identification case: a locked span whose start moves while a same-(source, role) sibling assumes its exact old start gets its value written onto the impostor. Content-anchored matching cannot beat identity-swapping content; ids can. Fits the Evidence layer: persist analysis into a lacing store #4 evidence-layer timeline (store-side identity adoption wants them anyway).
  • Merge after an id rename resurrects the old id: renaming a step severs the merge key, so a later regeneration re-introduces the old id alongside the renamed step (visible, not silent). Document, or map renames via the lock record (/id lock's was is exactly the old→new mapping).
  • Lock.at / Lock.by accept any string (empty by, junk timestamps) — validate.
  • Lock.was is an unconstrained wire channel: a float set via attrs lands as a float literal in an otherwise no-floats document. Same class as the pre-existing attrs/confidence holes; consider a document-wide float scan in validate_document.
  • Lock.path serialises snake_case into a camelCase wire document (input side accepts both; output is snake-only). A wire consumer resolving lock paths against the wire needs the camel spelling or a documented rule.
  • Renaming a step id onto an existing id is accepted and only flagged by validate_document — consider refusing at edit time.
  • Selection boost fires on fact presence, not usability: boundaries + N names + N matching chapters picks explicit (unnamed spans + mismatch flag) where chapters would have named everything. Honestly flagged today; revisit scoring when a fourth segmenter lands.
  • One-time "locks": [] re-serialisation churn on pre-0.0.3 documents (accepted).

Activity

  1. thorwhalen commented on Sep 8, 2026

    @thorwhalen
    CollaboratorAuthor

    Landed 4 of 7 in #13 (or the PR opened from branch fix/issue-12-edit-protection-residuals — see PR link below): Lock.by/Lock.at validation, document-wide float scan in validate_document, refuse renaming a step id onto an existing id, and merge_regenerated no longer resurrects the pre-rename id.

    Declining the remaining 3, each with a one-line reason (details in docs/07 §6.6/§7):

    • Optional span ids — the real cure for the identity-swap mis-identification case, but it's a schema change (new SourceSpan.id field on the wire); out of scope for this residuals pass. Best paired with Evidence layer: persist analysis into a lacing store #4's evidence layer, where store-side identity adoption wants ids anyway.
    • Lock.path camelCase vs snake_case — documented instead of changed: Lock.path stays snake_case (matching what apply_edits already writes to every existing document); a wire consumer converts each field segment with to_camel before indexing. Changing the recorded spelling would itself be an on-disk format change for every already-written lock — out of scope here.
    • Selection-boost scoring gap — already deferred by this issue's own text ("revisit scoring when a fourth segmenter lands"); no new information changes that call, so declining now rather than guessing at a fix with only two segmenters to generalize from.
  2. added a commit that references this issue on Sep 8, 2026
  3. thorwhalen commented on Sep 22, 2026

    @thorwhalen
    CollaboratorAuthor

    Closing: all 7 residuals from PR #11's review are accounted for — 4 fixed and merged in PR #22 (b310851), and the remaining 3 (optional span ids, Lock.path casing, selection-boost scoring) explicitly declined with reasons in the comment above. Nothing actionable remains here; optional span ids are noted as best paired with #4's evidence layer if picked up later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions