fix: prevent watcher ingestion starvation - #613
Conversation
Precompute live-parent evidence during offset pruning and checkpoint oversized unread JSONL suffixes so one rollout cannot monopolize the serial watcher. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_295f5c7c-b1ee-4b61-8e0d-2e6d4991748e) |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
@codex review |
|
You need to increase your spend limit or enable usage-based billing to run background agents. Go to Cursor |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
📝 WalkthroughWalkthroughThe watcher now validates a configurable pending-byte limit, skips oversized JSONL data with immediate checkpoints, handles rewinds, and resumes later appends. Offset pruning uses precomputed live parent directories, with tests covering performance, checkpoint boundaries, and size-limit behavior. ChangesWatcher behavior
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: abb9f39ae3
ℹ️ 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".
| offset = tailer.offset if tailer else self.registry.get(filepath)[0] | ||
| pending_bytes = max(file_stat.st_size - offset, 0) |
There was a problem hiding this comment.
When a watched path has a stored offset from a previous inode, this uses the stale offset to decide whether the new file exceeds BRAINLAYER_WATCH_MAX_FILE_BYTES. If the file was replaced or rotated with a large file whose size is still within max_file_bytes of the old offset (or below the old offset), the guard returns false, then _ensure_tailer() detects the inode mismatch and resets to offset 0, so read_new_lines() can read the entire oversized file and reintroduce the starvation this cap is meant to prevent.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/brainlayer/watcher.py`:
- Around line 1129-1165: Update the poll_once reconciliation flow to remove
paths from _oversized_files that are no longer among the currently discovered,
eligible files, including denylisted or disappeared paths. Preserve entries for
files still being watched and retain _skip_oversized_file’s existing discard
behavior when files fall back under the limit.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 817baf17-d3dc-4ff5-8342-ac6a84c94ca4
📒 Files selected for processing (2)
src/brainlayer/watcher.pytests/test_jsonl_watcher.py
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: test (3.13)
- GitHub Check: test (3.12)
- GitHub Check: test (3.11)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Flag risky DB or concurrency changes explicitly and do not hand-wave lock behavior
Enforce one-write-at-a-time concurrency constraint; reads are safe but brain_digest is write-heavy and must not run in parallel with other MCP work
Run pytest before claiming behavior changed safely; current test suite has 929 tests
Files:
src/brainlayer/watcher.pytests/test_jsonl_watcher.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: Usepaths.py:get_db_path()for all database path resolution; do not hard-code the BrainLayer database path.
Each worker must use its own database connection and retry onSQLITE_BUSY.
For bulk database operations, stop enrichment workers, checkpoint WAL before and after, drop and recreate FTS triggers around large deletes, batch deletes in 5–10K chunks, and checkpoint every three batches.
Use the canonical BrainLayer naming:BrainLayer (זיכרון)means “memory.”
Files:
src/brainlayer/watcher.py
src/brainlayer/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/brainlayer/**/*.py: Preserveai_code,stack_trace, anduser_messageverbatim; skip noise, summarize build logs, and retain only structure for directory listings.
Use AST-aware tree-sitter chunking, never split stack traces, and mask large tool output.
Keep Groq as the primary enrichment backend, Gemini as fallback, and Ollama as the offline last resort; honorBRAINLAYER_ENRICH_BACKENDandBRAINLAYER_ENRICH_RATE.
Default searches must exclude lifecycle-managed chunks;include_archived=Truemay expose history.brain_supersedemust apply its personal-data safety gate, andbrain_archivemust soft-delete with a timestamp.
Files:
src/brainlayer/watcher.py
src/brainlayer/watcher*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/brainlayer/watcher*.py: The JSONL watcher must apply filters in order: entry-type whitelist, classification, minimum chunk length, and system-reminder stripping.
Persist watcher offsets, detect file rewinds, and soft-archive chunks reverted by checkpoint restoration.
Files:
src/brainlayer/watcher.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Tests must not refresh the production backup heartbeat log; use
BRAINLAYER_BACKUP_LOG_PATHand set provenance topytest.
Files:
tests/test_jsonl_watcher.py
🪛 ast-grep (0.44.1)
tests/test_jsonl_watcher.py
[info] 1212-1212: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"role": "user", "content": "x" * 256})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 1213-1213: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"role": "user", "content": "healthy"})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 1248-1248: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"role": "user", "content": "x" * 256})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 1299-1299: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"role": "user", "content": "x" * 256})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 1316-1316: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"role": "user", "content": "small append"})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 1325-1325: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"role": "user", "content": "x" * 256})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🔇 Additional comments (11)
src/brainlayer/watcher.py (5)
27-27: LGTM!
48-73: LGTM!
882-885: LGTM!
1196-1197: LGTM!
529-537: 🎯 Functional CorrectnessKeep the current evidence scope.
_has_live_parent_evidencestill uses the same ancestor-based match as before; the set rewrite did not broaden pruning.> Likely an incorrect or invalid review comment.tests/test_jsonl_watcher.py (6)
139-163: LGTM!
1203-1244: LGTM!
1245-1265: LGTM!
1266-1295: LGTM!
1296-1321: LGTM!
1322-1340: LGTM!
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_3da89539-625e-43d8-9e9a-4cc54d20f6d4) |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
You need to increase your spend limit or enable usage-based billing to run background agents. Go to Cursor |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf7e85e074
ℹ️ 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".
| offset = tailer.offset if tailer else registry_offset | ||
| if registry_inode != 0 and registry_inode != file_stat.st_ino: | ||
| offset = 0 | ||
| pending_bytes = max(file_stat.st_size - offset, 0) |
There was a problem hiding this comment.
When a tracked JSONL is truncated/restored in place without changing inode and the old tailer offset is greater than the new file size, this uses the stale pre-rewind offset, so pending_bytes becomes 0 and the oversized check is bypassed. read_new_lines() then detects the rewind and reads from byte 0 without reapplying BRAINLAYER_WATCH_MAX_FILE_BYTES, so a restored file larger than the cap can still monopolize the watcher.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 794d5ce. _skip_oversized_file now treats size < tailer offset as a byte-zero rewind, and calls mark_rewind before persisting the smaller EOF so the registry generation safely permits the lower same-inode checkpoint. Added a RED→GREEN same-inode truncation test.
| return False | ||
|
|
||
| self._tailers.pop(filepath, None) | ||
| self.registry.set(filepath, file_stat.st_size, file_stat.st_ino) |
There was a problem hiding this comment.
If a previous poll read entries into BatchIndexer but they have not been confirmed yet due to batch_size, flush_interval_ms, or a flush failure, a later oversized append reaches this line and persists the registry at EOF. If the watcher exits before those buffered entries are successfully flushed, restart resumes at EOF and never replays them; previously the registry advanced only through confirmed watermarks. Please flush/confirm pending entries or avoid advancing past unconfirmed offsets before writing the oversized checkpoint.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 794d5ce. Before an oversized EOF checkpoint, the watcher now force-flushes buffered entries from that source and defers checkpointing while any remain retained/unconfirmed. Added a RED→GREEN flush-failure test proving the registry stays at its prior offset and the buffered item remains available. Watcher suite is 92 passed; full mandatory pre-push gate passed.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_cdb6cd80-6164-49ee-9a9d-7cbde89151a0) |
|
You need to increase your spend limit or enable usage-based billing to run background agents. Go to Cursor |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 704f598f20
ℹ️ 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".
| return False | ||
|
|
||
| self._tailers.pop(filepath, None) | ||
| self.registry.set(filepath, file_stat.st_size, file_stat.st_ino) |
There was a problem hiding this comment.
When a tailer has read past the last confirmed registry offset (for example after a partial flush or retained flush failure), this checkpoints the registry directly to EOF for an oversized append. That advances durable progress past bytes that were only parsed in memory and not confirmed by _advance_confirmed_offsets; if the watcher restarts before those entries become durable, it resumes at file_stat.st_size and never retries the unconfirmed transcript lines.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 794d5ce by the same pending-entry guard: if this source has buffered entries, the watcher force-flushes them and rechecks the buffer; while any remain retained after a full or partial flush failure, it returns without advancing the registry. The RED→GREEN regression asserts the prior registry offset and retained item are preserved. Successful production flushes return confirmed watermarks through _advance_confirmed_offsets before the EOF checkpoint.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_02be072b-f8e2-4fad-be02-ed30a8653109) |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
You need to increase your spend limit or enable usage-based billing to run background agents. Go to Cursor |
|
You need to increase your spend limit or enable usage-based billing to run background agents. Go to Cursor |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2001b07408
ℹ️ 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".
|
|
||
| tailer = self._tailers.get(filepath) | ||
| registry_offset, registry_inode = self.registry.get(filepath) | ||
| tailer_offset = tailer.offset if tailer else registry_offset |
There was a problem hiding this comment.
When max_lines_per_file limits a read, JSONLTailer.read_new_lines() can leave complete, unflushed records in tailer._buffer while tailer.offset only reflects the emitted lines. If that file then grows past BRAINLAYER_WATCH_MAX_FILE_BYTES, this uses only tailer.offset, so tailer_offset == registry_offset and the oversized path below can pop the tailer and checkpoint to file_stat.st_size, silently dropping buffered records that were never normalized or flushed. This affects hot transcripts that cross the byte cap between polls; the skip decision needs to account for buffered/unemitted bytes before advancing the registry to EOF.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7890d18. The watcher now separates parsing already-buffered complete records from reading additional file bytes. When a tailer has buffered records, poll_once drains them under max_lines_per_file without calling f.read(), while preserving inode-replacement and rewind handling. RED reproduced the EOF checkpoint/data loss; GREEN: 94 watcher tests and the full pre-push gate (3608 passed, 9 skipped, 1 xfailed).
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/brainlayer/watcher.py (1)
1172-1237: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftOversized checkpointing can be silently reverted to a stale offset when a file is replaced or rewinds while unconfirmed data is buffered.
The "don't checkpoint past unconfirmed entries" guard at line 1194 only applies when
not inode_changed and not file_rewound. When the file is instead replaced (new inode) or rewound while the indexer still has unconfirmed buffered entries for it (tailer_offset > registry_offset, e.g. batch not yet full or a prior flush failed), execution falls straight through to line 1212 (self._tailers.pop(...)) and the unconditionalself.registry.set(filepath, file_stat.st_size, file_stat.st_ino)at line 1220 — without flushing or checkingself.indexer.has_buffered_source(filepath)first.When those stale, pre-rewind/pre-replace buffered entries eventually flush (same poll's
indexer.tick()or a later poll),_advance_confirmed_offsetspairs their old (now invalid)_line_end_offsetwith whatever inode is current at that time and applies it becauseoffset >= current_offset. Since the stale offset is very likely larger than the just-written oversized checkpoint, this clobbers the correct checkpoint with an invalid offset/inode pairing, which can desync the tailer (seek past EOF, or never observe new appends) and stall ingestion for that file — the exact "ingestion starvation" class this PR targets. None of the new tests (test_poll_caps_oversized_replacement_from_start,test_poll_caps_oversized_same_inode_rewind_from_start) exercise this because both start from a fully-confirmed tailer (tailer_offset == registry_offset), so the gap is untested.As per coding guidelines, "Flag risky DB or concurrency changes explicitly and do not hand-wave lock behavior" — this checkpoint-write path needs the same unconfirmed-buffer protection regardless of which branch (growing / replaced / rewound) triggered the oversized skip.
🔧 Suggested direction: gate the checkpoint write on buffered-source state, not on inode/rewind branch
- if tailer is not None and not inode_changed and not file_rewound and tailer_offset > registry_offset: - if self.indexer.has_buffered_source(filepath): - self.indexer.flush() - confirmed_offset, confirmed_inode = self.registry.get(filepath) - if confirmed_inode != file_stat.st_ino or confirmed_offset < tailer_offset: - if filepath not in self._oversized_files: - logger.error( - "Oversized JSONL checkpoint deferred for unconfirmed entries: %s", - filepath, - ) - self._oversized_files.add(filepath) - return True - offset = confirmed_offset - pending_bytes = max(file_stat.st_size - offset, 0) - if pending_bytes <= self.max_file_bytes: - self._oversized_files.discard(filepath) - return False + if tailer is not None and tailer_offset > registry_offset: + if self.indexer.has_buffered_source(filepath): + self.indexer.flush() + if self.indexer.has_buffered_source(filepath): + if filepath not in self._oversized_files: + logger.error( + "Oversized JSONL checkpoint deferred for unconfirmed entries: %s", + filepath, + ) + self._oversized_files.add(filepath) + return True + if not inode_changed and not file_rewound: + confirmed_offset, _confirmed_inode = self.registry.get(filepath) + offset = confirmed_offset + pending_bytes = max(file_stat.st_size - offset, 0) + if pending_bytes <= self.max_file_bytes: + self._oversized_files.discard(filepath) + return FalsePlease also add regression tests for "oversized + inode-changed + unconfirmed buffered entries" and "oversized + rewound + unconfirmed buffered entries" alongside the fix, and re-run the watcher suite before merging given the guideline note that "current test suite has 929 tests."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/brainlayer/watcher.py` around lines 1172 - 1237, The oversized-file checkpoint path must protect unconfirmed buffered entries for inode changes and rewinds as well as normal growth. In _skip_oversized_file, check indexer.has_buffered_source(filepath) and confirm/flush the registry state before any checkpoint write, preventing stale buffered offsets from overwriting the new inode or rewind checkpoint; preserve the existing defer behavior when confirmation is unavailable. Add regression tests covering oversized inode replacement and rewind with unconfirmed entries, then run the watcher test suite.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/brainlayer/watcher.py`:
- Around line 1172-1237: The oversized-file checkpoint path must protect
unconfirmed buffered entries for inode changes and rewinds as well as normal
growth. In _skip_oversized_file, check indexer.has_buffered_source(filepath) and
confirm/flush the registry state before any checkpoint write, preventing stale
buffered offsets from overwriting the new inode or rewind checkpoint; preserve
the existing defer behavior when confirmation is unavailable. Add regression
tests covering oversized inode replacement and rewind with unconfirmed entries,
then run the watcher test suite.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8211ab15-a3cf-44c2-a88d-e46a0733128a
📒 Files selected for processing (2)
src/brainlayer/watcher.pytests/test_jsonl_watcher.py
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: test (3.11)
- GitHub Check: test (3.12)
- GitHub Check: test (3.13)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Flag risky DB or concurrency changes explicitly and do not hand-wave lock behavior
Enforce one-write-at-a-time concurrency constraint; reads are safe but brain_digest is write-heavy and must not run in parallel with other MCP work
Run pytest before claiming behavior changed safely; current test suite has 929 tests
Files:
src/brainlayer/watcher.pytests/test_jsonl_watcher.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: Usepaths.py:get_db_path()for all database path resolution; do not hard-code the BrainLayer database path.
Each worker must use its own database connection and retry onSQLITE_BUSY.
For bulk database operations, stop enrichment workers, checkpoint WAL before and after, drop and recreate FTS triggers around large deletes, batch deletes in 5–10K chunks, and checkpoint every three batches.
Use the canonical BrainLayer naming:BrainLayer (זיכרון)means “memory.”
Files:
src/brainlayer/watcher.py
src/brainlayer/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/brainlayer/**/*.py: Preserveai_code,stack_trace, anduser_messageverbatim; skip noise, summarize build logs, and retain only structure for directory listings.
Use AST-aware tree-sitter chunking, never split stack traces, and mask large tool output.
Keep Groq as the primary enrichment backend, Gemini as fallback, and Ollama as the offline last resort; honorBRAINLAYER_ENRICH_BACKENDandBRAINLAYER_ENRICH_RATE.
Default searches must exclude lifecycle-managed chunks;include_archived=Truemay expose history.brain_supersedemust apply its personal-data safety gate, andbrain_archivemust soft-delete with a timestamp.
Files:
src/brainlayer/watcher.py
src/brainlayer/watcher*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/brainlayer/watcher*.py: The JSONL watcher must apply filters in order: entry-type whitelist, classification, minimum chunk length, and system-reminder stripping.
Persist watcher offsets, detect file rewinds, and soft-archive chunks reverted by checkpoint restoration.
Files:
src/brainlayer/watcher.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Tests must not refresh the production backup heartbeat log; use
BRAINLAYER_BACKUP_LOG_PATHand set provenance topytest.
Files:
tests/test_jsonl_watcher.py
🪛 ast-grep (0.44.1)
tests/test_jsonl_watcher.py
[info] 1269-1269: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"role": "user", "content": "x" * 512})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 1289-1289: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"role": "user", "content": "y" * 256})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 1304-1304: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"role": "user", "content": "x" * 512})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 1322-1322: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"role": "user", "content": "y" * 256})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 1337-1337: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"role": "user", "content": "pending"})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 1354-1354: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"role": "user", "content": "x" * 256})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 1365-1365: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"role": "user", "content": "first"})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 1367-1367: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"role": "user", "content": "second"})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 1389-1389: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"role": "user", "content": "x" * 256})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 1400-1400: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"role": "user", "content": "x" * 256})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🔇 Additional comments (9)
src/brainlayer/watcher.py (7)
27-27: LGTM!Also applies to: 48-73
529-599: LGTM!
747-751: LGTM!
887-890: LGTM!
1134-1171: LGTM!
1246-1246: LGTM! This correctly reconciles_oversized_filesagainst currently discovered, non-denylisted paths, addressing the previously-flagged unbounded-growth concern.
1269-1281: LGTM!tests/test_jsonl_watcher.py (2)
1266-1421: LGTM! These new tests correctly exercise their targeted branches (replacement-inode cap, same-inode rewind cap, unconfirmed-entry retention, partial-watermark confirmation, and oversized-file forgetting on disappearance/denylist).
1432-1432: LGTM!Also applies to: 1444-1444, 1482-1482
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_7126e667-850c-427d-a554-9a8bde42c4ea) |
|
You need to increase your spend limit or enable usage-based billing to run background agents. Go to Cursor |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7890d188f2
ℹ️ 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".
| if self.indexer.has_buffered_source(filepath): | ||
| self.indexer.flush() | ||
| confirmed_offset, confirmed_inode = self.registry.get(filepath) | ||
| if confirmed_inode != file_stat.st_ino or confirmed_offset < tailer_offset: |
There was a problem hiding this comment.
When a tailer has advanced past records that _normalize_lines drops (for example Codex response_item rows whose payload is not a message), those rows never enter indexer, so has_buffered_source() is false and no watermark can move the registry. If that file then grows past BRAINLAYER_WATCH_MAX_FILE_BYTES, this check keeps returning early forever because confirmed_offset < tailer_offset, so the oversized checkpoint is never written and later small appends to the same file are never ingested. The cap should not treat tailer-only offsets from discarded records as unconfirmed buffered work, or those skipped offsets need to be confirmable.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 9be818a. The watcher now checkpoints intentionally discarded progress only when the registry has already confirmed every preceding indexable record and no source entry remains buffered. It therefore prevents the cap from deferring forever after dropped-only records without ever jumping past mixed-stream unconfirmed work. RED/GREEN regressions cover both cases; watcher suite 96 passed, full pre-push gate 3610 passed.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_3c9f754c-054e-4c8a-9400-8d1f47baaf5d) |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
You need to increase your spend limit or enable usage-based billing to run background agents. Go to Cursor |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9be818ad65
ℹ️ 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".
| read_start_offset = tailer.offset | ||
| new_lines = tailer.read_buffered_lines(max_lines=self.max_lines_per_file) | ||
| else: | ||
| if self._skip_oversized_file(filepath): |
There was a problem hiding this comment.
When a tailer has an unterminated buffered JSONL record, has_complete_buffered_line() is false, so this branch calls _skip_oversized_file before reading the newly appended bytes that may complete that record. If a writer appends the newline for a small buffered record together with a large following record that pushes pending bytes over the cap, _skip_oversized_file checkpoints to EOF and drops the now-complete buffered record, which regresses the watcher's documented partial-write handling.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_4bc09bc8-93a2-40c0-9a10-4293b0dc44ba) |
|
You need to increase your spend limit or enable usage-based billing to run background agents. Go to Cursor |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Final exact-head receipt for a4ed29d: local mandatory pre-push gate 3610 passed, 9 skipped, 1 xfailed; watcher suite 96 passed; GitHub Python 3.11/3.12/3.13, lint, Swift, CodeRabbit, Macroscope all green; Codex exact-head review found no major issues. Cursor/Bugbot were requested and returned quota/spend-limit skips. All actionable review findings are fixed and dispositioned. |
1 participant
Summary
BRAINLAYER_WATCH_MAX_FILE_BYTES(100 MiB default), checkpoint skipped oversized suffixes immediately, and continue processing other filesRoot cause
The serial watcher could spend each poll in a quadratic
any(... is_relative_to(...))prune scan across thousands of tracked files. Separately, one multi-gigabyte rollout suffix could be read without a bound and monopolize the watcher. Together these starved liveness and realtime inserts.Verification
88 passed3672 passed, 9 skipped, 77 deselected, 1 xfailed3602 passed, 9 skipped, 61 deselected, 1 xfailed; MCP registration3 passed; isolated eval/hook routing40 passed; Bun1 passed; FTS determinism passedgit diff --check origin/main...HEAD: passedReview note
Local CodeRabbit suggested retaining a missing tracked file when a live file exists beneath its parent. That is contrary to the existing prune contract: a live descendant is precisely the parent evidence that makes the missing candidate provably stale and removable. Existing and new tests cover direct-parent evidence, nested unavailable roots, and empty subtrees, so no semantic change was made for that suggestion.
Operational scope
This PR changes source and tests only. It does not deploy or alter the live LaunchAgent, Homebrew Cellar, release tags, package artifacts, tap, or cask.
Note
Medium Risk
Changes core realtime ingestion offset semantics (skip/checkpoint of large pending suffixes and discarded-byte advancement); misconfiguration or edge cases around unconfirmed flushes could drop or delay data, though tests target those paths.
Overview
Addresses watcher starvation from slow offset pruning and unbounded reads on huge rollout suffixes.
Prune performance:
OffsetRegistry.prune_missing_filesnow builds alive_parent_dirsset once per cycle and uses membership checks instead of scanning every live file withis_relative_to, making pruning linear in tracked paths rather than quadratic.Oversized pending reads:
BRAINLAYER_WATCH_MAX_FILE_BYTES(default 100 MiB) limits how many unread bytes the watcher will pull from a file in one go. When pending data exceeds the cap,_skip_oversized_filecheckpoints the registry to EOF (with immediate flush), logs a warning, and skips further reads for that file until a smaller append brings pending under the limit;0disables the cap.Poll loop behavior:
poll_oncedrains in-memory tailer buffers viaread_buffered_linesbefore opening files (so buffered lines are not lost when an oversized append would otherwise short-circuit reads). Rewind side effects move to_handle_rewind._checkpoint_discarded_progressadvances confirmed offsets past bytes that were read but not indexable, without jumping past unconfirmed indexer batches (has_buffered_source).Regression tests cover prune complexity, oversize skip/checkpoint, rewind-on-replace, and env validation.
Reviewed by Cursor Bugbot for commit a4ed29d. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Fix watcher ingestion starvation by skipping oversized files and draining buffered lines
_skip_oversized_filetoJSONLWatcher: when pending unread bytes exceed a configurable threshold (BRAINLAYER_WATCH_MAX_FILE_BYTES, default 100 MiB), the file is checkpointed to its end and skipped until it falls below the threshold.JSONLTailer.read_buffered_linesandhas_complete_buffered_lineso callers can consume buffered records without additional filesystem reads.BatchIndexer.has_buffered_sourceto prevent checkpointing past unconfirmed buffered entries during oversized-file skips.OffsetRegistry.prune_missing_filesto use precomputed parent-directory sets instead of per-file linear scans.Macroscope summarized a4ed29d.
Summary by CodeRabbit