fix(watcher): derive projects from workspace metadata - #632
Conversation
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
📝 WalkthroughWalkthroughThe PR adds runtime project extraction from workspace metadata and session files. It also adds a guarded CLI backfill that derives numeric projects, exports rollback data, updates rows in batches, and reports outcome counts. ChangesProject metadata recovery
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a5d4dac. Configure here.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a5d4dac0e7
ℹ️ 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 not _BATCH_SIZE_MIN <= batch_size <= _BATCH_SIZE_MAX: | ||
| raise ValueError("batch_size must be between 5,000 and 10,000") | ||
|
|
||
| conn = sqlite3.connect(Path(db_path), timeout=30) |
There was a problem hiding this comment.
Acquire the single-writer lock before backfilling
When this script runs against the canonical database while the drain or enrichment writer is active, this direct sqlite3 connection bypasses the writer pidfile acquired by VectorStore in src/brainlayer/vector_store.py:307-317. The subsequent BEGIN IMMEDIATE transactions can therefore contend with the purported sole writer, and each transaction may retain that lock while thousands of session files are scanned, causing write timeouts or failed production work. Acquire the standard writer lock or refuse to run until the active writer has stopped.
AGENTS.md reference: AGENTS.md:L28-L31
Useful? React with 👍 / 👎.
| def main(argv: list[str] | None = None) -> int: | ||
| parser = argparse.ArgumentParser(description="Re-derive numeric BrainLayer projects from session metadata") | ||
| parser.add_argument("--db", required=True, type=Path) | ||
| parser.add_argument("--rollback-path", type=Path) |
There was a problem hiding this comment.
Require a rollback artifact before mutating rows
When the CLI is invoked without --rollback-path, which is currently accepted, the migration overwrites every targeted numeric project and sets underivable rows to NULL without preserving the original values. A mistaken derivation or partial operational rollout then cannot be reversed even though this is a bulk repair of the canonical database; require an artifact path or generate one automatically before the first update.
AGENTS.md reference: AGENTS.md:L5-L8
Useful? React with 👍 / 👎.
|
VERDICT: ITERATE TASTE: The metadata helper plus the one-off repair script are proportionate to D2; this is not a framework. The speculative piece is the lifetime TESTS: On head MISSED BY FIRST REVIEW: Four new findings. (1) BLOCKERS:
|
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_2383a209-6cc4-408f-ae57-667a4f6afea3) |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@scripts/d2_project_backfill.py`:
- Around line 71-74: Update the rows_left_untouched query in
scripts/d2_project_backfill.py:71-74 to wrap NUMERIC_PROJECT_PREDICATE with
COALESCE(..., 0) = 0 so NULL project values are counted as untouched. In
tests/test_project_backfill.py:14-30, add a fixture row with project set to NULL
and assert that rows_left_untouched includes it.
- Around line 103-113: Update the row counters in the loop over rows to use the
UPDATE cursor’s affected-row count, incrementing rows_updated and either
rows_set_null or rows_rederived only when the statement changes a row. Preserve
the existing NUMERIC_PROJECT_PREDICATE guard and project classification for
successfully updated rows.
In `@src/brainlayer/watcher_bridge.py`:
- Around line 265-282: Update _extract_project_from_session_file to scan only a
bounded prefix containing session-header metadata instead of reading the entire
file. Extend the source_projects caching in the flush path around Lines 381-385
to memoize unresolved files with their size and mtime, and invalidate that
negative result when either changes so retries still occur as the session grows
or is modified.
In `@tests/test_project_backfill.py`:
- Around line 1-11: Run ruff format on tests/test_project_backfill.py and apply
its formatting changes throughout the file, without modifying behavior or
unrelated files.
- Around line 150-159: Make the lock-holder subprocess lifecycle in this test
robust: configure its Popen call with stderr=subprocess.PIPE, include captured
stderr in the readiness assertion message, and update the finally teardown to
avoid writing to a process that already exited, terminating it when necessary
and waiting for cleanup. Preserve the existing release handshake for a
still-running child and ensure teardown does not mask the original failure.
🪄 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 Plus
Run ID: 0baad056-1216-4ff2-b101-b80a61e33837
📒 Files selected for processing (4)
scripts/d2_project_backfill.pysrc/brainlayer/watcher_bridge.pytests/test_project_backfill.pytests/test_watcher_bridge.py
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: test (3.12)
- GitHub Check: test (3.11)
- GitHub Check: test (3.13)
- GitHub Check: Macroscope - Correctness Check
🧰 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:
scripts/d2_project_backfill.pysrc/brainlayer/watcher_bridge.pytests/test_project_backfill.pytests/test_watcher_bridge.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_bridge.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_bridge.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_bridge.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_project_backfill.pytests/test_watcher_bridge.py
🪛 ast-grep (0.45.0)
tests/test_project_backfill.py
[info] 40-40: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"type": "session_meta", "payload": {"cwd": "/Users/test/Gits/brainlayer"}})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[error] 122-148: Command coming from incoming request
Context: subprocess.Popen(
[
sys.executable,
"-c",
"""
import sys
from pathlib import Path
from brainlayer.vector_store import VectorStore
store = VectorStore.new(VectorStore)
store.db_path = Path(sys.argv[1])
store._writer_pidfile_acquired = False
store._acquire_writer_pidfile()
print("ready", flush=True)
sys.stdin.readline()
store._release_writer_pidfile()
""",
str(db_path),
],
env={
**os.environ,
"PYTHONPATH": f"{repo_root / 'src'}:{os.environ.get('PYTHONPATH', '')}",
},
stdin=subprocess.PIPE,
stdout=subprocess.PIPE,
text=True,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
tests/test_watcher_bridge.py
[info] 215-221: use jsonify instead of json.dumps for JSON output
Context: json.dumps(
{
"type": "session_meta",
"timestamp": "2026-07-31T12:26:57.941Z",
"payload": {"cwd": "/Users/etanheyman/Gits/t3code"},
}
)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 249-255: use jsonify instead of json.dumps for JSON output
Context: json.dumps(
{
"type": "session_meta",
"timestamp": "2026-07-31T12:26:57.941Z",
"payload": {"cwd": "/Users/etanheyman/Gits/t3code"},
}
)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🪛 OpenGrep (1.26.0)
scripts/d2_project_backfill.py
[ERROR] 37-37: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 72-72: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 81-90: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 105-108: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
🔇 Additional comments (6)
src/brainlayer/watcher_bridge.py (3)
226-244: LGTM!
247-262: LGTM!
306-306: LGTM!Also applies to: 381-385
tests/test_watcher_bridge.py (1)
154-188: LGTM!Also applies to: 208-264
tests/test_project_backfill.py (1)
11-11: 🗄️ Data Integrity & IntegrationNo change needed.
scripts/__init__.pyexists, and pytest’spythonpathincludessrc, soscripts.d2_project_backfillimports as intended.scripts/d2_project_backfill.py (1)
41-51: 🩺 Stability & AvailabilityNo change needed.
_operation_writer_locksets the single pidfile instance attribute thatVectorStoreuses:db_path.
| def _extract_project_from_session_file(source_file: str) -> str | None: | ||
| """Derive a project from the source file's durable session metadata.""" | ||
| project = _extract_project_from_source(source_file) | ||
| if project is not None: | ||
| return project | ||
| try: | ||
| with Path(source_file).open(encoding="utf-8") as handle: | ||
| for line in handle: | ||
| try: | ||
| entry = json.loads(line) | ||
| except json.JSONDecodeError: | ||
| continue | ||
| project = _extract_project_from_source(source_file, entry) | ||
| if project is not None: | ||
| return project | ||
| except OSError: | ||
| return None | ||
| return None |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
Bound the session-file scan and memoize unresolved results.
_extract_project_from_session_file reads the whole source file when metadata is absent. The flush path calls it again on every flush for the same file, because only successful derivations are cached (Lines 381-385). A Codex or Claude session file grows continuously during a live session, so each flush re-reads and re-parses the complete file. This adds unbounded I/O and JSON parsing to the flush hot path.
Workspace metadata appears in the session header (for example session_meta), so a bounded prefix scan is sufficient. Keep the retry behavior by invalidating the negative result when the file size or mtime changes.
♻️ Proposed bounded scan
+_SESSION_METADATA_SCAN_LINES = 200
+
def _extract_project_from_session_file(source_file: str) -> str | None:
"""Derive a project from the source file's durable session metadata."""
project = _extract_project_from_source(source_file)
if project is not None:
return project
try:
with Path(source_file).open(encoding="utf-8") as handle:
- for line in handle:
+ for line_number, line in enumerate(handle):
+ if line_number >= _SESSION_METADATA_SCAN_LINES:
+ break
try:
entry = json.loads(line)
except json.JSONDecodeError:
continue
- project = _extract_project_from_source(source_file, entry)
+ project = _extract_workspace_project(entry)
if project is not None:
return project
except OSError:
return None
return NoneFor the negative-result cache, store (size, mtime) alongside the None value in source_projects and re-derive only when the tuple changes.
🤖 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_bridge.py` around lines 265 - 282, Update
_extract_project_from_session_file to scan only a bounded prefix containing
session-header metadata instead of reading the entire file. Extend the
source_projects caching in the flush path around Lines 381-385 to memoize
unresolved files with their size and mtime, and invalidate that negative result
when either changes so retries still occur as the session grows or is modified.
| import json | ||
| import os | ||
| import sqlite3 | ||
| import subprocess | ||
| import sys | ||
| from pathlib import Path | ||
|
|
||
| import pytest | ||
|
|
||
| from brainlayer.vector_store import WriterInUseError | ||
| from scripts.d2_project_backfill import backfill_numeric_projects |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Run ruff format on this file.
The PR notes report that ruff format fails on tests/test_project_backfill.py while lint passes. Format the file so CI passes.
🤖 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 `@tests/test_project_backfill.py` around lines 1 - 11, Run ruff format on
tests/test_project_backfill.py and apply its formatting changes throughout the
file, without modifying behavior or unrelated files.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 06b73497cb
ℹ️ 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".
| source_projects: dict[str, str | None] = {} | ||
| try: | ||
| rows_left_untouched = int( | ||
| conn.execute(f"SELECT COUNT(*) FROM chunks WHERE NOT {NUMERIC_PROJECT_PREDICATE}").fetchone()[0] |
There was a problem hiding this comment.
Count NULL projects as untouched rows
When chunks contains rows whose project is already NULL, NOT (project GLOB ...) also evaluates to NULL, so this count omits those untouched rows. Because the column is nullable and this migration itself creates additional NULL projects, a subsequent idempotence run can substantially underreport rows_left_untouched; include project IS NULL explicitly in the predicate.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 9f3cf43: the untouched count now uses COALESCE(..., 0) = 0, with a pre-existing NULL fixture asserted in the idempotency test.
| if project is None: | ||
| project = _extract_project_from_session_file(source_file) |
There was a problem hiding this comment.
Avoid rescanning metadata-free transcripts for every entry
For a Codex, Cursor, or Gemini transcript that never contains a recognized cwd, workspace, or workdir, every accepted entry reaches this cache miss and _extract_project_from_session_file rereads the entire growing JSONL from byte zero. Since watcher batches contain multiple entries and files may reach the configured 100 MB limit, this becomes quadratic I/O and can stall real-time ingestion; retain a per-file scan offset or negative result tied to the current file size so only appended bytes are retried.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4a7c6c3: unresolved results are memoized by size/mtime and retried only after the source changes. Full scans remain deliberate: a fixed header prefix could convert a later trustworthy workspace signal into NULL.
| project = source_projects.get(source_file) | ||
| if project is None: | ||
| project = _extract_project_from_session_file(source_file) | ||
| if project is not None: | ||
| source_projects[source_file] = project |
There was a problem hiding this comment.
Invalidate cached projects when source files are replaced
For date-partitioned providers, this cache is keyed only by the path for the lifetime of the flush callback. JSONLWatcher explicitly handles inode changes and restarts replacement files from offset zero, but replacing a path with a transcript whose session metadata names another workspace will continue assigning the old cached project to every new chunk; key the cache by file identity or invalidate it when the source is replaced or rewound.
AGENTS.md reference: AGENTS.md:L5-L8
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4a7c6c3: successful cache entries track device/inode/size and re-derive when the source is replaced or rewound; append-only growth retains the cached project. Added a replacement regression.
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_65cdc813-ef5a-4d36-809b-544bc77c3ff3) |
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_f113b6e3-abfb-4263-b72e-452c5dc18d26) |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
@coderabbitai review |
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a7c6c3d02
ℹ️ 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 not isinstance(value, str) or not value.strip(): | ||
| continue | ||
| name = _normalize_project_name(Path(value).name) | ||
| if name and not name.isdigit(): |
There was a problem hiding this comment.
Preserve numeric names from trusted workspace metadata
When a legitimate repository or workspace is named with digits only (for example, /Users/me/Gits/42), this condition rejects the explicit workspace signal and queues every new chunk with project = NULL. The backfill compounds this by selecting existing one- or two-digit projects and using the same extractor, thereby replacing a legitimate 42 with NULL and breaking project-scoped recall; numeric filtering should apply only to inferred date-path segments, not trusted workspace metadata.
AGENTS.md reference: AGENTS.md:L5-L8
Useful? React with 👍 / 👎.
|
CI root cause fixed in 57afe1a: unbounded |
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_2f760ac6-3361-4df9-bc1d-de7a2555e0f0) |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
✏️ Learnings added
✅ Action performedReview finished.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 57afe1a650
ℹ️ 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".
| with Path(source_file).open(encoding="utf-8") as handle: | ||
| for line in handle: |
There was a problem hiding this comment.
Tolerate undecodable lines while scanning transcripts
When a watched JSONL contains an invalid UTF-8 record, JSONLTailer.read_buffered_lines deliberately skips that record and continues emitting surrounding valid entries, but this new text-mode whole-file scan raises UnicodeDecodeError because only JSONDecodeError and OSError are handled. Every flush for that source then fails, and BatchIndexer eventually quarantines otherwise valid entries instead of ingesting them; decode per line or catch the decoding error so corrupt records remain skippable.
AGENTS.md reference: AGENTS.md:L5-L8
Useful? React with 👍 / 👎.

What broke and how it was found
Date-partitioned Codex paths were parsed as project names, assigning chunks the day-of-month such as 30. The defect was found by querying numeric project values in the canonical database.
Evidence
Review
This branch was pair-reviewed by a separate agent under the taste gate: smallest correct solution, behavioural tests, and no speculative machinery.
Operational note
Enrichment and drain are currently disabled with launchctl to prevent provenance-tag decay. Re-enable deliberately after the provenance-column ownership question is settled.
Note
Medium Risk
Changes provenance tagging for all watcher ingests and ships a direct SQLite mutation path; scope is narrowed (numeric projects only, rollback + writer lock) but wrong extraction or backfill could mis-tag or bulk-update chunks.
Overview
Fixes incorrect
projectvalues (e.g. day-of-month from Codex/Cursor/Gemini date paths) by changing how the watcher assigns projects on ingest.Ingest (
watcher_bridge): Stops using parent directory names as projects. For date-partitioned session paths, project now comes from session JSONL workspace fields (cwd/workspace/workdir); Claude still uses non-numericprojects/<workspace>when present. The flush path caches resolved projects, re-scans session files when metadata is appended later, and invalidates cache when a source file is replaced or shrinks.Repair: Adds
scripts/d2_project_backfill.pyto update only one- and two-digitprojectvalues by re-deriving from session files—mandatory rollback TSV, batched WAL checkpoints, and the standard writer pidfile lock.Also caps
mcpto>=1.0.0,<2.0.0inpyproject.toml.Reviewed by Cursor Bugbot for commit 57afe1a. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Derive projects from workspace metadata in watcher flush callback
_extract_workspace_projectand_extract_project_from_session_filehelpers inwatcher_bridge.pyto derive a project name from session metadata (e.g.cwd,workspace,workdir) for date-partitioned tools like Codex, Cursor, and Gemini.scripts/d2_project_backfill.py, a one-off CLI utility to backfillprojectvalues in thechunkstable for rows where the project is a one- or two-digit string, with rollback TSV export and writer-lock enforcement.Macroscope summarized 57afe1a.
Summary by CodeRabbit