Insight stream: label gold evidence, collector /v1/insights, ingest token, OKF citation adapter#11
Conversation
CGS evidence-F1 needs reference support facts as typed SourceRefs, not free-text; labels written by the observatory inbox carry them here. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GET /v1/insights extracts full InsightDecisionRecord/InsightLabel payloads from loop_decision_envelope and insight_label rows. Optional HYRULE_COLLECTOR_INGEST_TOKEN bearer check guards POST /v1/trace* (labels feed gate relaxation, so ingest must be authenticatable before those go live). Insight rows are pruned past HYRULE_COLLECTOR_INSIGHT_RETENTION_DAYS (default 180, 0 disables) at startup and daily; durable copies live in the knowledge repo ledger. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Single helper all loops use so okf_concept refs stay joinable on the bare concept id, with the export version carried in tool_versions instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![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: d3b04dd949
ℹ️ 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".
| select(TraceEventRow) | ||
| .where(TraceEventRow.event_type.in_(_INSIGHT_EVENT_TYPES)) | ||
| .order_by(TraceEventRow.id.desc()) | ||
| .limit(min(row_limit, 5000)) |
There was a problem hiding this comment.
When loop or record_type is provided, the database query still caps results to limit * 5 before the Python-side filters below run. In a busy collector where the newest rows are mostly another loop/type or bare envelopes, /v1/insights?loop=noc&limit=200 can return too few items or even an empty list while older matching NOC records exist just beyond the first 1000 rows, which breaks the completeness of the insight stream used for labels/metrics. Push the filters into the query where possible or keep paging until limit matching records have been collected before applying the final cap.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7ec93a5: /v1/insights now keyset-paginates over insight-typed rows (batch 1000, id < cursor) until limit loop/record_type matches are collected or rows are exhausted — no pre-filter cap. Regression test: test_insights_paging_finds_matches_beyond_first_batch (batch patched to 3, older soc rows behind a wall of newer noc rows).
| _AUTHORITY_WORD_TO_TIER = { | ||
| "canonical": "A1", | ||
| "advisory": "A4", | ||
| "evidence": "A3", | ||
| "stale": "A5", | ||
| "disputed": "A5", |
There was a problem hiding this comment.
When an OKF citation arrives with authority: "proposed"—one of the frontmatter words explicitly called out in the comment above—this mapping falls through to None, so source_ref_from_knowledge_citation() drops the authority tier from the resulting SourceRef. That makes proposed-but-still-tiered evidence sort/display as unknown on the wire instead of the intended A4+ bucket; add the missing proposed mapping (for example to A4) alongside advisory.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7ec93a5: added proposed → A4 to the authority word map (the docstring promised it; the map didn't deliver). Test: test_source_ref_authority_proposed_maps_to_a4.
| if not isinstance(record, dict): | ||
| return None |
There was a problem hiding this comment.
When a producer posts an insight_label or loop_decision_envelope TraceEvent with a malformed embedded record (for example an invalid reference_action or loop), the outer TraceEvent still validates because payload is arbitrary, and this code only checks that the embedded value is a dict before returning it from /v1/insights. Since this endpoint is meant to expose InsightLabel/InsightDecisionRecord data for labels and metrics, it can serve records that the committed contracts would reject; validate the dict with the corresponding contract or skip invalid rows before returning them.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7ec93a5: _insight_item now validates the embedded dict against InsightDecisionRecord/InsightLabel and skips rows the contracts reject, returning the normalized dump for valid ones. Test: test_insights_skips_contract_invalid_records.
|
|
||
|
|
||
| def _owner_from_hints(rows: list[dict[str, Any]]) -> InsightLoop | None: | ||
| by_loop = {_loop(row.get("loop")): _hint_text(row) for row in rows} |
There was a problem hiding this comment.
When more than one candidate from the same loop is present, this dict comprehension keeps only the last row's hint text for that loop. If an earlier NOC row carries a hotspot/routing hint but a later NOC row does not, and a SOC candidate is also present without security hints, the hint pass misses NOC entirely and the priority fallback assigns SOC ownership instead of the hinted NOC owner. Combine hint text per loop rather than overwriting it so duplicate candidates cannot erase the evidence used for arbitration.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7ec93a5: _owner_from_hints now accumulates hint text per loop across all candidate rows instead of last-row-wins, so a hint-less duplicate can no longer erase earlier evidence. Test: test_duplicate_loop_candidates_do_not_erase_hints.
|
|
||
| _SOC_HINTS = {"security", "control_drift", "posture", "attack", "abuse", "detection"} | ||
| _NOC_HINTS = {"availability", "hotspot", "alert", "infra", "network", "routing", "disk"} | ||
| _ENGINEERING_HINTS = {"approved_queue", "github_issue", "implementation", "code", "pr"} |
There was a problem hiding this comment.
pr inside unrelated words
Because _matches() does substring checks, the engineering hint pr fires on unrelated text such as proactive_scanner or proposed_action. In a mixed candidate set where NOC has no NOC-specific hint, an engineering candidate whose source contains one of those words is selected over the normal NOC priority even though no PR/code signal is present; use a less ambiguous token such as github_pr/pull_request or word-boundary matching.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7ec93a5: _matches now does whole-token matching ([a-z0-9_]+ tokens ∩ hint set) and the ambiguous pr hint became github_pr/pull_request. Test: test_engineering_pr_hint_requires_whole_token (proactive_scanner/proposed no longer fire).
Same workflow as network-operations (v0.39.0, per-event concurrency groups from #408/#409) on the unprivileged hyrule-public-pr runner, with agent-core-specific review instructions: this repo's highest risk is silent cross-loop wire breakage (non-additive contract changes, schema drift, collector auth/retention weakening, sanitization violations). Model env-pins stay in the workflow because .pr_agent.toml is only read from the default branch. Org prerequisites (operator, one-time): add this repo to the public-pr runner group and to OPENROUTER_API_KEY's selected repositories. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dd3f9a9885
ℹ️ 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".
| for row in rows: | ||
| if _loop(row.get("loop")) == owner: | ||
| action = str(row.get("action_selected") or "stay_silent") | ||
| if action in {"notify", "question", "draft", "stay_silent"}: | ||
| return action |
There was a problem hiding this comment.
When the candidate list contains more than one record for the selected owner loop, this returns the first owner row's action even if it is stay_silent and a later owner row selected notify, question, or draft. In a replay that mixes quiet samples with surfaced decisions for the same fingerprint, the arbiter can set speak_loop to None and suppress an escalation that the owning loop actually chose to surface; reconcile all owner rows or prefer non-silent valid actions before falling back to silence.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in eab0c35: _selected_action_for_owner now reconciles across ALL owner rows and prefers a surfaced choice (notify/question/draft) over quiet samples — a stay_silent sample can no longer mute an escalation the loop actually made. Test: test_owner_prefers_surfaced_action_over_quiet_sample.
| def _selected_action_for_owner(rows: list[dict[str, Any]], owner: InsightLoop) -> str: | ||
| for row in rows: | ||
| if _loop(row.get("loop")) == owner: | ||
| action = str(row.get("action_selected") or "stay_silent") |
There was a problem hiding this comment.
When a caller passes the new LoopDecisionEnvelope contract as a candidate, the selected action lives in its decision field (agent_core/contracts/loop.py:29), not action_selected. This line therefore defaults those envelope candidates to stay_silent, so an owning loop that emitted a notify, question, or draft envelope gets speak_loop=None and its escalation is suppressed; fall back to decision before treating the owner as silent.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in eab0c35: owner-action resolution falls back from action_selected to the envelope's decision field, so raw LoopDecisionEnvelope candidates no longer default to silence. Test: test_owner_action_reads_envelope_decision_field.
/v1/insights now keyset-paginates until the requested number of loop/
record_type matches is collected (filters ran after a row cap before, so
matching records past the newest rows could be silently missed) and
contract-validates embedded records, skipping rows the committed
contracts would reject. The knowledge adapter maps authority word
'proposed' to A4 as its own comment promised. The arbiter aggregates
hint text across duplicate candidates from the same loop (a hint-less
later row could erase an earlier row's evidence) and matches hints as
whole tokens ('pr' fired inside 'proactive'/'proposed'; now github_pr /
pull_request).
Addresses the five inline Codex P2 findings on PR #11.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
1 similar comment
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Re the PR-Agent suggestions: utcnow import — false positive: |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ec93a5828
ℹ️ 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".
| GraphSpecs. | ||
|
|
||
| ● Write(agent_core/contracts/__init__.py) | ||
| ⎿ Wrote 70 lines to agent_core/contracts/__init__.py | ||
|
|
There was a problem hiding this comment.
This new file is a raw local agent transcript; in shared or public repo contexts it exposes internal operational details such as live Vault bootstrap instructions/addressing and later DB/Vault topology, while also adding thousands of lines of unrelated prompt and command history. Please remove the transcript from source and keep only sanitized runbook/documentation content that is intentionally meant to be published.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in eab0c35 (history rewrite + force-push): the transcript, two local plan docs, and uv.lock were swept into the style commit by an unscoped git add -A — the tip commit was rewritten so none of them exist anywhere on the branch now. Caveat noted honestly: the pre-rewrite commits (7ec93a5/e0aed51) remain fetchable by bare SHA on GitHub until GC; if the transcript content warrants it, a support request can purge them explicitly.
|
|
||
| on: | ||
| pull_request: | ||
| types: [opened, synchronize, reopened, ready_for_review] |
There was a problem hiding this comment.
When a PR is updated with new commits, this workflow now starts because synchronize is listed here, but PR-Agent's GitHub Action docs say synchronize processing is opt-in via github_action_config.pr_actions or handle_push_trigger; the new .pr_agent.toml only sets the auto flags, so push-triggered runs will be skipped instead of posting the review that the workflow comments/concurrency are designed around.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Working as intended, so no change: PR-Agent skipping bare synchronize pushes is the documented fleet behavior (network-operations/docs/ci/pr-agent.md: "auto-review fires on opened/reopened/ready_for_review — to re-run, close and reopen; a bare synchronize push is intentionally skipped"). The trigger stays in the workflow for parity with network-operations/noc-agent, for the concurrency-group semantics, and so a future github_action_config.pr_actions opt-in needs no workflow change.
| if _matches(" ".join(by_loop.get("engineering", [])), _ENGINEERING_HINTS): | ||
| return "engineering" | ||
| return None |
There was a problem hiding this comment.
When a knowledge candidate for context, citation, memory, or learning-ledger work is present alongside a generic NOC/SOC/engineering row with no recognized hints, this function returns None because it only checks SOC/NOC/engineering hints. The later priority fallback then assigns ownership to NOC/SOC ahead of knowledge, so Knowledge never owns those duplicate events unless it is the only candidate, contradicting the rationale below and causing the wrong loop to speak.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in eab0c35: added _KNOWLEDGE_HINTS (context/context_pack/citation/memory/learning_ledger/knowledge_gap/promotion), checked after engineering and before the priority fallback — knowledge now owns its hinted duplicates as the rationale promised. Test: test_knowledge_hints_grant_knowledge_ownership.
| action_space: list[InsightAction] = Field(default_factory=_default_action_space) | ||
| action_selected: InsightAction |
There was a problem hiding this comment.
For records from policies that narrow action_space, Pydantic validates action_space and action_selected independently, so a payload such as action_space=["notify"] with action_selected="draft" is accepted. Since this contract uses the action space as the replay/learning denominator, impossible decisions will be stored and surfaced instead of being rejected at ingest.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in eab0c35: InsightDecisionRecord gains a model_validator rejecting action_selected outside a (policy-narrowed) action_space at ingest; the default full space and narrowed-but-consistent records validate unchanged. Test: test_action_selected_outside_action_space_rejected. Schemas re-exported (validators don't alter the JSON schema).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Svaag
force-pushed
the
feat/insight-collector
branch
from
7ec93a5 to
e0aed51
Compare
Arbiter: the owner's action now reconciles across all of its candidate rows and prefers a surfaced choice (notify/question/draft) over quiet samples, reads raw LoopDecisionEnvelope candidates via their decision field instead of defaulting them to silence, and knowledge hints (context/citation/memory/learning_ledger/knowledge_gap/promotion) grant knowledge ownership before the priority fallback hands its work to NOC/SOC. Contract: InsightDecisionRecord rejects an action_selected outside a policy-narrowed action_space at ingest (the space is the replay/learning denominator; impossible decisions must not be stored). Also expunged from the branch (git history rewrite, tip commit only): a local session transcript, two local plan docs, and uv.lock that an unscoped git add swept into the style commit — none were meant for a public repo. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Svaag
force-pushed
the
feat/insight-collector
branch
from
e0aed51 to
eab0c35
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eab0c35e8c
ℹ️ 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".
| # promises contract-valid records to labels/metrics consumers, so rows the | ||
| # committed contracts would reject are skipped rather than served. | ||
| try: | ||
| record = model.model_validate(record).model_dump(mode="json") |
There was a problem hiding this comment.
When an embedded insight/label omits created_at (which TraceEvent payloads can do), this read path validates the raw dict and lets the model's default_factory=utcnow populate created_at at GET time. Re-reading the same stored row then returns a different record timestamp and can make downstream label/metric exports treat old decisions as freshly created; seed created_at from the stored event/received_at before validation or reject rows missing it.
Useful? React with 👍 / 👎.
| def _selected_action_within_space(self) -> InsightDecisionRecord: | ||
| # action_space is the replay/learning denominator; a decision outside a | ||
| # policy-narrowed space is impossible and must be rejected at ingest. | ||
| if self.action_space and self.action_selected not in self.action_space: |
There was a problem hiding this comment.
The current guard still skips membership validation when a producer sends action_space=[], so action_selected is accepted even though no action is in the replay/learning denominator. For policies that accidentally serialize an empty narrowed space, /v1/insights will surface an impossible decision as contract-valid; reject empty spaces or remove the truthiness check before comparing.
Useful? React with 👍 / 👎.
| TraceEventRow.event_type.in_(_INSIGHT_EVENT_TYPES), | ||
| TraceEventRow.received_at < cutoff, |
There was a problem hiding this comment.
When a producer emits a loop_decision_envelope without the embedded insight_decision_record (a shape /v1/insights explicitly skips), this delete still removes it after 180 days solely because of its event type. That silently drops ordinary trace/run history that has no durable insight copy, despite the retention behavior being scoped to insight rows; add predicates for the embedded decision/label before deleting.
Useful? React with 👍 / 👎.
| speak_loop: InsightLoop | None = None | ||
| selected_action: InsightAction = "stay_silent" |
There was a problem hiding this comment.
When callers instantiate the exported CrossLoopArbiterDecision contract directly, Pydantic accepts contradictory records such as selected_action="stay_silent" with speak_loop="noc", or a speaker that is not the owner. Consumers that use speak_loop to suppress duplicates can then disagree with consumers that use selected_action; add a model validator requiring no speaker for silence and the owner/candidate speaker for surfaced actions.
Useful? React with 👍 / 👎.
| evidence_precision: float | None = None | ||
| evidence_recall: float | None = None |
There was a problem hiding this comment.
When a label or decision record carries out-of-range evidence scores (for example -0.2 or 1.4), the contract accepts it and /v1/insights will surface it as valid. Because these fields feed CGS/F1-style evidence metrics, one bad producer can corrupt aggregate gate decisions; constrain both fields with ge=0 and le=1.
Useful? React with 👍 / 👎.
| learning_allowed: bool = True | ||
| never_learn: bool = False |
There was a problem hiding this comment.
When a caller sets never_learn=True but leaves learning_allowed at its default True, this contract accepts a governance object with contradictory learning policy. Any downstream code that gates on only one of those fields can either learn from data explicitly marked never-learn or unnecessarily block safe learning, so the model should reject never_learn unless learning_allowed is also false.
Useful? React with 👍 / 👎.
| ref: str | ||
| kind: str | None = None | ||
| evidence_id: str | None = None | ||
| sensitivity_class: str | None = None |
There was a problem hiding this comment.
When a producer attaches a SourceRef with a mistyped sensitivity such as sensitve or public-data, Pydantic and the generated schema accept it because this new field is just str. Evidence references are the per-source place consumers can use to avoid displaying or learning from sensitive material, so accepting unknown labels makes those controls unreliable; use the same SensitivityClass literal as GovernanceControls.
Useful? React with 👍 / 👎.
1 participant
User description
Context
First PR of the agentic-loops v-next program (arXiv 2605.06717 grounding): every loop's surface/withhold decisions become an auditable insight stream, the OKF corpus becomes the evidence substrate those insights cite, and operator labels feed IDQ/CGS metrics that justify graduated gate relaxation. This PR is the shared-contract prerequisite — merge first, then tag (suggest v0.8.0) so the four consumer repos can re-pin from their temporary rev pins to the tag.
Includes the pre-existing insight/loop-governance contract commits from
codex/loop-engineering-insight-contract(this branch stacks on it).Changes
InsightLabel.evidence_refs: list[SourceRef]— gold evidence for CGS F1 (additive; schemas regenerated).GET /v1/insights?loop=&record_type=&since=&limit=extracts fullInsightDecisionRecord/InsightLabelpayloads fromloop_decision_envelope/insight_labelrows; optionalHYRULE_COLLECTOR_INGEST_TOKENbearer check onPOST /v1/trace*(labels drive gate relaxation, so ingest must be authenticatable before observatory labels go live); insight-row retention prune (HYRULE_COLLECTOR_INSIGHT_RETENTION_DAYS, default 180d, 0 disables) at startup + daily.agent_core/adapters/knowledge.py::source_ref_from_knowledge_citation— the single mapping every loop uses to cite OKF concepts (refs joinable on bare concept id; export version travels intool_versions).Verification
uv run pytest60 passed; ruff + mypy clean. New tests cover the endpoint (filters, since validation, skip-bare-envelopes), token enforcement incl. reads-stay-open, and retention prune (insight rows only, 0-disables).🤖 Generated with Claude Code
PR Type
Enhancement, Tests
Description
Add insight-policy, loop-decision, governance, and arbiter contracts
Add collector /v1/insights endpoint with filtering and ingest token auth
Add OKF citation adapter for evidence refs
Add deterministic cross-loop arbitration
Add tests for new endpoints, contracts, and arbiter
Onboard PR-Agent CI workflow and config
Diagram Walkthrough
flowchart LR subgraph Loops NOC[NOC Loop] SOC[SOC Loop] ENG[Engineering Loop] KNOW[Knowledge Loop] end Loops --"trace events"--> Collector Collector --"GET /v1/insights"--> InsightStream["Insight Stream API"] Collector --"ingest token auth"--> Auth["Bearer Check"] Arbiter["Cross-Loop Arbiter"] --> Decision["Owner Assignment"] KnowledgeAdapter["Knowledge Adapter"] --> SourceRef["SourceRef Mapping"]File Walkthrough
4 files
Version bump to 0.7.0Add PR-Agent advisory review workflowAdd PR-Agent configurationUpdate project version to 0.7.017 files
Add source ref mapping from knowledge citationsAdd deterministic cross-loop arbitration logicAdd insight stream endpoint, ingest token, and retention pruneExport new insight, governance, and loop contractsExtend SourceRef with evidence_id, raw_ref, sensitivity_classAdd governance controls for loop decisionsAdd insight decision record and label contractsAdd loop decision envelope and arbiter decision contractsAdd schema for cross-loop arbiter decisionUpdate SourceRef in DecisionPacket schemaUpdate SourceRef in EvidencePacket schemaAdd schema for governance controlsAdd schema for insight decision recordAdd schema for insight labelAdd schema for insight scoreAdd schema for loop decision envelopeUpdate SourceRef schema with new fields7 files
Add tests for knowledge citation source ref mappingAdd tests for insights endpoint, token, and retention pruneAdd test for invalid insight action rejectionAdd roundtrip tests for new contractsAdd tests for loop decision and arbiter contractsAdd test for insight decision defaultsAdd tests for cross-loop arbiter logic1 files
Update scope to include arbitration