Skip to content

fix(watcher): derive projects from workspace metadata - #632

Merged
EtanHey merged 6 commits into
mainfrom
fix/d2-project-derivation
Aug 1, 2026
Merged

fix(watcher): derive projects from workspace metadata#632
EtanHey merged 6 commits into
mainfrom
fix/d2-project-derivation

Conversation

@EtanHey

@EtanHey EtanHey commented Aug 1, 2026

Copy link
Copy Markdown
Owner

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

  • Project derivation now uses trusted session workspace metadata, preserving Claude project encodings and returning NULL when no workspace is known.
  • The production repair re-derived 132,546 rows; zero numeric project values remain.
  • The backfill is narrow, resumable, batched, and records a rollback artifact before mutation.

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 project values (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-numeric projects/<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.py to update only one- and two-digit project values by re-deriving from session files—mandatory rollback TSV, batched WAL checkpoints, and the standard writer pidfile lock.

Also caps mcp to >=1.0.0,<2.0.0 in pyproject.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

  • Adds _extract_workspace_project and _extract_project_from_session_file helpers in watcher_bridge.py to derive a project name from session metadata (e.g. cwd, workspace, workdir) for date-partitioned tools like Codex, Cursor, and Gemini.
  • Extends the flush callback to cache resolved projects per source file and retry derivation only when the source file changes (append) or is replaced/truncated.
  • Adds scripts/d2_project_backfill.py, a one-off CLI utility to backfill project values in the chunks table for rows where the project is a one- or two-digit string, with rollback TSV export and writer-lock enforcement.
  • Behavioral Change: flush now attaches projects for previously unresolved date-partitioned sources; rows that had numeric project values may be updated or set to NULL by the backfill script.

Macroscope summarized 57afe1a.

Summary by CodeRabbit

  • New Features
    • Added a utility to restore meaningful project names for records currently using numeric identifiers.
    • Supports safe, resumable processing with rollback artifacts and outcome reporting.
  • Bug Fixes
    • Improved project detection using trusted workspace metadata across supported session sources.
    • Prevents unreliable numeric or arbitrary directory names from being assigned as projects.
    • Rechecks session files when metadata becomes available and avoids redundant processing.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Your trial has ended. Reactivate Greptile to resume code reviews.

@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The 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.

Changes

Project metadata recovery

Layer / File(s) Summary
Runtime project extraction and flush resolution
src/brainlayer/watcher_bridge.py, tests/test_watcher_bridge.py
Project extraction now uses explicit metadata or trusted Claude paths. It rejects numeric and root names, scans session files when needed, caches successful results, and retries unresolved files after metadata changes.
Guarded numeric-project backfill
scripts/d2_project_backfill.py, tests/test_project_backfill.py
The backfill validates inputs, acquires the writer lock, creates rollback data before mutation, updates numeric projects in batches, supports idempotent reruns, and exposes a CLI.

Estimated code review effort: 4 (Complex) | ~45 minutes

Suggested reviewers: bhanuprasad14

Poem

A rabbit found names in session-file trails,
And swept numeric crumbs from database pails.
With locks held tight and rollback near,
Each project grew clear, each change sincere.
Hop, batch, checkpoint—metadata sings!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main watcher change: deriving projects from trusted workspace metadata.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/d2-project-derivation

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.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

Fix All in Cursor

❌ 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.

Comment thread src/brainlayer/watcher_bridge.py Outdated
Comment thread src/brainlayer/watcher_bridge.py
Comment thread src/brainlayer/watcher_bridge.py Outdated

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

Copy link
Copy Markdown

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: 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".

Comment thread scripts/d2_project_backfill.py Outdated
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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge 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 👍 / 👎.

Comment thread scripts/d2_project_backfill.py Outdated
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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge 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 👍 / 👎.

@EtanHey

EtanHey commented Aug 1, 2026

Copy link
Copy Markdown
Owner Author

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 source_projects cache: it is only an optimization, and caching None converts transient underivability into permanent missing projects. The 77-line implementation-plan artifact is also scenic/stale: it promises audit/progress state and an explicit live-DB opt-in that the implementation does not contain (docs/plans/2026-08-01-d2-project-derivation-design.md:7,57 @ a5d4dac0e7a13cb88b1a8ce7300d6597b0765953). Delete it or make it accurately describe the minimal shipped design; do not add machinery merely to satisfy the plan.

TESTS: On head a5d4dac0e7a13cb88b1a8ce7300d6597b0765953, the targeted D2 set was 16/16 passing. I then restored the original date-directory fallback: 6 tests died (Codex, Cursor, Gemini, no-signal, flush integration, and backfill), with 10 passing. After restoring the PR code, the same 16/16 passed and the worktree was clean. The complete relevant pair (tests/test_watcher_bridge.py tests/test_project_backfill.py) is 51 passed. ruff check passes, but ruff format --check fails on tests/test_project_backfill.py:11-13 @ a5d4dac0e7a13cb88b1a8ce7300d6597b0765953, matching CI. The Python CI jobs fail during unrelated MCP test collection (Server.list_tools missing) before D2 tests run; I did not classify that as a D2 regression.

MISSED BY FIRST REVIEW: Four new findings. (1) None project derivations are cached forever at src/brainlayer/watcher_bridge.py:306,381-383 @ a5d4dac0e7a13cb88b1a8ce7300d6597b0765953; a temp-file probe with metadata appended after the first flush produced None for both flushes. (2) cwd="/" returns project="" because the guard only rejects digits at src/brainlayer/watcher_bridge.py:241-243 @ a5d4dac0e7a13cb88b1a8ce7300d6597b0765953. (3) rollback is optional at scripts/d2_project_backfill.py:41,54-55,107 @ a5d4dac0e7a13cb88b1a8ce7300d6597b0765953; a temp DB was mutated with zero rollback artifacts. (4) the direct SQLite migration bypasses the repository's writer pidfile acquired by VectorStore at src/brainlayer/vector_store.py:307-317,346-396 @ a5d4dac0e7a13cb88b1a8ce7300d6597b0765953; a temp probe successfully mutated while that standard lock was held.

BLOCKERS:

  1. Cache only successful non-None derivations (or retry unresolved paths), and add the append-later regression.
  2. Require a non-empty workspace basename (name and not name.isdigit()), with a root-workspace regression.
  3. Make rollback creation mandatory before any matching row can mutate, and make the script acquire the standard single-writer lock or refuse to run while it is owned. Add tests for both rails. Keep session-file scanning outside BEGIN IMMEDIATE so the write transaction covers writes, not filesystem discovery (scripts/d2_project_backfill.py:74-96 @ a5d4dac0e7a13cb88b1a8ce7300d6597b0765953).
  4. Run ruff format on tests/test_project_backfill.py; branch-owned lint is currently red.
  5. Remove or correct the stale implementation-plan artifact without implementing its speculative audit/progress framework.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Your trial has ended. Reactivate Greptile to resume code reviews.

@cursor

cursor Bot commented Aug 1, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot 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)

Comment thread src/brainlayer/watcher_bridge.py

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

📥 Commits

Reviewing files that changed from the base of the PR and between 0a8cbbf and 06b7349.

📒 Files selected for processing (4)
  • scripts/d2_project_backfill.py
  • src/brainlayer/watcher_bridge.py
  • tests/test_project_backfill.py
  • tests/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.py
  • src/brainlayer/watcher_bridge.py
  • tests/test_project_backfill.py
  • tests/test_watcher_bridge.py
src/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

src/**/*.py: Use paths.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 on SQLITE_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: Preserve ai_code, stack_trace, and user_message verbatim; 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; honor BRAINLAYER_ENRICH_BACKEND and BRAINLAYER_ENRICH_RATE.
Default searches must exclude lifecycle-managed chunks; include_archived=True may expose history. brain_supersede must apply its personal-data safety gate, and brain_archive must 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_PATH and set provenance to pytest.

Files:

  • tests/test_project_backfill.py
  • tests/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 & Integration

No change needed. scripts/__init__.py exists, and pytest’s pythonpath includes src, so scripts.d2_project_backfill imports as intended.

scripts/d2_project_backfill.py (1)

41-51: 🩺 Stability & Availability

No change needed. _operation_writer_lock sets the single pidfile instance attribute that VectorStore uses: db_path.

Comment thread scripts/d2_project_backfill.py
Comment thread scripts/d2_project_backfill.py Outdated
Comment on lines +265 to 282
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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🚀 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 None

For 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.

Comment on lines +1 to +11
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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 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.

Comment thread tests/test_project_backfill.py Outdated

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

Copy link
Copy Markdown

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: 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".

Comment thread scripts/d2_project_backfill.py Outdated
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]

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 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in 9f3cf43: the untouched count now uses COALESCE(..., 0) = 0, with a pre-existing NULL fixture asserted in the idempotency test.

Comment thread src/brainlayer/watcher_bridge.py Outdated
Comment on lines +382 to +383
if project is None:
project = _extract_project_from_session_file(source_file)

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 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

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.

Comment thread src/brainlayer/watcher_bridge.py Outdated
Comment on lines +381 to +385
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

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 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

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.

@cursor

cursor Bot commented Aug 1, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot 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)

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Your trial has ended. Reactivate Greptile to resume code reviews.

@cursor

cursor Bot commented Aug 1, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot 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)

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Your trial has ended. Reactivate Greptile to resume code reviews.

@EtanHey

EtanHey commented Aug 1, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai review
@codex review
@cursor @BugBot review

@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown

@EtanHey I will review the current changes and verify the previously reported requirements.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

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

Copy link
Copy Markdown

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: 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():

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 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 👍 / 👎.

@EtanHey

EtanHey commented Aug 1, 2026

Copy link
Copy Markdown
Owner Author

CI root cause fixed in 57afe1a: unbounded mcp>=1.0.0 resolved MCP 2.0.0, which removed the 1.x Server.list_tools API. The branch now constrains MCP to <2.0.0; focused MCP + D2 coverage is 96 passed. @coderabbitai review @codex review

@cursor

cursor Bot commented Aug 1, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot 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)

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Your trial has ended. Reactivate Greptile to resume code reviews.

@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown

@EtanHey I noted the MCP 1.x compatibility constraint and the reported focused test result. I will review the current changes in PR #632.


✏️ Learnings added
Learnt from: EtanHey
URL: https://github.com/EtanHey/brainlayer/pull/632

Timestamp: 2026-08-01T15:14:30.761Z
Learning: This repository uses the MCP 1.x `Server.list_tools` API. The dependency must constrain `mcp` to `<2.0.0`, because MCP 2.0.0 removes that API.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

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

Copy link
Copy Markdown

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: 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".

Comment on lines +271 to +272
with Path(source_file).open(encoding="utf-8") as handle:
for line in handle:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge 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 👍 / 👎.

@EtanHey
EtanHey merged commit d8c1590 into main Aug 1, 2026
9 checks passed
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