feat(kg): expire stale entity/relation facts via reconciliation pass#316
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
📝 WalkthroughWalkthroughAdds KG expiration/validity columns and indexes, updates repo and search paths to respect validity, refactors code-intelligence to track/refresh/expire observed dependencies, and hardens benchmark evaluation with a ranx Run/Qrels fallback plus manual metric computations. ChangesKG Validity and Expiration System
Benchmark Evaluation Resilience
Sequence DiagramssequenceDiagram
participant Enrich as enrich_projects
participant Schema as _ensure_reconciliation_schema
participant Upsert as _upsert_project
participant AddRel as _add_dependency_relation
participant Expire as _expire_stale_dependency_relations
participant DB as SQLite KG DB
Enrich->>Schema: ensure validity columns and indexes
Schema->>DB: ALTER TABLE / CREATE INDEX
Enrich->>Enrich: compute observed_at, valid_until
Enrich->>Upsert: upsert project with timestamps
Upsert->>DB: UPDATE/INSERT (refresh valid_until, clear expired_at)
Enrich->>AddRel: add dependency for notable libraries
AddRel->>DB: INSERT/UPDATE depends_on (set valid_from/valid_until, clear expired_at)
Enrich->>Expire: expire unobserved depends_on relations
Expire->>DB: UPDATE relations/entities -> set expired_at
sequenceDiagram
participant RunPipeline as run_pipeline
participant Eval as evaluate_pipeline
participant Ranx as ranx.evaluate
participant Fallback as _evaluate_without_ranx
RunPipeline->>Eval: provide run dict (wrapped by _run_from_dict)
Eval->>Eval: has ranx_qrels?
alt ranx_qrels exists
Eval->>Ranx: call ranx.evaluate()
Ranx-->>Eval: ranx metrics
else
Eval->>Fallback: call _evaluate_without_ranx()
Fallback-->>Eval: manual metrics (ndcg/recall/map/mrr)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![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: 8169a6a468
ℹ️ 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".
| SET properties = ?, confidence = 0.99, valid_from = COALESCE(valid_from, ?), | ||
| valid_until = ?, expired_at = NULL |
There was a problem hiding this comment.
When a depends_on edge already exists for the same (source_id, target_id), this update unconditionally rewrites properties to {"source": "code_intelligence"} and resets confidence. That mutates non-code-intelligence facts (for example, manually curated or pipeline-generated facts) into code-intelligence facts, which then makes them eligible for later reconciliation expiry and loses original provenance/evidence.
Useful? React with 👍 / 👎.
| has_active_code_fact = any(_is_code_intelligence_relation(properties) for (properties,) in active_rows) | ||
| if has_active_code_fact: | ||
| continue |
There was a problem hiding this comment.
This liveness check only looks for active code-intelligence relations, so a library is marked expired even when active non-code depends_on relations still point to it. Because search now filters on se/te.expired_at IS NULL, expiring the entity hides still-valid manual/other-pipeline facts across the KG, which is a correctness regression for retrieval.
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/brainlayer/pipeline/code_intelligence.py (1)
414-427:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDo not overwrite non-code-intelligence relation properties on refresh.
The current update path rewrites
propertiesto{"source":"code_intelligence"}for any existingdepends_ontriple. That can clobber non-code-intelligence provenance and later make unrelated facts eligible for stale-expiry logic.💡 Suggested patch
- existing = conn.execute( - "SELECT id FROM kg_relations WHERE source_id = ? AND target_id = ? AND relation_type = 'depends_on'", + existing = conn.execute( + "SELECT id, properties FROM kg_relations WHERE source_id = ? AND target_id = ? AND relation_type = 'depends_on'", (source_id, target_id), ).fetchone() if existing: - if not dry_run: + rel_id, rel_props = existing + if not dry_run and (not rel_props or _is_code_intelligence_relation(rel_props)): conn.execute( """UPDATE kg_relations SET properties = ?, confidence = 0.99, valid_from = COALESCE(valid_from, ?), valid_until = ?, expired_at = NULL WHERE id = ?""", - (json.dumps({"source": CODE_INTELLIGENCE_SOURCE}), observed_at, valid_until, existing[0]), + (json.dumps({"source": CODE_INTELLIGENCE_SOURCE}), observed_at, valid_until, rel_id), ) return target_id🤖 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/pipeline/code_intelligence.py` around lines 414 - 427, The update currently overwrites the entire properties column with {"source": CODE_INTELLIGENCE_SOURCE} which clobbers existing provenance; instead, fetch the current properties for the matched relation (kg_relations row id from existing[0]), parse the JSON, merge/augment it so you add or record CODE_INTELLIGENCE_SOURCE without removing other keys (e.g., preserve an existing "source" value by turning it into a list or appending CODE_INTELLIGENCE_SOURCE, or add a new key like "sources"), then write the merged JSON back in the UPDATE for the kg_relations row (keep the same UPDATE fields confidence/valid_from/valid_until/expired_at but replace the hard overwrite of properties with the merged JSON).
🤖 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/pipeline/code_intelligence.py`:
- Around line 269-273: After switching the DB to WAL mode (where conn =
sqlite3.connect(...) and conn.execute("PRAGMA journal_mode = WAL") is called)
add explicit checkpoints: call conn.execute("PRAGMA wal_checkpoint(FULL)")
immediately after switching to WAL and again after the bulk
reconciliation/upsert work but before the final conn.commit(); only perform
these checkpoints when not dry_run and around the bulk reconciliation pass that
follows _ensure_reconciliation_schema(conn) so the WAL is
flushed/checked-pointed before and after heavy KG upserts.
- Around line 269-273: enrich_projects() currently opens a raw sqlite3 writer
and runs DDL/long transactional writes without the one-writer pidfile,
SQLITE_BUSY retry/busy-timeout, or WAL checkpointing and also clobbers
kg_relations.properties; fix it by (1) acquiring the same one-writer pidfile
mutex used in src/brainlayer/vector_store.py before opening/doing writes in
enrich_projects(), (2) set a busy timeout or retry loop around sqlite3
operations to retry on SQLITE_BUSY (wrap commits/executes that can block), (3)
run PRAGMA wal_checkpoint(FULL) before the big write and again after the final
conn.commit() to reduce WAL bloat, and (4) when updating kg_relations in the
branch that currently uses json.dumps({"source": CODE_INTELLIGENCE_SOURCE}),
merge the existing properties JSON with {"source": CODE_INTELLIGENCE_SOURCE}
(preserve other keys) instead of overwriting; reference the connection variable
conn, _ensure_reconciliation_schema(conn), CODE_INTELLIGENCE_SOURCE and the
kg_relations UPDATE/INSERT logic to locate changes.
---
Outside diff comments:
In `@src/brainlayer/pipeline/code_intelligence.py`:
- Around line 414-427: The update currently overwrites the entire properties
column with {"source": CODE_INTELLIGENCE_SOURCE} which clobbers existing
provenance; instead, fetch the current properties for the matched relation
(kg_relations row id from existing[0]), parse the JSON, merge/augment it so you
add or record CODE_INTELLIGENCE_SOURCE without removing other keys (e.g.,
preserve an existing "source" value by turning it into a list or appending
CODE_INTELLIGENCE_SOURCE, or add a new key like "sources"), then write the
merged JSON back in the UPDATE for the kg_relations row (keep the same UPDATE
fields confidence/valid_from/valid_until/expired_at but replace the hard
overwrite of properties with the merged JSON).
🪄 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: 0336d3b0-eb97-4264-a186-48e96a02ca1b
📒 Files selected for processing (10)
src/brainlayer/eval/benchmark.pysrc/brainlayer/kg_repo.pysrc/brainlayer/mcp/_shared.pysrc/brainlayer/mcp/search_handler.pysrc/brainlayer/pipeline/code_intelligence.pysrc/brainlayer/vector_store.pytests/test_code_intelligence.pytests/test_kg_schema.pytests/test_kg_standard.pytests/test_search_filter_params.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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
**/*.py: Usepaths.py:get_db_path()for all database path resolution; all scripts and CLI must use this function rather than hardcoding paths
When performing bulk database operations: stop enrichment workers first, checkpoint WAL before and after, drop FTS triggers before bulk deletes, batch deletes in 5-10K chunks, and checkpoint every 3 batches
Files:
tests/test_search_filter_params.pysrc/brainlayer/mcp/_shared.pysrc/brainlayer/mcp/search_handler.pytests/test_kg_standard.pytests/test_kg_schema.pysrc/brainlayer/vector_store.pytests/test_code_intelligence.pysrc/brainlayer/kg_repo.pysrc/brainlayer/eval/benchmark.pysrc/brainlayer/pipeline/code_intelligence.py
src/brainlayer/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/brainlayer/**/*.py: Use retry logic onSQLITE_BUSYerrors; each worker must use its own database connection to handle concurrency safely
Classification must preserveai_code,stack_trace, anduser_messageverbatim; skipnoiseentries entirely and summarizebuild_loganddir_listingentries (structure only)
Use AST-aware chunking via tree-sitter; never split stack traces; mask large tool output
For enrichment backend selection: use Groq as primary backend (cloud, configured in launchd plist), Gemini as fallback viaenrichment_controller.py, and Ollama as offline last-resort; allow override viaBRAINLAYER_ENRICH_BACKENDenv var
Configure enrichment rate viaBRAINLAYER_ENRICH_RATEenvironment variable (default 0.2 = 12 RPM)
Implement chunk lifecycle columns:superseded_by,aggregated_into,archived_aton chunks table; exclude lifecycle-managed chunks from default search; allowinclude_archived=Trueto show history
Implementbrain_supersedewith safety gate for personal data (journals, notes, health/finance); use soft-delete forbrain_archivewith timestamp
Addsupersedesparameter tobrain_storefor atomic store-and-replace operations
Run linting and formatting with:ruff check src/ && ruff format src/
Run tests withpytest
UsePRAGMA wal_checkpoint(FULL)before and after bulk database operations to prevent WAL bloat
Files:
src/brainlayer/mcp/_shared.pysrc/brainlayer/mcp/search_handler.pysrc/brainlayer/vector_store.pysrc/brainlayer/kg_repo.pysrc/brainlayer/eval/benchmark.pysrc/brainlayer/pipeline/code_intelligence.py
🪛 OpenGrep (1.21.0)
src/brainlayer/pipeline/code_intelligence.py
[ERROR] 83-83: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 92-92: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
🔇 Additional comments (14)
src/brainlayer/eval/benchmark.py (10)
6-6: LGTM!Also applies to: 10-10, 22-22
165-195: LGTM!
204-204: LGTM!Also applies to: 212-217
231-231: LGTM!
233-245: LGTM!
247-267: LGTM!
269-296: LGTM!
298-303: LGTM!
305-313: LGTM!
315-340: LGTM!tests/test_kg_schema.py (2)
91-91: LGTM!
130-137: LGTM!tests/test_kg_standard.py (2)
163-163: LGTM!
230-230: LGTM!
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4eaa1d370d
ℹ️ 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 properties: | ||
| return False | ||
| try: | ||
| return json.loads(properties).get("source") == CODE_INTELLIGENCE_SOURCE |
There was a problem hiding this comment.
Guarding json.loads(properties).get("source") assumes every JSON payload is an object, but valid JSON scalars/arrays ("foo", 123, null, [1,2]) raise AttributeError here and crash reconciliation. In that case enrich_projects aborts while expiring/refreshing dependencies, so a single malformed or legacy kg_relations.properties value can stop the entire code-intelligence pass.
Useful? React with 👍 / 👎.
| continue | ||
| hits += 1 | ||
| precision_sum += hits / rank | ||
| return precision_sum / len(relevant) |
There was a problem hiding this comment.
The fallback MAP implementation divides AP by len(relevant) even when a cutoff is requested (e.g. map@10), which under-scores runs whenever relevant docs exceed k. For a perfect top-10 with 20 relevant docs this returns 0.5 instead of 1.0, so benchmark comparisons become biased whenever ranx falls back to this path.
Useful? React with 👍 / 👎.
| def _ndcg(self, run: dict[str, Any], query_id: str, cutoff: int | None) -> float: | ||
| ranked = self._ranked_docs(run, cutoff) | ||
| gains = self.qrels.get(query_id, {}) | ||
| dcg = sum((2 ** gains.get(doc_id, 0) - 1) / math.log2(rank + 2) for rank, doc_id in enumerate(ranked)) |
There was a problem hiding this comment.
ndcg fallback is computed with Burges gain (2^rel-1), but the metric name requested in this file is ndcg (not ndcg_burges), so fallback scores diverge from normal ranx ndcg on graded qrels. This changes absolute scores and can reorder pipeline comparisons when ranx is unavailable or errors.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
@coderabbitai review |
|
@codex review |
|
You need to increase your spend limit or enable usage-based billing to run background agents. Go to Cursor |
✅ Actions performedReview triggered.
|
|
Codex Review: Didn't find any major issues. Chef's kiss. ℹ️ 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". |
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1bc7ef8d29
ℹ️ 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".
| FROM kg_current_facts | ||
| WHERE target_id = ? | ||
| AND relation_type = 'depends_on' | ||
| LIMIT 1 |
There was a problem hiding this comment.
When deciding whether to expire a library entity, the liveness check only looks for active depends_on rows, so libraries with other active relations (for example uses_tool) are still marked expired. Because _kg_facts_sql now filters out rows where either endpoint has expired_at, this hides still-valid facts from retrieval even though their relations remain active. Fresh evidence: reproducing with an active uses_tool relation to fastapi still results in fastapi.expired_at being set and _kg_facts_sql(..., "mytool") returning no facts.
Useful? React with 👍 / 👎.
| owner_pid, owner_start_time = _read_pidfile_owner(pidfile) | ||
| if owner_pid is not None and _pidfile_owner_matches(owner_pid, owner_start_time): | ||
| raise RuntimeError(f"another writer is using {db_path} (pid {owner_pid})") | ||
| try: | ||
| pidfile.unlink() |
There was a problem hiding this comment.
In the FileExistsError path, an unreadable/empty pidfile is treated as stale and unlinked immediately. There is a race where another writer has created the pidfile but has not finished writing payload yet; _read_pidfile_owner then returns (None, None), this branch deletes the live lockfile, and a second writer can acquire the lock concurrently. That breaks the one-writer guarantee and can reintroduce DB write contention/corruption under concurrent starts.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
@coderabbitai review |
|
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 635e3db. Configure here.
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/eval/benchmark.py`:
- Around line 333-334: The average-precision computation currently divides
precision_sum by denominator which can become zero or negative when cutoff is 0
or <0; add a guard before computing denominator in the function that uses
precision_sum/relevant/cutoff (the block that sets denominator =
min(len(relevant), cutoff) ...) to validate cutoff: if cutoff is None proceed as
before, but if cutoff <= 0 raise a clear ValueError (or return 0 if you prefer a
silent result) with a message mentioning the invalid cutoff, preventing
ZeroDivisionError and invalid scores.
🪄 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: b44afa44-cd1c-4960-a2b8-e1c55ee57a47
📒 Files selected for processing (8)
src/brainlayer/eval/benchmark.pysrc/brainlayer/mcp/_shared.pysrc/brainlayer/mcp/search_handler.pysrc/brainlayer/pipeline/code_intelligence.pytests/test_code_intelligence.pytests/test_eval_framework.pytests/test_search_filter_params.pytests/test_vector_store_readonly.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: test (3.11)
- GitHub Check: test (3.13)
- GitHub Check: test (3.12)
- GitHub Check: Macroscope - Correctness Check
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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
**/*.py: Usepaths.py:get_db_path()for all database path resolution; all scripts and CLI must use this function rather than hardcoding paths
When performing bulk database operations: stop enrichment workers first, checkpoint WAL before and after, drop FTS triggers before bulk deletes, batch deletes in 5-10K chunks, and checkpoint every 3 batches
Files:
src/brainlayer/mcp/search_handler.pytests/test_vector_store_readonly.pytests/test_search_filter_params.pytests/test_eval_framework.pysrc/brainlayer/mcp/_shared.pytests/test_code_intelligence.pysrc/brainlayer/eval/benchmark.pysrc/brainlayer/pipeline/code_intelligence.py
src/brainlayer/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/brainlayer/**/*.py: Use retry logic onSQLITE_BUSYerrors; each worker must use its own database connection to handle concurrency safely
Classification must preserveai_code,stack_trace, anduser_messageverbatim; skipnoiseentries entirely and summarizebuild_loganddir_listingentries (structure only)
Use AST-aware chunking via tree-sitter; never split stack traces; mask large tool output
For enrichment backend selection: use Groq as primary backend (cloud, configured in launchd plist), Gemini as fallback viaenrichment_controller.py, and Ollama as offline last-resort; allow override viaBRAINLAYER_ENRICH_BACKENDenv var
Configure enrichment rate viaBRAINLAYER_ENRICH_RATEenvironment variable (default 0.2 = 12 RPM)
Implement chunk lifecycle columns:superseded_by,aggregated_into,archived_aton chunks table; exclude lifecycle-managed chunks from default search; allowinclude_archived=Trueto show history
Implementbrain_supersedewith safety gate for personal data (journals, notes, health/finance); use soft-delete forbrain_archivewith timestamp
Addsupersedesparameter tobrain_storefor atomic store-and-replace operations
Run linting and formatting with:ruff check src/ && ruff format src/
Run tests withpytest
UsePRAGMA wal_checkpoint(FULL)before and after bulk database operations to prevent WAL bloat
Files:
src/brainlayer/mcp/search_handler.pysrc/brainlayer/mcp/_shared.pysrc/brainlayer/eval/benchmark.pysrc/brainlayer/pipeline/code_intelligence.py
🔇 Additional comments (20)
src/brainlayer/eval/benchmark.py (1)
293-293: LGTM!Also applies to: 308-312
tests/test_eval_framework.py (1)
5-5: LGTM!Also applies to: 91-132
src/brainlayer/mcp/_shared.py (1)
29-30: LGTM!Also applies to: 50-59
src/brainlayer/mcp/search_handler.py (1)
427-428: LGTM!tests/test_search_filter_params.py (1)
14-14: LGTM!Also applies to: 20-45, 48-82
tests/test_vector_store_readonly.py (1)
222-236: LGTM!src/brainlayer/pipeline/code_intelligence.py (8)
69-72: LGTM!
75-110: LGTM!
113-241: LGTM!
244-270: LGTM!
449-465: LGTM!
470-535: LGTM!
538-614: LGTM!
617-672: LGTM!tests/test_code_intelligence.py (6)
4-4: LGTM!Also applies to: 6-6
321-417: LGTM!
419-584: LGTM!
586-624: LGTM!
626-698: LGTM!
700-766: LGTM!
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 635e3dbf4e
ℹ️ 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".
| python -m brainlayer.pipeline.code_intelligence [--base-dir ~/Gits] [--dry-run] | ||
| """ | ||
|
|
||
| import fcntl |
There was a problem hiding this comment.
Importing brainlayer.pipeline.code_intelligence now fails on Windows because fcntl is imported at module load time, but Python documents fcntl as Unix-only. That means any workflow that imports this module (including enrich_projects) crashes before execution on Windows hosts, which regresses the cross-platform behavior expected for this package. Please guard this import behind a platform check or move it inside the Unix-specific lock path with a non-Unix fallback.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
@coderabbitai review |
|
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c237216ef7
ℹ️ 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".
| continue | ||
| hits += 1 | ||
| precision_sum += hits / rank | ||
| denominator = min(len(relevant), cutoff) if cutoff is not None else len(relevant) |
There was a problem hiding this comment.
When Ranx is unavailable, _average_precision computes map@k with min(len(relevant), k) as the denominator, which inflates scores versus Ranx semantics. For example, with 20 relevant docs and a perfect top-10 ranking, this fallback returns 1.0 while Ranx returns 0.5, so benchmark comparisons become inconsistent exactly in the degraded mode this commit introduced.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
@coderabbitai review |
|
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 332c688399
ℹ️ 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 _is_code_intelligence_relation(existing[1]): | ||
| return target_id |
There was a problem hiding this comment.
When a depends_on row already exists for the same (source_id, target_id) but is not tagged as code-intelligence, this early return skips any reactivation/update even if the dependency is currently observed in the project scan. If that existing row was previously soft-closed (expired_at set), reconciliation leaves the dependency absent from kg_current_facts, producing a false negative in KG/search despite live evidence in code. This is reproducible with an expired manual relation on the same edge.
Useful? React with 👍 / 👎.
|
@coderabbitai review Follow-up pushed for current-head Codex finding: observed expired manual depends_on facts are reactivated without taking code_intelligence ownership. Local validation: targeted red/green, focused KG suite, ruff, full non-live pytest, and pre-push gate passed. |
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 78ca0ac44c
ℹ️ 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".
| conn.execute( | ||
| """UPDATE kg_relations | ||
| SET valid_from = COALESCE(valid_from, ?), | ||
| valid_until = ?, expired_at = NULL | ||
| WHERE id = ?""", |
There was a problem hiding this comment.
When an observed depends_on relation already exists but is not tagged as code-intelligence, this branch reactivates it by clearing expired_at without updating properties to include the code-intelligence source. That makes the fact visible again, but future reconciliation skips it because _expire_stale_dependency_relations only expires relations recognized as code-intelligence-owned, so the relation can become permanently stale after the dependency is removed again.
Useful? React with 👍 / 👎.
|
@coderabbitai review Follow-up pushed for current-head Codex finding: revived expired manual depends_on facts now preserve manual provenance while adding code_intelligence evidence, so later absence scans can expire them again. Local validation: red/green regression, focused KG suite, ruff, full non-live pytest, and pre-push gate passed. |
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
✅ Actions performedReview triggered.
|
|
Codex Review: Didn't find any major issues. Hooray! ℹ️ 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". |
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/eval/benchmark.py (1)
165-187: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueBroad exception silencing in fallback paths may hide root causes.
The fallback mechanism uses bare
except Exception: passin multiple locations (lines 193, 215-216, 243-244, 260-261). While this ensures resilience, it silently swallows all errors including unexpected ones (e.g.,TypeErrorfrom API changes,MemoryError).Consider logging the exception at
DEBUGlevel before falling back, so debugging is possible when the fallback is triggered unexpectedly:♻️ Optional: Add debug logging for suppressed exceptions
+import logging + +_logger = logging.getLogger(__name__) + def _run_from_dict(run_dict: dict[str, dict[str, float]]) -> Run: try: return Run(run=run_dict) except Exception: + _logger.debug("ranx.Run() failed, applying fallback patch", exc_info=True) _patch_ranx_run_fallback() return Run(run=run_dict)Apply similarly to
_build_ranx_qrels,evaluate_pipeline, andcompare_pipelines.🤖 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/eval/benchmark.py` around lines 165 - 187, Replace the bare "except Exception: pass" in the fallback/backup blocks with an explicit exception capture and debug log so failures are visible: in _build_ranx_qrels, evaluate_pipeline, compare_pipelines (and any similar fallback in _patch_ranx_run_fallback) change to "except Exception as e:" and call the module logger at DEBUG (e.g., logger.debug("Suppressed exception in <function_name> fallback", exc_info=True)) before performing the fallback behavior; ensure a module-level logger is imported/created if missing and avoid catching BaseException/KeyboardInterrupt by keeping Exception as the caught type.
🤖 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/eval/benchmark.py`:
- Around line 165-187: Replace the bare "except Exception: pass" in the
fallback/backup blocks with an explicit exception capture and debug log so
failures are visible: in _build_ranx_qrels, evaluate_pipeline, compare_pipelines
(and any similar fallback in _patch_ranx_run_fallback) change to "except
Exception as e:" and call the module logger at DEBUG (e.g.,
logger.debug("Suppressed exception in <function_name> fallback", exc_info=True))
before performing the fallback behavior; ensure a module-level logger is
imported/created if missing and avoid catching BaseException/KeyboardInterrupt
by keeping Exception as the caught type.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 51b85acc-2ee1-415b-909e-ea00d703359b
📒 Files selected for processing (6)
src/brainlayer/eval/benchmark.pysrc/brainlayer/pipeline/code_intelligence.pysrc/brainlayer/vector_store.pytests/test_code_intelligence.pytests/test_eval_framework.pytests/test_kg_schema.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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
**/*.py: Usepaths.py:get_db_path()for all database path resolution; all scripts and CLI must use this function rather than hardcoding paths
When performing bulk database operations: stop enrichment workers first, checkpoint WAL before and after, drop FTS triggers before bulk deletes, batch deletes in 5-10K chunks, and checkpoint every 3 batches
Files:
src/brainlayer/vector_store.pytests/test_kg_schema.pytests/test_eval_framework.pysrc/brainlayer/eval/benchmark.pytests/test_code_intelligence.pysrc/brainlayer/pipeline/code_intelligence.py
src/brainlayer/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/brainlayer/**/*.py: Use retry logic onSQLITE_BUSYerrors; each worker must use its own database connection to handle concurrency safely
Classification must preserveai_code,stack_trace, anduser_messageverbatim; skipnoiseentries entirely and summarizebuild_loganddir_listingentries (structure only)
Use AST-aware chunking via tree-sitter; never split stack traces; mask large tool output
For enrichment backend selection: use Groq as primary backend (cloud, configured in launchd plist), Gemini as fallback viaenrichment_controller.py, and Ollama as offline last-resort; allow override viaBRAINLAYER_ENRICH_BACKENDenv var
Configure enrichment rate viaBRAINLAYER_ENRICH_RATEenvironment variable (default 0.2 = 12 RPM)
Implement chunk lifecycle columns:superseded_by,aggregated_into,archived_aton chunks table; exclude lifecycle-managed chunks from default search; allowinclude_archived=Trueto show history
Implementbrain_supersedewith safety gate for personal data (journals, notes, health/finance); use soft-delete forbrain_archivewith timestamp
Addsupersedesparameter tobrain_storefor atomic store-and-replace operations
Run linting and formatting with:ruff check src/ && ruff format src/
Run tests withpytest
UsePRAGMA wal_checkpoint(FULL)before and after bulk database operations to prevent WAL bloat
Files:
src/brainlayer/vector_store.pysrc/brainlayer/eval/benchmark.pysrc/brainlayer/pipeline/code_intelligence.py
🔇 Additional comments (8)
src/brainlayer/eval/benchmark.py (3)
288-295: LGTM!
327-338: LGTM!
190-196: LGTM!Also applies to: 212-217, 269-286, 297-325, 340-345
tests/test_eval_framework.py (1)
91-141: LGTM!src/brainlayer/vector_store.py (1)
1313-1315: LGTM!tests/test_kg_schema.py (1)
15-15: LGTM!Also applies to: 140-161
src/brainlayer/pipeline/code_intelligence.py (1)
25-28: LGTM!Also applies to: 214-215, 589-610
tests/test_code_intelligence.py (1)
421-424: LGTM!Also applies to: 465-555, 827-839
1 participant
Mandate
Implements the user mandate for PHASE 1 — KG STATEFUL RECONCILIATION on branch
feat/kg-stateful-reconciliation, following the Etan-mandate 2026-05-22 plan and/pr-loopinvariant.Source citations:
brainlayer depends_on fastapiexample:docs.local/plans/2026-05-22-brainlayer-actually-works/phase-1-kg-stateful/scope.md:8-17.docs.local/plans/2026-05-22-brainlayer-actually-works/phase-1-kg-stateful/scope.md:21-32,:48-58.code_intelligence.pyand confirmsbrainlayer -> fastapi depends_onpersisted without validity state:/Users/etanheyman/Gits/orchestrator/docs.local/audits/2026-05-22-issue-1b-fts5-kg-investigation.md:7-17./pr-loop, all 7 checks green, no squash, and brain_store on merge:docs.local/plans/2026-05-22-brainlayer-actually-works/collab.md:38-47.Implementation
kg_entities.expired_at, existing/required validity columns, and expired indexes insrc/brainlayer/vector_store.py:1184-1216.kg_current_factsview compatibility insrc/brainlayer/pipeline/code_intelligence.py:75-104.src/brainlayer/pipeline/code_intelligence.py:239-365.valid_untiland clearsexpired_at; missing code-intelligencedepends_onfacts are soft-expired without deletes insrc/brainlayer/pipeline/code_intelligence.py:368-501.expired_atinsrc/brainlayer/kg_repo.py:168-230,:282-299.brain_searchSQL KG fallback now excludes expired relations/entities and invalid validity windows by default insrc/brainlayer/mcp/search_handler.py:420-430.src/brainlayer/mcp/_shared.py:29-56.src/brainlayer/eval/benchmark.py:165-267.Regression Tests
tests/test_code_intelligence.py:319-368.valid_untiland remains unexpired:tests/test_code_intelligence.py:370-415.brain_searchSQL KG path excludes expired relations:tests/test_search_filter_params.py:19-40.expired_aton entities and expired indexes:tests/test_kg_schema.py:74-135,tests/test_kg_standard.py:153-228.Acceptance
(brainlayer, depends_on, fastapi), remove fastapi, rerun reconcile, and assertexpired_at != NULLplus exclusion fromkg_current_facts.valid_untiland clears/does not setexpired_at.brain_searchdefault SQL KG query excludes expired facts.Validation
expired_at/valid_until, missing expired indexes, and_kg_facts_sqlreturning a soft-closed relation..venv/bin/pytest tests/test_code_intelligence.py tests/test_kg_schema.py tests/test_kg_standard.py tests/test_search_filter_params.py -q→131 passed..venv/bin/ruff check src/ tests/ && .venv/bin/ruff format --check src/ tests/→ passed..venv/bin/pytest tests/ -m "not integration and not live" -x→2139 passed, 9 skipped, 75 deselected, 1 xfailed.Review Requests
@codex review
@cursor review
@BugBot review
Note
Medium Risk
Touches KG reconciliation update logic for existing relations, which could affect provenance metadata and whether edges are considered current/expired across scans.
Overview
Ensures
enrich_projectsupdateskg_relations.propertieswhen a previously expired, non-code_intelligencedepends_onrelation is re-observed, merging provenance so the edge can be revived without taking ownership.Updates the regression test to assert the merged
sourcesprovenance and adds coverage that the revived manual edge is later soft-expired and removed fromkg_current_factswhen the dependency disappears again.Reviewed by Cursor Bugbot for commit 0d1f6ba. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Expire stale entity and relation facts via reconciliation pass in code intelligence
valid_from,valid_until, andexpired_atcolumns tokg_entitiesandkg_relations, with indexes and a rebuiltkg_current_factsview using julianday-based timestamp comparisons in vector_store.py and code_intelligence.py.enrich_projects, dependency relations not observed in the current scan are marked expired; library entities with no remaining active facts are also expired via_expire_stale_dependency_relations.valid_untilrefreshed andexpired_atcleared, preserving provenance and avoiding duplicates.fcntlfallback) prevents concurrent enrichment runs from corrupting the KG.kg_current_factsuntil re-observed.Macroscope summarized 0d1f6ba.
Summary by CodeRabbit
New Features
Bug Fixes
Performance
Tests