Wire the insight ledger into daemon/governor/intake; capability success-history CLI#37
Conversation
The three CLI entry points (the systemd/timer surfaces) now map their reports to InsightDecisionRecords: governor decisions route via the existing action mapping (dedup-skipped issues emit nothing, dry-run is withheld_logged, knowledge context pack + export version are cited); daemon cycles map published->draft, needs_triage->question, idle->quiet -interval silence, over_budget/refused_ci/error->withheld silence; intake files as draft and fingerprint-dedupes as explicit silence. Records land in the private JSONL ledger and ship as LoopDecisionEnvelope TraceEvents with the full record beside the envelope. All off unless HYRULE_ENGINEERING_INSIGHT_RECORDS=1. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
decide_policy's Tier-2 branch is dormant because every registry entry ships success_count 0. The new capability-history CLI joins stored decision records (90-day window, newest per issue) with PR terminal state via gh — merged-by-human behind required checks with no revert = success; closed-unmerged or reverted = failure; open/bot-merged = pending — and emits per-capability aggregates plus a proposed registry patch. The patch is evidence for a reviewed promotion PR against the deployed registry (policy_version reliability-governor.tier2-history.v1), never applied automatically. 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: 14b853cb26
ℹ️ 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".
| "--state", | ||
| "merged", | ||
| "--search", | ||
| f"Revert #{pr_number} in:title", |
There was a problem hiding this comment.
When a merged PR is reverted via GitHub's normal revert flow, the generated revert PR title is Revert "<original title>" and the PR reference is in the body (Reverts #N), not the title (see https://github.com/orgs/community/discussions/121757). Because this lookup searches only in:title for Revert #N, those reverted changes are still reported as successes, so the proposed capability counts can satisfy the zero-failure Tier-2 history gate with known-bad work included.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 90e1cc1: _was_reverted now checks both forms — GitHub's revert-button body text ("Reverts <owner>/<repo>#N" in:body) and hand-written "Revert #N" in:title. Test: test_revert_detected_in_github_button_body_form.
| return "none", "", f"pr lookup failed: {exc.__class__.__name__}" | ||
| if not isinstance(rows, list) or not rows: | ||
| return "none", "", "no PR references the issue" | ||
| rows.sort(key=lambda row: str(row.get("number") or 0), reverse=True) |
There was a problem hiding this comment.
When an issue has more than one matching PR and the numbers cross a digit boundary, for example #99 closed and #100 merged, this lexicographic sort puts "99" ahead of "100". The history command then evaluates the wrong PR and can record a failure or pending state instead of the latest success, corrupting the per-capability aggregates used in the registry proposal.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 90e1cc1: candidate PRs sort by int(number) — #100 now beats #99. Test: test_newest_pr_sorts_numerically_not_lexicographically (closed #99 + merged #100 → success on u100).
| "--state", | ||
| "all", | ||
| "--search", | ||
| f"{issue_url} in:body", |
There was a problem hiding this comment.
This query only requires the issue URL to appear anywhere in the PR body. If a later merged PR links the issue as Related: or background context while the actual closing PR is still open or was closed unmerged, this row is treated as the issue outcome even though it never closed the work, inflating or deflating the capability history used for the registry proposal. Verify a closing reference (or use GitHub closing-issue metadata) before counting the PR.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 90e1cc1: the lookup now searches "Closes <issue_url>" in:body — the exact closing line the daemon writes (daemon.py pr_body=f"Closes {item.url}") — so a merged PR that merely mentions the issue as related context is no longer treated as its outcome. Test: test_outcome_search_requires_closing_keyword asserts the search term.
| except Exception: | ||
| return False |
There was a problem hiding this comment.
If the first PR lookup succeeds but this second gh pr list used to check for reverts fails due to rate limits, auth, or host search support, the exception path returns False and the caller records the merged PR as a success. For a history gate that requires no revert, unknown revert state should be pending/none or surfaced as an error; otherwise transient lookup failures can manufacture successful capability history.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 90e1cc1: _was_reverted returns tri-state (None on lookup failure) and the caller maps unknown to pending with 'revert lookup failed; outcome unknown' — transient rate limits can no longer manufacture zero-failure history. Test: test_revert_lookup_failure_is_unknown_not_success.
| return None | ||
| action, sampling = mapped | ||
| issue = report.get("issue") or {} | ||
| issue_ref = str(issue.get("url") or issue.get("issue_id") or "") |
There was a problem hiding this comment.
Normal daemon_once reports pass issue with only repo, number, and title (see daemon.py:867), but this mapper only accepts url or issue_id. In those production daemon cycles, published and triage insight records get an empty issue_ref/evidence_refs, so the ledger and decision envelope cannot be joined back to the GitHub issue that triggered the run. Build a structural ref from repo and number when no URL is present.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 90e1cc1: daemon_report_insight builds https://github.com/{repo}/issues/{number} when the report's issue dict carries only {repo, number, title} (the shape daemon_once actually emits), so published/triage records keep a joinable evidence ref. Test: test_daemon_insight_builds_structural_issue_ref.
The PR-outcome lookup now requires the closing keyword the daemon writes
verbatim ('Closes <issue_url>' in body) so related-context mentions never
count as an issue's outcome, sorts candidate PRs numerically (99 vs 100),
detects both revert forms (GitHub's button writes 'Reverts owner/repo#N'
in the BODY; hand reverts keep 'Revert #N' in the title), and treats a
failed revert lookup as unknown/pending — a zero-failure Tier-2 history
gate must never be satisfied by a rate-limited search. Daemon insight
records now build a structural issue ref from {repo, number} (the shape
daemon_once actually reports) so ledger records stay joinable to the
triggering issue.
Addresses the five inline Codex P2 findings on PR #37.
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. |
Same v0.39.0 workflow as the other loop repos on the unprivileged hyrule-public-pr runner, with repo-specific review instructions. Model env-pins stay in the workflow because .pr_agent.toml is only read from the default branch. Org grants (public-pr runner group + OPENROUTER_API_KEY selection) were applied 2026-07-10. 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. |
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
agent-core#11 squash-merged as 4329ec9 and was tagged v0.8.0; the temporary branch-rev pin (or the stale v0.7.0 tag) is replaced by the durable release tag. Full suite green against the new contract (incl. the action_space ingest validator). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 47e2b7362a
ℹ️ 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".
| return "pending", pr_url, "revert lookup failed; outcome unknown" | ||
| if reverted: | ||
| return "failure", pr_url, "a merged revert references this PR" | ||
| return "success", pr_url, f"merged by {merged_by or 'human'} behind required checks" |
There was a problem hiding this comment.
When GitHub returns a merged PR with mergedBy as null (the PullRequest mergedBy field is nullable in GitHub’s GraphQL schema), merged_by becomes an empty string and this line still records the outcome as a success via merged_by or 'human'. In that deleted-user/unavailable-actor case there is no evidence that the merge was performed by a non-bot human, so the capability-history report can inflate success_count for the Tier-2 zero-failure gate; treat an empty merger as pending/unknown instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4df27f7: a merged PR with mergedBy: null (deleted/unavailable actor) is now pending with 'merger unknown' — no evidence of a human merge means no success toward zero-failure history. Test: test_null_merged_by_is_pending_not_success.
| if not enabled() or not insights: | ||
| return 0 |
There was a problem hiding this comment.
With only the new HYRULE_ENGINEERING_INSIGHT_RECORDS=1 flag enabled, record_insights() writes the JSONL ledger and then calls this function, but this guard immediately returns unless the older HYRULE_ENGINEERING_AGENT_CORE_TRACE flag is also set. That makes the advertised insight decision-envelope TraceEvents silently disappear during the feature flag flip unless operators discover and enable a second flag, so the ledger and trace sink diverge for normal insight-recording deployments.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4df27f7: HYRULE_ENGINEERING_INSIGHT_RECORDS=1 is now a complete opt-in — emit_insight_decision_envelopes accepts it alongside the master trace flag, and _sink_from_env overlays the gate for the build (same pattern as the default-path overlay), so the ledger and trace sink can't silently diverge during the flag flip. The sink still honours {FLAG_ENV}_COLLECTOR_URL/_PATH with the JSONL fallback. Test: test_insight_flag_alone_emits_envelopes (incl. both-flags-off ⇒ 0).
| now = now or datetime.now(UTC) | ||
| history = CapabilityHistory(window_days=window_days, generated_at=now.isoformat()) | ||
| for record in load_decision_records(state_dir, window_days=window_days, now=now): | ||
| if not record.matched_capability: |
There was a problem hiding this comment.
Here every CDR with a matched_capability contributes to the proposed counts regardless of record.risk_tier. The built-in tier2.internal-service-low-risk capability also matches lower-risk domains such as non_prod_tooling, so a streak of human-merged Tier-1 work can raise success_count for that Tier-2 capability and satisfy the decide_policy zero-failure Tier-2 gate without any Tier-2 success evidence; keep separate counts or filter this report to the tier it is meant to unlock.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4df27f7: registry_proposal() counts only risk_tier >= 2 successes toward success_count (the number that unlocks decide_policy's Tier-2 gate) and reports lower_tier_success_count + pending_count separately; evidence rows carry risk_tier. A streak of human-merged Tier-0/1 work can no longer satisfy the Tier-2 gate. Test: test_registry_proposal_counts_only_tier2_successes.
| except Exception as exc: | ||
| return "none", "", f"pr lookup failed: {exc.__class__.__name__}" |
There was a problem hiding this comment.
If the primary gh pr list lookup fails for one issue because of rate limiting, auth, or GitHub search availability, this path records the issue as none. Since registry_proposal() ignores none outcomes and only carries success/failure counts, a capability can still be proposed with enough successes and failure_count: 0 even though some audited issues had unknown PR outcomes that might include closed-unmerged work; surface lookup failures as pending/unknown or fail the report instead of folding them into none.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4df27f7: a failed primary gh pr list lookup now yields pending ('outcome unknown') instead of none, and pending_count is part of the registry proposal — unknown outcomes are visible to the promotion-PR reviewer rather than folded away. Test: test_primary_lookup_failure_is_pending_and_reported.
Parity with the Codex findings on soc-agent#4: payload carries untrusted_loop_text / model_consumption_allowed=false (support facts include LHP-untrusted issue titles) and trace_id falls back to the daemon/governor run id for stream correlation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b9dfd40697
ℹ️ 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".
| sampling_class="withheld_logged" if dry_run else "surfaced", | ||
| why_now="new mined signal filed as loop:candidate" | ||
| + (" (dry-run, nothing filed)" if dry_run else ""), | ||
| issue_ref=str(entry.get("url") or ""), |
There was a problem hiding this comment.
When intake scan files a candidate, this URL is only passed as issue_ref; signal_insight_record() stores that value in budget_context but does not add it to evidence_refs, while _insight_decision_event() builds the TraceEvent/envelope from validated.evidence_refs. With HYRULE_ENGINEERING_INSIGHT_RECORDS enabled, filed intake records therefore emit envelopes that cannot be joined back to the GitHub issue they created (and the dedupe path below has the same problem for the existing issue number), defeating the ledger/IDQ correlation for normal intake runs.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4df27f7: signal_insight_record puts issue_ref first in evidence_refs (kind: github_issue), and the dedupe path builds the structural URL from repo + existing issue number — filed and deduped intake envelopes now join back to their GitHub issues. Test: test_intake_records_carry_issue_evidence_refs.
|
|
||
| on: | ||
| pull_request: | ||
| types: [opened, synchronize, reopened, ready_for_review] |
There was a problem hiding this comment.
For PRs that receive new commits after opening, this workflow now fires on synchronize, but neither .pr_agent.toml nor the env block sets github_action_config.pr_actions or handle_push_trigger. I checked PR-Agent’s GitHub Action docs: adding "synchronize" to pr_actions enables auto tools on new commits, and the documented default omits it; as written, follow-up pushes start the job but skip the advertised review/improve pass, so updated diffs can miss advisory review.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Declining for this PR, same as the two prior rounds (agent-core#11, soc-agent#4): skipping bare synchronize pushes is the documented fleet behavior (network-operations/docs/ci/pr-agent.md — re-run via close/reopen), and this repo stays consistent with network-operations/noc-agent. Noted as a fleet-policy option for the operator: adding "synchronize" to github_action_config.pr_actions in each repo's .pr_agent.toml enables push-refresh reviews everywhere if wanted.
Third-round Codex findings on #37: a merged PR with null mergedBy is now pending (no evidence of a human merge — deleted/unavailable actors must not inflate zero-failure history); primary PR-lookup failures are pending instead of vanishing as 'none'; and registry_proposal counts only tier>=2 successes toward the Tier-2 gate, reporting lower-tier successes and pending outcomes separately so the promotion-PR reviewer sees exactly what the evidence covers. HYRULE_ENGINEERING_INSIGHT_RECORDS=1 is now a complete opt-in for envelope emission (no second master trace flag to discover — the sink honours the same URL/PATH vars with the JSONL fallback). Intake insight records put the filed/deduped issue into evidence_refs (structural URL for the dedupe path) so envelopes join back to the GitHub issue. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4df27f789f
ℹ️ 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".
| rows = [outcome for outcome in self.outcomes if outcome.capability == capability] | ||
| proposal[capability] = { | ||
| "success_count": sum( | ||
| 1 for row in rows if row.outcome == "success" and row.risk_tier >= 2 |
There was a problem hiding this comment.
The current predicate uses risk_tier >= 2, so Tier-3/4 human-handled work with a matched capability still increments the success_count that is later intended to unlock decide_policy's Tier-2 auto-approval gate. For example, a Tier-3 tier2.internal-service-low-risk issue is routed as human-required, but if its PR is merged this report counts it as Tier-2 success evidence even though no Tier-2 outcome was observed; this can satisfy the zero-failure history gate with the wrong risk class. This is fresh evidence beyond the earlier lower-tier issue because the patched predicate still admits higher tiers.
Useful? React with 👍 / 👎.
| now = now or datetime.now(UTC) | ||
| history = CapabilityHistory(window_days=window_days, generated_at=now.isoformat()) | ||
| for record in load_decision_records(state_dir, window_days=window_days, now=now): | ||
| if not record.matched_capability: |
There was a problem hiding this comment.
Only checking matched_capability lets records through even when the Governor matched an envelope but then denied it because expected paths/source/risk exceeded that envelope (decide_policy returns needs_human with the capability in that case). If a human later closes such an out-of-envelope issue, this report can still count it as success evidence for the same capability and raise the proposed Tier-2 history despite the work being explicitly outside the capability bounds; filter to in-envelope candidate/approved decisions before appending outcomes.
Useful? React with 👍 / 👎.
| if report.get("pr_url"): | ||
| budget_context["pr_url"] = report["pr_url"] |
There was a problem hiding this comment.
When the daemon outcome is published, this stores pr_url only in budget_context; daemon_insight_record() builds evidence_refs from the issue ref, and _insight_decision_event() copies only validated.evidence_refs into the LoopDecisionEnvelope. Published draft insights therefore emit envelopes that can be joined to the issue but not to the PR they created unless consumers parse the non-envelope sidecar, which breaks PR-outcome correlation for normal daemon cycles; include the PR as a github_pr evidence ref.
Useful? React with 👍 / 👎.
1 participant
User description
Context
Part of the agentic-loops v-next program. The private insight ledger (
insights.py) existed with zero callers; this wires it into the three production entry points and adds the evidence tooling for the dormant Tier-2 auto-approval branch.Changes
HYRULE_ENGINEERING_INSIGHT_RECORDS=0default), at the CLI entry points the systemd timers run:Records land in the private JSONL ledger and ship as LoopDecisionEnvelope TraceEvents with the full record beside the envelope.
policy_version reliability-governor.tier2-history.v1). The patch is evidence for a reviewed promotion PR against the deployed registry in network-operations, never applied automatically.Verification
uv run pytest249 passed (8 new wiring tests, 3 new history tests); ruff + mypy clean.Depends on AS215932/agent-core#11 only at flag-flip time (pin bump before enabling emission; the writer degrades gracefully today).
🤖 Generated with Claude Code
PR Type
Enhancement
Description
Wire insight ledger into daemon, governor, and intake
Add capability success-history CLI for Tier-2 auto-approval
Add PR-Agent CI workflow and configuration
Diagram Walkthrough
File Walkthrough
4 files
Add insight decision envelope emissionAdd capability success-history evidence moduleWire insights into CLI entry pointsAdd report-to-insight mappings and recording2 files
Add tests for capability historyAdd tests for insight wiring2 files
Add PR-Agent CI workflowAdd PR-Agent configuration