Skip to content

feat: add highlight toggle workflow#14

Open
nonnil wants to merge 5 commits intomainfrom
symphony/tra-8-highlight-system
Open

feat: add highlight toggle workflow#14
nonnil wants to merge 5 commits intomainfrom
symphony/tra-8-highlight-system

Conversation

@nonnil
Copy link
Copy Markdown
Member

@nonnil nonnil commented May 10, 2026

Summary

Implements TRA-8 highlight workflow:

  • reader text selection captures text, line-local prefix/suffix context, and offsets, then applies optimistic mark/unmark UI with rollback on persistence failure
  • POST /api/highlights validates the selection payload and persists highlight toggles through SQLite plus CONTENT.md
  • server highlight range logic handles exact removal, partial shrink/split, multi-highlight overlap removal, and merged creation without overlapping canonical rows
  • /highlights now loads repository-backed highlight rows with source memory titles and links title/quote rows to /memories/:id#<highlight-id>
  • reader sanitizer preserves persisted <mark data-highlight-id> and emits matching id anchors

Selection anchoring strategy

The frontend sends selected text, prefix, suffix, and offsets from the reader DOM. The server resolves against CONTENT.md after stripping persisted highlight marks: offsets are preferred when they match, and text/context disambiguates repeated selections when rendered-reader offsets differ from raw markdown.

Toggle and mark behavior

  • Unhighlighted selection creates a canonical range and writes <mark data-highlight-id="...">selected text</mark> into CONTENT.md.
  • Exact highlighted selection removes that row/range.
  • Subset selection shrinks or splits the existing range, preserving unselected highlighted text on both sides.
  • Multi-highlight selection removes only selected overlaps from each affected range.
  • Markdown marks are rebuilt from canonical SQLite ranges, so stale persisted highlight tags are removed while plain author <mark> tags remain for the sanitizer boundary.

Browse and search behavior

/highlights rows show source memory title, muted prefix, highlighted text, and muted suffix. /memories?q=... continues to include highlight text/prefix/suffix through existing browse filtering, now backed by repository highlight data.

Failure and backup behavior

The reader restores the previous HTML and shows Highlight failed if persistence fails. The server writes markdown, replaces highlight rows transactionally, attempts content restore on DB failure, and enqueues backup after the markdown write through the existing Task 8 boundary.

Mark insertion example

Beta <mark data-highlight-id="highlight-created">target</mark> appears in the detail paragraph.

Validation

  • npm run typecheck -> pass
  • npm test -> pass, 20 files / 110 tests; non-fatal mise tracked-config symlink warnings due sandbox Operation not permitted
  • npm run build -> pass
  • GIT_DIR=.tmp/gitdir GIT_WORK_TREE=$PWD git diff --check -> pass
  • PATH="$HOME/.local/share/mise/installs/bun/1.3.13/bin:$PATH" npm run test:e2e -> blocked before app code by local Chromium/macOS sandbox: bootstrap_check_in org.chromium.Chromium.MachPortRendezvousServer... Permission denied (1100); 11/11 tests fail at browser launch

Note: bun is not on the default PATH in this workspace, so npm-equivalent commands were used locally; the pushed branch's pre-push hook reran typecheck, tests, and build successfully.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 10, 2026

Warning

Rate limit exceeded

@nonnil has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 7 minutes and 51 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c195c5c7-d5d5-422b-8679-c48d0007764e

📥 Commits

Reviewing files that changed from the base of the PR and between 9c09e07 and 8405a6e.

📒 Files selected for processing (21)
  • e2e/reader.spec.ts
  • src/components/highlights/highlights-loader.ts
  • src/components/reader/MemoryReader.tsx
  • src/routes/api/highlights.ts
  • src/routes/highlights/index.tsx
  • src/server/db/index.ts
  • src/server/db/repositories.ts
  • src/server/highlights/browse.ts
  • src/server/highlights/index.ts
  • src/server/highlights/ranges.ts
  • src/server/highlights/toggle.ts
  • src/server/reader/markdown-renderer.ts
  • src/server/store/highlight-markers.ts
  • src/server/store/index.ts
  • tests/server/highlights/highlight-markers.test.ts
  • tests/server/highlights/ranges.test.ts
  • tests/server/highlights/repository.test.ts
  • tests/server/highlights/toggle.test.ts
  • tests/server/reader/markdown-renderer.test.ts
  • tests/server/reader/page-data.test.ts
  • tests/server/routes/api-highlights.test.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch symphony/tra-8-highlight-system

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 208775767c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/components/reader/MemoryReader.tsx
Comment thread src/server/store/highlight-markers.ts
Comment thread src/components/reader/MemoryReader.tsx
Comment thread src/components/reader/MemoryReader.tsx Outdated
Comment thread src/server/highlights/toggle.ts
Comment thread src/components/reader/MemoryReader.tsx
Comment thread src/server/store/highlight-markers.ts Outdated
Comment thread src/routes/api/highlights.ts
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8405a6eb92

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +91 to +97
const projectedCandidates = findTextCandidates(
projectedMarkdown.text,
selection.text,
)
.map((startOffset) => ({
resolved: mapProjectedRange(projectedMarkdown, startOffset, selection),
score: scoreCandidate(projectedMarkdown.text, selection, startOffset),
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject protected selections before trying duplicates

When the user selects text inside rendered code and the same text appears later outside code, this fallback drops the protected candidate (mapProjectedRange returns undefined) and then resolves the selection to the unprotected duplicate instead of rejecting it. For example, in `target` target selecting the first rendered target sends rendered offsets for the code span; the protected match is filtered out here, and the plain target is highlighted, so the saved highlight appears on text the user did not select. Fresh evidence in this version is that protected ranges are filtered during projected candidate resolution, creating a wrong-location highlight rather than the earlier raw-code insertion failure.

Useful? React with 👍 / 👎.

Comment on lines +234 to +237
const sourceEndOffset = lastSourceOffset + 1;
if (sourceEndOffset - sourceStartOffset !== selection.text.length) {
return undefined;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve selections that span markdown syntax

Selections that cross any skipped markdown syntax are currently unresolvable because the projected rendered text maps to non-contiguous source offsets and this check rejects the range. For example, selecting a bold b in rendered a **bold** b, or selecting text that starts before a markdown link and continues after it, hits this branch and then the raw-markdown fallback cannot find the rendered phrase contiguously, so normal reader selections fail and roll back even though the text is visible and selectable.

Useful? React with 👍 / 👎.

class={readerArticle}
data-reader-content
innerHTML={props.result.rendered.html}
onKeyUp={handleSelectionToggle}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Do not toggle highlights on ordinary key releases

Because every keyup inside the reader calls the same toggle handler, a user who selects text just to copy it and releases Ctrl+C/Cmd+C will create a highlight, and doing the same over highlighted text can remove it. This only needs a current selection inside [data-reader-content]; the handler does not check the key or modifier state, so normal keyboard actions mutate persisted reader content unexpectedly.

Useful? React with 👍 / 👎.

Comment on lines +521 to +522
text.push(markdown[cursor] ?? "");
sourceOffsets.push(cursor);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Decode rendered entities when resolving selections

The projected text is built from raw markdown characters, but the reader is rendered through MarkdownIt, so HTML entities are decoded before the browser selection is captured. If content contains visible text like Tom &amp; Jerry, selecting Tom & Jerry cannot be resolved because this projection still contains &amp;, and the raw fallback also searches for the decoded text in the encoded markdown; highlights fail for selectable text containing common entities.

Useful? React with 👍 / 👎.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant