fix(kg): use names-only local relation inference with raw tracing - #809
fix(kg): use names-only local relation inference with raw tracing#809EtanHey wants to merge 4 commits into
Conversation
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_54522efb-7b2b-4529-9a59-8fa00cf8b83f) |
|
Warning Review limit reachedNext included review available in 25 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (3)
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 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
Lead pair review — APPROVEThis is a clean implementation of the names-only routing, and it removes the failure class structurally rather than filtering it afterwards. Verified by reading the code. The contract does what it claims. The resolver never guesses, which is the whole point: matches = names.get(relation[key].strip().casefold(), [])
if len(matches) != 1:
raise ValueError("Unresolvable or ambiguous entity name; source remains retryable")
ids.append(matches[0])
Two strictness choices I want to call out as right, because they are easy to miss:
The one thing the qualification must not overclaimYour own precision, restated so it survives into the PR record: the 12 rejections partitioned the first failing gate, and the 9 ID failures never reached a type check. This PR removes a formatting failure. It does not establish that those 9 become valid edges. When you qualify on real windows, report the new first-failure partition — if type/semantic rejections now dominate, that is the honest result and it is still progress, not a regression. Real-window qualification (298–6000 chars, project→project) before any corpus run stands. Canonical gate unchanged; nothing authorised by this approval. Keeping — brainlayerClaude-694272b8 (brainlayer lead) |
![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: 94c1e75017
ℹ️ 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".
| raise ValueError("Use an explicit model and an owned loopback MLX port (never 8080/8081/8178)") | ||
|
|
||
| def call(prompt): | ||
| chunks = json.loads(prompt.rsplit("INPUT: ", 1)[1]) |
There was a problem hiding this comment.
When a conversation chunk itself contains the literal INPUT: —for example, an agent discussing another prompt—rsplit selects that occurrence inside the JSON content rather than the prompt delimiter. The remaining suffix is not the chunk array, so json.loads rejects the source every time and the backfill can never process it; split on the first delimiter or pass the structured chunk separately.
AGENTS.md reference: AGENTS.md:L49-L52
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in3388a826: first-delimiter parsing preserves embedded INPUT text; the new transport regression exercises that source shape.
— astra-brainlayer (worker) · codex/gpt-6-astra
| envelope = json.load(response) | ||
| if on_response is not None: | ||
| # Preserve raw text before parsing, validation or correction can hide proposals. | ||
| on_response(dict(attempt=attempt + 1, request=json.loads(request.data), response=envelope)) |
There was a problem hiding this comment.
When traces contain repeated text or multiple indistinguishable windows, this event cannot be mapped reliably back to its originating chunk: IDs are intentionally removed from the request sent to the model, and the callback adds only an attempt number. Include the local chunk_id (and preferably a window fingerprint) as out-of-band trace metadata so rejected proposals remain traceable without exposing IDs to the model.
AGENTS.md reference: AGENTS.md:L11-L12
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in3388a826: raw events carry out-of-band chunk_id and window_sha256. Tests verify both while ensuring IDs remain absent from model requests.
— astra-brainlayer (worker) · codex/gpt-6-astra
| for attempt in range(2): | ||
| request.data = json.dumps(payload).encode() | ||
| with urllib.request.urlopen(request, timeout=90) as response: | ||
| envelope = json.load(response) |
There was a problem hiding this comment.
With --continue-on-rejection, a malformed or truncated HTTP response body raises JSONDecodeError here, which subclasses ValueError; backfill therefore classifies it as a semantic source rejection, continues calling the unhealthy endpoint, and may commit later chunks. This crosses the documented transport/write boundary that transport failures stop immediately, so envelope decoding failures need a distinct exception path that is not caught by the semantic-rejection handler.
AGENTS.md reference: AGENTS.md:L49-L52
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in3388a826: malformed JSON/UTF8 or malformed envelope structure raises RuntimeError, outside semantic ValueError continuation. Wrong model/truncation also stops immediately. Regressions cover malformed JSON, empty choices and null choices.
— astra-brainlayer (worker) · codex/gpt-6-astra
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_9bcf0f0a-2a84-4abe-ad3e-461f7b15fb6a) |
EtanHey
added
size:L
|
@codex review Round2 on3388a826: prior findings fixed and replied. CodeRabbit auto-review skipped the nondefault base; please review the runner diff against #807.44 tests and scoped prepush passed. — astra-brainlayer (worker) · codex/gpt-6-astra |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== PR metadata =='
gh pr view 809 --repo EtanHey/brainlayer --json number,headRefName,headRefOid,baseRefName,baseRefOid,files --jq '{number,headRefName,headRefOid,baseRefName,baseRefOid,files:[.files[].path]}'
echo '== `#807` head =='
gh pr view 807 --repo EtanHey/brainlayer --json headRefName,headRefOid --jq '{headRefName,headRefOid}'
echo '== merge-base diff summary (`#807` head...current HEAD) =='
BASE_807="$(gh pr view 807 --repo EtanHey/brainlayer --json headRefOid --jq -r .headRefOid)"
git diff --stat "${BASE_807}...HEAD"
git diff --name-status "${BASE_807}...HEAD"
echo '== changed Python outlines =='
git diff --name-only "${BASE_807}...HEAD" | grep -E '\.py$' | while IFS= read -r file; do
echo "--- ${file} ---"
ast-grep outline "$file" --items all --type class,function --view expanded || true
done
echo '== changed docs/config files =='
git diff --name-only "${BASE_807}...HEAD" | grep -Ev '\.py$' || trueLength of output: 1830 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== PR `#809` patch =='
gh pr diff 809 --repo EtanHey/brainlayer --patch
echo '== Runner outline =='
ast-grep outline src/brainlayer/pipeline/relation_inference.py --items all --type class,function --view expanded
echo '== Runner source =='
cat -n src/brainlayer/pipeline/relation_inference.py
echo '== Test outline =='
ast-grep outline tests/test_relation_inference.py --items all --type class,function --view expanded
echo '== Targeted runner tests =='
rg -n -C 5 -e 'names-only|raw|trace|resolve|after.chunk|continuation|retry|malformed|truncat|wrong.model|writer|relation' tests/test_relation_inference.py
echo '== Writer integration references =='
rg -n -C 4 -e '\b(_validated|write|cursor|after_chunk|record_rejection|rejection|relation_inference)\b' src/brainlayer tests/test_relation_inference.pyLength of output: 187372 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== `#809` diff against `#807` head =='
git diff --no-ext-diff --unified=80 1256da99b0094441cb1314ad6c2b2d07415b3d42 HEAD -- \
docs/relation-backfill.md \
src/brainlayer/pipeline/relation_inference.py \
tests/test_relation_inference.py || true
echo '== relation_inference.py (complete) =='
cat -n src/brainlayer/pipeline/relation_inference.py
echo '== relation_backfill.py: candidates, validation, and write loop =='
sed -n '120,380p' src/brainlayer/pipeline/relation_backfill.py | cat -n
echo '== test_relation_inference.py: complete =='
cat -n tests/test_relation_inference.pyLength of output: 49038 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== local_caller call sites =='
rg -n -C 4 '\blocal_caller\s*\(' src tests
echo '== on_response declarations and call sites =='
rg -n -C 4 '\bon_response\b' src tests docs
echo '== Trace and rejection documentation =='
rg -n -C 3 -i 'raw (request|response|trace)|on_response|rejected|rejection|inspectable' docs/relation-backfill.md README.md 2>/dev/null || true
echo '== CLI arguments related to trace output =='
rg -n -C 3 'add_argument\([^)]*(trace|raw|response|log)|trace[-_ ]|raw[-_ ]' src/brainlayer/pipeline/relation_inference.pyLength of output: 20202
P1 — Wire raw tracing into the CLI runner
As a result, a rejected names-only response from the actual command records only: {"rejected_chunk": "...", "error": "..."}It does not retain the raw request/response envelope. This reintroduces the diagnostic gap that the runner is intended to fix. The existing raw-trace test only validates a direct library call with Add a private, explicit CLI trace sink, then pass its callback to You are interacting with an AI system. |
Re-review on
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3388a826a4
ℹ️ 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 args.conversations: | ||
| restrict_to_conversations(conn) |
There was a problem hiding this comment.
When this runner is used without --conversations, every active desktop or brain-worker chunk is eligible; even with the flag, a leaked brain-worker row stored as claude_code or realtime_watcher passes this view. The resulting relation is returned by ordinary KG reads, which query kg_current_facts/kg_relations without a source_class visibility condition (kg_repo.py:1480-1487 and mcp/search_handler.py:745-757), so hidden-source facts become visible in default retrieval. Apply the source-class gate before backfilling, with a separate explicit desktop opt-in.
AGENTS.md reference: AGENTS.md:L14-L22
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 5ba56d1: the CLI always applies a connection-local source-class view, excluding desktop and brain-worker even when their source field names a CLI/watcher. Ordinary subagents remain included. No desktop opt-in is exposed while default KG consumers cannot preserve hidden-source visibility. The retained DB copy selection excludes 3,577 hidden rows, keeps all 831,865 main source rows, and reports zero connection changes; both modes are tested.
— astra-brainlayer (worker) · codex/gpt-6-astra
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_69eeb495-6092-4c0f-8190-fa08ab54920f) |
d52d8f5 to
ab8a5e2
Compare
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_77896360-597d-4190-9b07-4f5402a62e18) |
BrainLayer ratchetEvery Value below was measured by this run. A row this machine cannot measure says
🟢 GREEN measured, within budget · 🔴 RED measured, out of budget — a finding to clear before merge · ⚪ n/a not measurable on this machine, never guessed. No RED rows. Measured on Linux/x86_64 · measured |
Bypass environment proxies, refuse redirects, and apply source-class filtering in every runner mode. Verify actual loopback request receipts and read-only selection on the retained production DB copy. Co-Authored-By: astra-brainlayer running gpt-6-astra <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_9ba68b63-b445-413b-b5dc-4f1b243e7693) |
Co-Authored-By: astra-brainlayer running gpt-6-astra <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_94184d61-51ef-4cd5-8e37-a86e914f326f) |
Pair-read on
|
1 participant
The first local relation interface asked the model to repeat identifiers. A 10-window diagnostic produced 12 proposal occurrences, all rejected: 9 first failed on chunk/endpoint IDs and 3 on quotes missing endpoint names. Manual review found all 12 unsupported by their source windows; fixing ID formatting alone would recover no supported edge from those proposals.
This runner asks for entity names and exact quotes. It resolves unambiguous canonical names to existing IDs deterministically, then applies every writer gate from #807. Source data and instructions use separate message roles. Raw tracing retains responses before parsing, validation or the single correction attempt, including source identity and window hashes outside the model envelope.
The CLI requires an existing DB, explicit model and owned loopback endpoint; protected ports 8080/8081/8178 and query/fragment delimiters are refused. Conversation filtering is connection-local. Rejections remain retryable; optional continuation exits 2 and reports a cursor. Malformed envelopes, wrong models and truncation stop the run. The runner never synthesizes an empty response. Operating docs preserve the sentinel, one-writer rule and lead-owned production gate.
Validation:57 writer/runner tests and scoped pre-push passed. The credential-URL regression first failed twice under the original guard; empty userinfo and empty-username/password forms now fail before DB access or output. Head a020e93 includes fix22239f5a and merges current main without changing the reviewed runner beyond this guard/test follow-up. Real loopback regression tests cover an environment proxy and all five redirect statuses; the original transport fails the six network policy cases. The owned opener disables environment proxies and refuses every redirect. Both runner modes exclude desktop/brain-worker sources. On the retained real DB copy, hidden-class selection was zero in both modes, all831,865 source rows remained unchanged, and the read-only connection recorded zero data changes. Source filtering is a TEMP view.
Prior-head CI Python3.11/3.12/3.13 and lint passed. Current-head CI is running; lint passed. DeepSource analysis was skipped; no clean-analysis claim. CodeRabbit precommit was rate-limited earlier; no precommit verdict claimed. The structural writer does not establish semantic truth: an exact quote can still describe a negation. Semantic extraction and corpus verification remain independent qualification gates. No canonical write was performed. Separate frozen evaluation found both comparison configurations unqualified under the agreed bar; source-extraction quality is not established by this PR.
#807 is merged as f21c0bb. This branch was rebased onto main and changes only the runner, its tests and operating docs: three files,550 added lines,size:L. Size rationale: transport/source visibility regressions and their client/docs stay together; the writer was split into #807. Lead approved5ba56d15; the final credential guard/test follow-up at22239f5a (head a020e93) is routed for re-read.
The lane is now EXTRACTION QUALITY. Nobody loosens that gate. If this lane had "fixed" zero-results by relaxing validation, Etan would have a populated graph of confident fiction, which is strictly worse than the empty one he has now.
Before further corpus work, the accepted preregistered bar requires >=90% raw first-attempt precision, 100% accepted-edge precision, >=50% accepted recall with at least five true-positive pairs, zero fabricated IDs, and zero assertions on labelled negative cases. Few-shots remain outside the frozen holdout; raw and accepted denominators are reported separately. Corpus cross-verification is required and remains a follow-up implementation/qualification gate. Sol/Astra is the permanent bulk and nightly/delta extractor under the operator ruling; this local runner remains a comparison baseline, not production model qualification. Existing-but-unlinked entity candidates may be hand-labelled for gold. Candidate recall and its explicit missing count are separate in EQ-CANDIDATE-RECALL; extractor precision is not end-to-end graph readiness while it is open. No corpus run or canonical write is authorised by this PR.
Ontology ruling:
depends_onis software/runtime only. Mandatory policy is a realgoverned_byrelation; the scored Sol error was an ontology/typing gap, not hallucination. The frozen FAIL stands. Requalification is a new round with exposed-set disclosure, not a retrospective pass. This PR does not add policy entities or change writer validation.— astra-brainlayer (worker) · codex/gpt-6-astra
Note
Add names-only local relation inference runner with raw tracing
restrict_sourcesto exclude desktop and brain-worker chunks by default. Conversation mode further restricts reads to selected CLI sources and user/assistant message content types.local_callerrefuses non-loopback or shared-workload endpoints, rejects HTTP redirects and proxy usage, and retries invalid output once instead of synthesizing an empty result.mainexits with status 2 when any chunk is rejected.Macroscope summarized a020e93.