fix(salvage): #42's catalog-resolution commit and #34's README item, both stranded post-merge - #84
Merged
Merged
Conversation
…both stranded post-merge Both branches were held back from the branch cleanup because their tips carried commits pushed AFTER their PR merged, so "PR merged" did not mean "work landed". Verified per-symbol rather than by diff size — both branches are thousands of lines behind main, so a raw diff conflates stale with unlanded. #42 / commit 4e0d6ae — `adapters.py` catalog resolution. Main has `advertised_models` and NONE of the generalisation around it: `advertised_catalog`, `_advertised_catalog`, `_cached_catalog`, `agy_log_for`, `AGY_LOG_SUFFIX`, `CATALOG_ROUTING_TAGS`, `_catalog_model_id` were all absent. This is learning-loop provenance code (CLAUDE.md 2: "never treat a generic trace model as provider resolution"), and its whole point is that THE CATALOG IS THE AUTHORITY — a label resolves against the ids the CLI actually advertises, with routing TAGS (`auto`, `default`, `cli-default`) refused as non-identities. The commit's own note records that `VENDOR_MODEL_RE` rejects 42 of 204 real cursor ids, so shape-matching an id the CLI itself advertised is both redundant and wrong. Cherry-picked; `adapters.py`, `dispatcher.py` and `ledger_reconcile.py` applied clean. Two conflicts: * `.verify-floor.json` — took main's. A floor is a property of the MERGE RESULT, never carried in from a branch, so it is re-measured below. * `test_feedback_model_provenance.py` — TWO DIFFERENT tests in one region: main's `test_late_sweep_completes_terminal_attempts_never_one_in_flight` (from #63) and the branch's `test_gemini_provenance_reads_the_per_run_log_before_the_conversation_store`. Kept BOTH; they are independent. #34 / commit c1dc9a7 — README item 11 for `evidence_acquisition.py`, which main documented nowhere (zero occurrences). Every factual claim was re-verified against main's code rather than trusted: `capabilities.unblock()` exists; `ORCH_EVIDENCE_ACQUISITION_MAX_FEEDS`/`_MAX_ITEMS` default to 1 and 3; `LIVE_FLAG = "ORCH_EVIDENCE_ACQUISITION"` with SHADOW as the documented default; and the quoted summary line matches the format string verbatim (`feedable {n} / capped {n} / candidates {n} / fed {n}`). It is the drainable-vs-blocking line the latched-gate rule asks for, and it was the only place that reported it. DELIBERATE-BREAK -> REVERT: emptying `CATALOG_ROUTING_TAGS` fires `assert model_id_for_label("cursor", "Auto (default)") is None` in adapters' OWN selftest; reverted clean. Worth recording that `pytest test_feedback_model_provenance.py` did NOT catch that break — the guard is covered by a `--selftest`, not by a test_*.py, which is precisely why `verify.py` is the gate and a pytest subset is not. A redundant pytest test written before checking was dropped. FLOOR 427 -> 428, one new test, note appended not replaced. Verified FRESH-STATE (both ORCH_STATE_DIR and ORCH_LOCAL_RUNTIME at empty dirs, reproducing CI): VERIFIED — 420 passed, 0 failed, 79 selftests, 3/5 gates green, 8 tests + 5 selftests + 2 gates skipped for named prerequisites; 420 + 8 = 428 = floor. ruff + black -l 100 clean. NOT FIXED HERE, and not caused here: on this machine `test_capabilities.py`'s `test_gate_blocks_execution_is_opt_in_and_narrow` and `test_evidence_gate_kind_is_not_blanket_observer` fail on PRISTINE main too — the hourly fleet tick mutated the machine-local ledger and range-lane-rollout now classifies `matched_not_invoked` instead of `deliberately_gated`. Ledger STATE, not code; they skip with a named reason under a fresh ledger, which is what CI uses. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 27 minutes. View limit detailsLimit details: You’ve used the included review currently available. Your 73 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
Comment |
stranske
temporarily deployed
to
agent-standard
August 23, 2026 23:16 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
Contributor
Workflow source neededPR #84 needs either a linked GitHub issue or one valid non-issue Workflow Source before PR metadata automation can manage it safely. Please do one of:
Once a valid source is present, this warning will not be reposted. |
Automated Status SummaryHead SHA: dad66ce
Coverage Overview
Coverage Trend
Top Coverage Hotspots (lowest coverage)
Low Coverage Files (<50.0%)
Updated automatically; will refresh on subsequent CI/Docker completions. Keepalive checklistScopeNo scope information available Tasks
Acceptance criteria
|
Contributor
|
Workflow state fingerprint for Agents Gate Followups. Do not edit. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Workflow Source
Started from:
Automation intent:
Notes:
Companion to #82. Both branches were held out of the branch cleanup because their tips carried commits
pushed after their PR merged — so "PR merged" did not mean "work landed".
Summary
Verified per-symbol, not by diff size: both branches are thousands of lines behind
main, so a rawdiff conflates stale with unlanded.
#42 /
4e0d6ae—adapters.pycatalog resolutionMain had
advertised_modelsand none of the generalisation around it:advertised_catalog/_advertised_catalog/_cached_catalogagy_log_for/AGY_LOG_SUFFIXCATALOG_ROUTING_TAGS/_catalog_model_idThis is learning-loop provenance code — CLAUDE.md §2, "never treat a generic trace model as provider
resolution" — and its point is that the catalog is the authority: a label resolves against the ids
the CLI actually advertises, with routing tags (
auto,default,cli-default) refused asnon-identities. The commit's own note records that
VENDOR_MODEL_RErejects 42 of 204 real cursorids, so shape-matching an id the CLI itself advertised is both redundant and wrong.
Cherry-picked.
adapters.py,dispatcher.py,ledger_reconcile.pyapplied clean. Two conflicts:.verify-floor.json— took main's. A floor is a property of the merge result, never carried infrom a branch, so it is re-measured below.
test_feedback_model_provenance.py— two different tests in one region: main'stest_late_sweep_completes_terminal_attempts_never_one_in_flight(fix(provenance): the late sweep may only complete TERMINAL worker attempts #63) and the branch'stest_gemini_provenance_reads_the_per_run_log_before_the_conversation_store. Kept both — they areindependent.
#34 /
c1dc9a7— README item 11 forevidence_acquisition.pyMain documented this lane nowhere (zero occurrences). Every factual claim re-verified against main's
code rather than trusted:
capabilities.unblock()exists;ORCH_EVIDENCE_ACQUISITION_MAX_FEEDS/_MAX_ITEMSdefault to 1 and 3;
LIVE_FLAG = "ORCH_EVIDENCE_ACQUISITION"with SHADOW as the documented default; andthe quoted summary line matches the format string verbatim (
feedable {n} / capped {n} / candidates {n} / fed {n}). That line is the blocking-vs-drainable pair the latched-gate rule asks for, and this was theonly place reporting it.
Testing
Verified fresh-state — both
ORCH_STATE_DIRandORCH_LOCAL_RUNTIMEat empty dirs, reproducing CI:420 + 8 = 428 = the floor. Floor 427 → 428 for the one new test; note appended, not replaced.
ruffandblack -l 100clean.Deliberate-break → revert
Emptying
CATALOG_ROUTING_TAGSfiresassert model_id_for_label("cursor", "Auto (default)") is Nonein adapters' own selftest; reverted clean.
Worth recording:
pytest test_feedback_model_provenance.pydid not catch that break. The guard iscovered by a
--selftest, not by atest_*.py— which is exactly whyverify.pyis the gate and apytest subset is not. A redundant pytest test written before checking was dropped rather than kept.
Not fixed here, and not caused here
On the owner's machine,
test_capabilities.py'stest_gate_blocks_execution_is_opt_in_and_narrowandtest_evidence_gate_kind_is_not_blanket_observerfail on pristine main as well: the hourly fleettick mutated the machine-local ledger, so
range-lane-rolloutnow classifiesmatched_not_invokedinstead of
deliberately_gated. That is ledger state, not code — both skip with a named reasonunder a fresh ledger, which is what CI uses. Flagging it because it will make any local
verify.pyonmainread red until the ledger settles.Once this lands,
claude/profile-per-agentandclaude/sad-grothendieck-20324bare safe to delete —they were the last two branches with unlanded work.