TimZander / TimZander/life-compass

Assistant import: gaps left open after the bounded review of #86

Open
#87 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
TypeScript
Stars
0
Forks
0
Avg merge
8h 34m
Merged PRs (30d)
3

Description

Findings from two independent passes over #86 that were deliberately not fixed there, under an agreed stopping rule of "merge unless it loses or corrupts a reader's answers". None of these meet that bar. Recorded so they are decided rather than rediscovered.

The review surface cannot tell repeated instances apart

Change is a per-field type reused for a collection. Three overwritten chapters produce three rows all labelled "Title", with no instance discriminator — and if two before values are blank the rows are byte-identical. ADR 0007 · C3's consent gate degrades exactly where a repeat makes it matter most: the reader is asked to approve three overwrites they cannot tell apart.

Needs a decision alongside the confirmation UI, since the fix is as much presentational as structural (an instance ordinal? the first field's value as a heading?).

Silent partial success is still reachable through a phantom fence

A line beginning with a fence run followed by backtick-free prose — ```json is what you write at the start of a line — opens a fence under CommonMark rules. A following real block is swallowed as its body, and a later one survives, so readBlocks returns ok with a block missing and nothing said. Verified: two valid blocks in, one out.

This is the silence ADR 0015 forbids, surviving the unterminated-fence fix because the fence counts balance. It needs an unusual line shape, and the scanner is CommonMark-correct as written, so there is no obviously right fix — which is why it is here rather than in the PR.

Nothing defends store.ts's transaction-completion await

Replacing Promise.all([run(store), landed]) with return run(store) in transact leaves all 522 tests green. The fake database in store.test.ts can never fire onabort, so the guarantee is prose-only.

A quota abort after every request has succeeded would resolve merge() / replaceAll() / claim() successfully and report an import that actually rolled back. Pre-existing and affecting all four store operations, not just the new one. The fix is in the fake: give it a way to abort after its requests succeed.

Three scanFences rules are load-bearing and untested

Mutations survived for !rest.includes(run[0]) (an info string containing the fence character is not an opener), rest.trim() === "" on a closing fence, and run[0] === fence[0] (a tilde does not close a backtick). All three are correct as written; the second silently truncates a block body if it ever regresses.

Smaller

  • ~~~info — a CommonMark-valid tilde fence with an info string — is not opened, because the no-fence-char-in-info rule is applied as a backtick rule. It then escalates to a whole-paste unterminated-fence refusal reading "that reply looks cut off".
  • Every refusal names its group by storage identifier (day1.chapters). Change.label exists precisely because falling back to the raw identifier shows the reader something the page never says; the same reasoning is not applied to the messages.
  • The "answer" branch of planFor writes to question.id while every other branch uses group. Identical values today.

Acceptance criteria

  1. The confirmation surface can distinguish two overwrites of different instances of the same repeat.
  2. A phantom fence either cannot swallow a block, or the reader is told the paste was only partly read.
  3. A transaction that aborts after its requests succeed fails a test.
  4. The three scanFences rules are pinned.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reading the referenced #86 and ADRs, then inspect the confirmation UI, readBlocks, scanFences, planFor, store.ts, and store.test.ts's fake database. Map each acceptance criterion to a focused change and test; done means repeated overwrites are distinguishable, partial reads are reported, post-success aborts fail, and the three scanFences rules are pinned.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
full-stack, testing
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
32/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.