Skip to content

Preserve context edits when they exceed the size limit#138

Open
mvanhorn wants to merge 1 commit into
dhth:mainfrom
mvanhorn:fix/65-context-limit-recovery
Open

Preserve context edits when they exceed the size limit#138
mvanhorn wants to merge 1 commit into
dhth:mainfrom
mvanhorn:fix/65-context-limit-recovery

Conversation

@mvanhorn

@mvanhorn mvanhorn commented Jun 3, 2026

Copy link
Copy Markdown

Summary

When a task's context is edited in an external editor and the saved content
exceeds pers.ContextMaxBytes (1 MiB), the textEditorClosed handler in
internal/ui/update.go removed the temporary file before the size check, so
the over-limit branch discarded the user's edits with no way to recover them.

This moves the size check ahead of the file removal. On the over-limit path the
temp file is now kept on disk, and the error message names its location so the
user can recover their text, shorten it, and re-open the editor. The success
path still removes the temp file exactly as before.

Why this matters

Issue #65 reports that editing a large context silently throws the work away.
As the issue puts it, "the main thing I would like is for work to not be lost."
The existing // TODO: allow reopening the text editor with the same content again at the over-limit branch confirmed this was the intended direction. The
1 MiB cap is unchanged; this is purely about not losing work.

Testing

  • go build ./..., go vet ./..., gofmt -l . / gofumpt -l .: clean
  • go test ./...: all packages pass
  • Behavior:
    • Content under 1 MiB: saved normally, temp file removed (unchanged)
    • Content over 1 MiB: error names the preserved temp-file path, file remains on disk
    • Editor error / read error: existing paths preserved (temp file still removed on editor error)
    • Empty content with no prior context: still a no-op

Fixes #65

AI was used for assistance.

When an edited task context exceeds ContextMaxBytes (1 MiB), omm removed
the temporary file before reporting the error, discarding the user's
work. Move the size check before the file removal so the temp file is
kept on the over-limit path, and surface its location in the error
message so the user can recover and shorten the content.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 3, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d0c7ff8f-58b4-45a8-9860-a1481d5db2aa

📥 Commits

Reviewing files that changed from the base of the PR and between 6e18a7c and bdf35c2.

📒 Files selected for processing (1)
  • internal/ui/update.go

📝 Walkthrough

Walkthrough

The textEditorClosed message handler in internal/ui/update.go now validates that edited context content does not exceed pers.ContextMaxBytes before deleting the temporary editor file. If the content exceeds the limit, the handler sets a detailed error message (including the byte limit and temp file path) and preserves the file so the user can reopen and edit. If the content is within limits, the handler removes the temporary file and warns if removal fails.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely summarizes the main change: preserving user edits when they exceed the size limit.
Description check ✅ Passed The description is comprehensive and directly related to the changeset, explaining the problem, solution, and testing performed.
Linked Issues check ✅ Passed The pull request directly addresses issue #65 by preserving the temp file when content exceeds ContextMaxBytes and providing the file path in error messages, allowing users to recover their work.
Out of Scope Changes check ✅ Passed All changes in internal/ui/update.go are within scope and directly target the size-check logic and temp file handling as required by issue #65.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

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.

Warn/allow further editing if context exceeds limit

1 participant