Skip to content

refactor: protocols audit follow-up (REVIEW + fold pass)#1869

Merged
Aureliolo merged 6 commits into
mainfrom
refactor/issue-1865-protocols-review-fold
May 11, 2026
Merged

refactor: protocols audit follow-up (REVIEW + fold pass)#1869
Aureliolo merged 6 commits into
mainfrom
refactor/issue-1865-protocols-review-fold

Conversation

@Aureliolo
Copy link
Copy Markdown
Owner

Closes #1865.

Resolves the 13 _PrivatePrefixed REVIEW rows and the 2 duplicate-named protocol pairs flagged by the protocols audit.

Summary

5 retained with one-line rationale comments (KEEP):

File Protocol Rationale
api/lifecycle.py _AsyncStartStop Structural seam over the optional synthorg[distributed] JetStreamTaskQueue; consumed by _cleanup_on_failure and _safe_shutdown.
core/state_machine.py _HasValue Generic bound for StateMachine[S]; 4 structural users: TaskStatus, RequestStatus, KanbanColumn, SprintStatus.
persistence/_shared/audit.py IsDuplicate Driver-abstraction predicate; impls in sqlite._shared and postgres.audit_repository.
templates/_inheritance.py _RenderToDictFn Self-referential recursive callback for render_parent_config.
providers/management/_capabilities_mixin.py _ServiceProtocol Narrows the mixin's self-type to the 3 attrs + 3 methods consumed.

5 dead/single-consumer typing seams deleted (REMOVE):

  • engine/quality/graders/heuristic.py _HeuristicGraderBridge, engine/routing/scorer.py _RoutingScorerBridge, templates/model_matcher.py _ModelMatcherBridge: collapsed to bridge: EngineBridgeConfig with a TYPE_CHECKING import so the cycle-breaking property is preserved without the structural seam.
  • engine/coordination/attribution.py _ExecutionResultLike, _AgentRunResultLike: deleted (TYPE_CHECKING-only, runtime code already used getattr()); duck-type contract documented inline at the call site.

2 duplicate-named protocols folded:

  • ConflictDetector: orphan in communication/meeting/conflict_detection.py removed; canonical kept at communication/meeting/protocol.py:41 (5 importers already pointed there).
  • SignalAggregator: meta/signals/protocol.py deleted entirely (0 importers); canonical at meta/protocol.py:38.

Also: annotated _MAX_EVIDENCE_LENGTH (attribution.py) and _MAX_INHERITANCE_DEPTH (_inheritance.py) as Final[int] so they sit cleanly on the no-magic-numbers allowlist. Updated docs/reference/protocols-audit.md with the outcome table.

Test plan

  • uv run ruff check src/ tests/: clean.
  • uv run ruff format src/ tests/: 3712 files unchanged.
  • uv run mypy src/ tests/: clean (3712 source files).
  • uv run python -m pytest tests/ -n 8: 29905 passed, 49 environment-conditional skips.
  • Grep confirmed: no surviving imports of the deleted protocol names anywhere in the repo.

Review coverage

Pre-reviewed by 18 agents (code-reviewer, python-reviewer, security-reviewer, persistence-reviewer, type-design-analyzer, docs-consistency, comment-quality-rot, conventions-enforcer, logging-audit, resilience-audit, async-concurrency-reviewer, issue-resolution-verifier, comment-analyzer, pr-test-analyzer, plus 4 codebase-audit mini-passes: missing-logger, missing-event-constants, missing-state-transition-log, race-conditions). 1 valid finding addressed: stale line numbers in the _AsyncStartStop rationale comment were dropped (the function names alone are stable searchable references).

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 11, 2026

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 11, 2026

Review Change Stack

Walkthrough

This PR executes the protocol cleanup follow-up from issue #1865. It removes two protocol definitions (SignalAggregator and ConflictDetector) and consolidates typing across three modules (heuristic.py, scorer.py, model_matcher.py) by replacing local structural Protocol seams with direct EngineBridgeConfig references imported under TYPE_CHECKING. The attribution.py module's duck-typing scaffolding is simplified by removing Protocol definitions and introducing an explicit Final[int] constant. Structural documentation comments are added to seven files clarifying the intended usage of remaining internal typing seams. The protocol audit document is updated to reflect the post-cleanup classification outcomes and the resolved status from this PR.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and specifically summarizes the main changes: a protocols audit follow-up involving REVIEW-designated private protocols and fold-pass operations.
Description check ✅ Passed The description is comprehensive and directly related to the changeset, explaining the KEEP/REMOVE decisions for private protocols and the folding of duplicates with detailed rationales.
Linked Issues check ✅ Passed All objectives from issue #1865 are addressed: 5 protocols retained with rationale comments, 5 deleted/collapsed to concrete types, 2 duplicate pairs folded with canonical selections, and documentation updated.
Out of Scope Changes check ✅ Passed All changes are directly scoped to issue #1865 objectives: removing protocols, adding rationale comments, updating documentation, and consolidating duplicates without unrelated modifications.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 40.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@Aureliolo Aureliolo temporarily deployed to cloudflare-preview May 11, 2026 18:20 — with GitHub Actions Inactive
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented May 11, 2026

Merging this PR will not alter performance

✅ 33 untouched benchmarks
⏩ 21 skipped benchmarks1


Comparing refactor/issue-1865-protocols-review-fold (89d975b) with main (dd1eebc)

Open in CodSpeed

Footnotes

  1. 21 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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 `@docs/reference/protocols-audit.md`:
- Around line 40-46: The doc uses hard-coded numeric counts in
docs/reference/protocols-audit.md; replace those literal numbers with
runtime-stat markers that pull values from data/runtime_stats.yaml using the
<!--RS:NAME--> pattern. Locate the table rows (e.g., the "Protocol classes
inventoried", "KEEP", "MAKE-RUNTIME-CHECKABLE", "REMOVE candidates", "REVIEW"
cells) and swap each numeric literal for the appropriate marker name (follow
existing marker naming convention or add new keys in data/runtime_stats.yaml),
then ensure the build or doc generation step correctly substitutes
<!--RS:NAME--> markers with values from data/runtime_stats.yaml.
- Line 410: The sentence reporting “11 REVIEW rows” is inconsistent with its
breakdown (“10 `_PrivatePrefixed` … plus the 2 duplicate-named pair REVIEW
rows”) which sums to 12; update the wording so the headline and the breakdown
agree — either change “11 REVIEW rows” to “12 REVIEW rows” or adjust the
breakdown (e.g., “9 `_PrivatePrefixed` … plus the 2 duplicate-named pair REVIEW
rows”) and ensure the rest of the paragraph (the counts for retained protocols
and deletions) remains consistent; update the exact text that mentions “11
REVIEW rows”, the phrase containing “10 `_PrivatePrefixed`”, and the “2
duplicate-named pair REVIEW rows” phrase so all numbers reconcile.
🪄 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: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 81d0f071-2b8d-4c3e-924f-0b0be33cc61e

📥 Commits

Reviewing files that changed from the base of the PR and between dd1eebc and ec29efa.

📒 Files selected for processing (12)
  • docs/reference/protocols-audit.md
  • src/synthorg/api/lifecycle.py
  • src/synthorg/communication/meeting/conflict_detection.py
  • src/synthorg/core/state_machine.py
  • src/synthorg/engine/coordination/attribution.py
  • src/synthorg/engine/quality/graders/heuristic.py
  • src/synthorg/engine/routing/scorer.py
  • src/synthorg/meta/signals/protocol.py
  • src/synthorg/persistence/_shared/audit.py
  • src/synthorg/providers/management/_capabilities_mixin.py
  • src/synthorg/templates/_inheritance.py
  • src/synthorg/templates/model_matcher.py
💤 Files with no reviewable changes (1)
  • src/synthorg/meta/signals/protocol.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). (11)
  • GitHub Check: Build Backend
  • GitHub Check: Build Web Assets (melange)
  • GitHub Check: Lighthouse Site
  • GitHub Check: Type Check
  • GitHub Check: Test (Python 3.14)
  • GitHub Check: Lint
  • GitHub Check: CodSpeed Python benchmarks
  • GitHub Check: Build Preview
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: Analyze (go)
  • GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (5)
src/synthorg/!(persistence)/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Only src/synthorg/persistence/ may import sqlite/psycopg or emit raw SQL.

Files:

  • src/synthorg/providers/management/_capabilities_mixin.py
  • src/synthorg/core/state_machine.py
  • src/synthorg/api/lifecycle.py
  • src/synthorg/engine/coordination/attribution.py
  • src/synthorg/communication/meeting/conflict_detection.py
  • src/synthorg/engine/quality/graders/heuristic.py
  • src/synthorg/templates/_inheritance.py
  • src/synthorg/templates/model_matcher.py
  • src/synthorg/engine/routing/scorer.py
src/synthorg/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

src/synthorg/**/*.py: Configuration precedence: DB > env > YAML > code default via SettingsService/ConfigResolver; no os.environ.get outside startup.
No hardcoded numerics; numerics live in settings/definitions/; allowlist 0/1/-1, HTTP codes, hex masks, powers-of-2, and module-level annotated named constants of the form NAME: int|float|Final|Final[int]|Final[float] = literal. Enforced by scripts/check_no_magic_numbers.py.
Comments explain WHY only; no reviewer citations / issue back-refs / migration framing. Enforced by check_no_review_origin_in_code.py + check_no_migration_framing.py.
No from __future__ import annotations (3.14 has PEP 649). Use PEP 758 except: except A, B: requires parens when binding.
Type hints on public functions; mypy strict. Google-style docstrings. Line length 88; functions <50 lines; files <800 lines.
Errors: <Domain><Condition>Error from DomainError; never inherit Exception/RuntimeError/etc directly. Enforced by check_domain_error_hierarchy.py.
Pydantic v2 frozen + extra="forbid" on API DTOs (Request/Response/Snapshot/Result/Envelope/Status/Info/Summary suffixes); @computed_field for derived; NotBlankStr for identifiers.
Args models at every system boundary; parse_typed() for every external dict ingestion. Enforced by check_boundary_typed.py.
Immutability: use model_copy(update=...) or copy.deepcopy(); deepcopy at system boundaries.
Async: use asyncio.TaskGroup for fan-out/fan-in; helpers catch Exception (re-raise MemoryError/RecursionError).
Clock seam: declare clock: Clock | None = None; tests inject FakeClock. Lifecycle: services own _lifecycle_lock; timed-out stops mark unrestartable.
Untrusted content (SEC-1): use wrap_untrusted() from engine.prompt_safety; use HTMLParseGuard for HTML.
Repository CRUD: use methods save(entity), get(id), delete(id) -> bool, list_items(...), query(...) returning tuples.
Datetime in persistence: use parse_iso_utc / `format_iso_...

Files:

  • src/synthorg/providers/management/_capabilities_mixin.py
  • src/synthorg/persistence/_shared/audit.py
  • src/synthorg/core/state_machine.py
  • src/synthorg/api/lifecycle.py
  • src/synthorg/engine/coordination/attribution.py
  • src/synthorg/communication/meeting/conflict_detection.py
  • src/synthorg/engine/quality/graders/heuristic.py
  • src/synthorg/templates/_inheritance.py
  • src/synthorg/templates/model_matcher.py
  • src/synthorg/engine/routing/scorer.py
src/**/*.py

⚙️ CodeRabbit configuration file

This project uses Python 3.14+ with PEP 758 except syntax: "except A, B:" (comma-separated, no parentheses) is correct and mandatory -- do NOT flag it as a typo or suggest parenthesized form. The "except builtins.MemoryError, RecursionError: raise" pattern is intentional project convention for system-error propagation. When evaluating the 50-line function limit, count only the function body excluding the signature lines, decorators, and docstring. Functions 1-5 lines over due to docstrings or multi-line signatures should not be flagged. Do not suggest extracting single-use helper functions called exactly once -- this reduces readability without improving maintainability.

Files:

  • src/synthorg/providers/management/_capabilities_mixin.py
  • src/synthorg/persistence/_shared/audit.py
  • src/synthorg/core/state_machine.py
  • src/synthorg/api/lifecycle.py
  • src/synthorg/engine/coordination/attribution.py
  • src/synthorg/communication/meeting/conflict_detection.py
  • src/synthorg/engine/quality/graders/heuristic.py
  • src/synthorg/templates/_inheritance.py
  • src/synthorg/templates/model_matcher.py
  • src/synthorg/engine/routing/scorer.py
{README.md,docs/**/*.md}

📄 CodeRabbit inference engine (CLAUDE.md)

Numerics in README and public docs sourced from data/runtime_stats.yaml via <!--RS:NAME--> markers.

Files:

  • docs/reference/protocols-audit.md
docs/**/*.md

📄 CodeRabbit inference engine (CLAUDE.md)

Use d2 for architecture / nested containers, mermaid for flowcharts / sequence / pipelines. Markdown tables for tabular data. D2 theme 200 (Dark Mauve), D2 CLI pinned to v0.7.1 in CI.

Files:

  • docs/reference/protocols-audit.md
🧠 Learnings (1)
📚 Learning: 2026-05-05T09:04:46.195Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1760
File: scripts/_dual_backend_parity_lib.py:215-216
Timestamp: 2026-05-05T09:04:46.195Z
Learning: This repository targets Python 3.14+ and follows PEP 758. Therefore, reviewer tooling should NOT treat unparenthesized multi-exception `except` clauses written without an `as` clause (e.g., `except MemoryError, RecursionError:`) as syntax errors. Only flag `except`-clause problems when they are genuinely invalid for Python 3.14+.

Applied to files:

  • src/synthorg/providers/management/_capabilities_mixin.py
  • src/synthorg/persistence/_shared/audit.py
  • src/synthorg/core/state_machine.py
  • src/synthorg/api/lifecycle.py
  • src/synthorg/engine/coordination/attribution.py
  • src/synthorg/communication/meeting/conflict_detection.py
  • src/synthorg/engine/quality/graders/heuristic.py
  • src/synthorg/templates/_inheritance.py
  • src/synthorg/templates/model_matcher.py
  • src/synthorg/engine/routing/scorer.py
🪛 LanguageTool
docs/reference/protocols-audit.md

[style] ~410-~410: Consider using “who” when you are referring to a person instead of an object.
Context: ...` rationale comment naming the consumer that justifies the seam; the 7 deletions col...

(THAT_WHO)

🔇 Additional comments (11)
src/synthorg/api/lifecycle.py (1)

86-87: Comment rationale is clear and appropriately scoped.

This documents the seam intent and local usage without introducing review-origin or migration-framing text.

src/synthorg/persistence/_shared/audit.py (1)

160-161: Good rationale annotation for the protocol seam.

This clarifies why IsDuplicate exists and where its concrete predicates live, without introducing behavior changes.

src/synthorg/templates/_inheritance.py (2)

8-8: Good typing hardening with Final import.

This change supports explicit immutability typing without introducing runtime impact.


24-25: Rationale comment + named Final[int] constant are well aligned with project rules.

The callback note explains intent, and _MAX_INHERITANCE_DEPTH is now a compliant module-level named numeric constant.

Also applies to: 44-44

src/synthorg/core/state_machine.py (1)

36-37: LGTM! Clear documentation of the protocol's structural role.

The inline comment effectively documents both the purpose of _HasValue (as a generic bound for StateMachine[S]) and its concrete consumers across the codebase. This documentation improves maintainability by making the protocol's scope and usage explicit, which is especially valuable for private typing constructs that might otherwise appear orphaned.

src/synthorg/providers/management/_capabilities_mixin.py (1)

55-56: The retained self-protocol rationale is clear and appropriately scoped.

This explains why the private protocol still exists without reintroducing a concrete service dependency into the mixin.

src/synthorg/engine/coordination/attribution.py (1)

9-10: The constantization and duck-typing documentation both land well.

The named Final[int] keeps the evidence cap explicit, and the inline contract note makes the remaining duck-typed seam easy to reason about.

Also applies to: 37-40, 248-253

src/synthorg/engine/routing/scorer.py (1)

10-10: The concrete bridge typing change is a good simplification.

Switching from_bridge_config to EngineBridgeConfig keeps the seam concrete while the TYPE_CHECKING import preserves the runtime cycle boundary.

Also applies to: 23-26, 92-99

src/synthorg/communication/meeting/conflict_detection.py (1)

15-15: Import cleanup is consistent with the protocol fold.

Dropping the structural-typing import keeps this module focused on the concrete detector implementations it owns.

src/synthorg/engine/quality/graders/heuristic.py (1)

4-5: This bridge-config fold keeps the API simpler without changing behavior.

Using EngineBridgeConfig directly here removes a redundant local seam and still avoids a runtime import cycle.

Also applies to: 19-23, 51-58

src/synthorg/templates/model_matcher.py (1)

10-10: The matcher bridge cleanup is coherent with the rest of the fold pass.

The concrete EngineBridgeConfig type makes the projection path easier to follow, and the deferred import keeps the existing runtime boundary intact.

Also applies to: 24-29, 399-407

Comment thread docs/reference/protocols-audit.md Outdated
Comment on lines +40 to +46
| Total | Snapshot (2026-05-10) | After #1864 (2026-05-11) | After #1865 (2026-05-11) |
|---|---|---|---|
| Protocol classes inventoried | 250 | 248 (2 deleted) | 241 (7 more deleted) |
| KEEP | 193 | 237 (44 audit re-flagged, plus pre-existing 193) | 241 (5 `_PrivatePrefixed` retained with rationale; 1 fold-deletion drops conflict_detection.py's KEEP) |
| MAKE-RUNTIME-CHECKABLE | 0 (no `isinstance` sites surveyed) | 0 | 0 |
| REMOVE candidates (flag-only; must be re-verified at cleanup time) | 46 | 0 (2 deleted, 44 reclassified KEEP) | 0 |
| REVIEW (`_PrivatePrefixed` typing seams; intent inspection needed) | 11 | 11 (unchanged; #1865 owns these) | 0 (5 retained with rationale, 5 deleted, 1 fold-deleted) |
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Replace hard-coded counts with <!--RS:NAME--> runtime-stats markers

These new/updated numeric values are committed as literals in a public docs file. Please source them from data/runtime_stats.yaml via <!--RS:NAME--> markers to prevent drift.

As per coding guidelines, “Numerics in README and public docs sourced from data/runtime_stats.yaml via <!--RS:NAME--> markers.”

Also applies to: 410-434

🤖 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 `@docs/reference/protocols-audit.md` around lines 40 - 46, The doc uses
hard-coded numeric counts in docs/reference/protocols-audit.md; replace those
literal numbers with runtime-stat markers that pull values from
data/runtime_stats.yaml using the <!--RS:NAME--> pattern. Locate the table rows
(e.g., the "Protocol classes inventoried", "KEEP", "MAKE-RUNTIME-CHECKABLE",
"REMOVE candidates", "REVIEW" cells) and swap each numeric literal for the
appropriate marker name (follow existing marker naming convention or add new
keys in data/runtime_stats.yaml), then ensure the build or doc generation step
correctly substitutes <!--RS:NAME--> markers with values from
data/runtime_stats.yaml.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai resolve

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Post @coderabbitai resolve or @coderabbitai approve as a new top-level PR comment. Resolve/approve commands are disabled for review-thread replies.

Comment thread docs/reference/protocols-audit.md Outdated
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request performs a comprehensive cleanup of Protocol definitions across the project, addressing Issue #1865 by removing duplicates and documenting the rationale for retained protocols. Several local protocols used to manage circular dependencies were replaced with direct type hints to EngineBridgeConfig. The reviewer identified a critical runtime issue where these new type hints will cause a NameError because they are imported only under TYPE_CHECKING and are not string-quoted as forward references. Feedback was provided to correct this in the heuristic grader, routing scorer, and model matcher modules.

def from_bridge_config(
cls, bridge: _HeuristicGraderBridge
) -> HeuristicGraderConfig:
def from_bridge_config(cls, bridge: EngineBridgeConfig) -> HeuristicGraderConfig:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

The EngineBridgeConfig type is only imported within a TYPE_CHECKING block. Since this file does not use from __future__ import annotations, Python will attempt to evaluate the type hint at runtime when the class is defined, leading to a NameError. You should wrap the type hint in quotes to treat it as a forward reference.

Suggested change
def from_bridge_config(cls, bridge: EngineBridgeConfig) -> HeuristicGraderConfig:
def from_bridge_config(cls, bridge: "EngineBridgeConfig") -> HeuristicGraderConfig:


@classmethod
def from_bridge_config(cls, bridge: _RoutingScorerBridge) -> RoutingScorerConfig:
def from_bridge_config(cls, bridge: EngineBridgeConfig) -> RoutingScorerConfig:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

The EngineBridgeConfig type is only imported within a TYPE_CHECKING block. Without from __future__ import annotations, this will cause a NameError at runtime during module initialization. Please wrap the type hint in quotes.

Suggested change
def from_bridge_config(cls, bridge: EngineBridgeConfig) -> RoutingScorerConfig:
def from_bridge_config(cls, bridge: "EngineBridgeConfig") -> RoutingScorerConfig:

@classmethod
def from_bridge_config(cls, bridge: _ModelMatcherBridge) -> ModelMatcherConfig:
"""Project the matcher subset out of an ``EngineBridgeConfig``."""
def from_bridge_config(cls, bridge: EngineBridgeConfig) -> ModelMatcherConfig:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

The EngineBridgeConfig type is imported under TYPE_CHECKING to break circular dependencies. However, because from __future__ import annotations is missing, Python evaluates this type hint at runtime, which will trigger a NameError. Wrapping the type in quotes is necessary here.

Suggested change
def from_bridge_config(cls, bridge: EngineBridgeConfig) -> ModelMatcherConfig:
def from_bridge_config(cls, bridge: "EngineBridgeConfig") -> ModelMatcherConfig:

@codecov
Copy link
Copy Markdown

codecov Bot commented May 11, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.90%. Comparing base (dd1eebc) to head (89d975b).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1869   +/-   ##
=======================================
  Coverage   84.90%   84.90%           
=======================================
  Files        1805     1804    -1     
  Lines      105208   105195   -13     
  Branches     9214     9214           
=======================================
- Hits        89324    89321    -3     
+ Misses      13652    13644    -8     
+ Partials     2232     2230    -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Fix the inconsistent breakdown reported on lines 46 and 410: the
snapshot has 11 REVIEW table rows but 12 protocol names because
engine/coordination/dispatcher_types.py:(extras) lists two protocol
names (_ExecutionResultLike, _AgentRunResultLike) in a single row.

Line 46 cell: '5 retained, 5 deleted, 1 fold-deleted' (=11) was wrong;
both ConflictDetector and SignalAggregator are fold-deletes, so the
breakdown is '5 retained, 5 deleted, 2 fold-deleted' = 12 names.

Line 410 prose: '10 _PrivatePrefixed + 2 duplicate-named pair = 11'
was 12, not 11. New phrasing makes the rows-vs-names distinction
explicit and matches the Retained-with-rationale (5) and Deleted (7)
detail tables further down.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
docs/reference/protocols-audit.md (1)

40-46: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Replace literal counts with runtime-stats markers

These updated public-doc numerics are still hard-coded. Please source them from data/runtime_stats.yaml via <!--RS:NAME--> markers to prevent drift across future audit regenerations and follow-up edits.

As per coding guidelines, “Numerics in README and public docs sourced from data/runtime_stats.yaml via <!--RS:NAME--> markers.”

Also applies to: 410-434

🤖 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 `@docs/reference/protocols-audit.md` around lines 40 - 46, Replace the
hard-coded numeric table values in docs/reference/protocols-audit.md (the rows
like "Protocol classes inventoried", "KEEP", "MAKE-RUNTIME-CHECKABLE", "REMOVE
candidates", "REVIEW" shown in the diff) with the runtime-stats markers
<!--RS:NAME--> that pull keys from data/runtime_stats.yaml; use the exact marker
names that correspond to those stats, keep the existing markdown table
cells/formatting, and update the other affected region (lines ~410-434) the same
way so every literal count is replaced by the appropriate <!--RS:...--> marker
referenced in data/runtime_stats.yaml.
🤖 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.

Duplicate comments:
In `@docs/reference/protocols-audit.md`:
- Around line 40-46: Replace the hard-coded numeric table values in
docs/reference/protocols-audit.md (the rows like "Protocol classes inventoried",
"KEEP", "MAKE-RUNTIME-CHECKABLE", "REMOVE candidates", "REVIEW" shown in the
diff) with the runtime-stats markers <!--RS:NAME--> that pull keys from
data/runtime_stats.yaml; use the exact marker names that correspond to those
stats, keep the existing markdown table cells/formatting, and update the other
affected region (lines ~410-434) the same way so every literal count is replaced
by the appropriate <!--RS:...--> marker referenced in data/runtime_stats.yaml.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: f8ca5664-4842-4284-acb2-16a34c256007

📥 Commits

Reviewing files that changed from the base of the PR and between ec29efa and 89d975b.

📒 Files selected for processing (1)
  • docs/reference/protocols-audit.md
📜 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). (11)
  • GitHub Check: Build Backend
  • GitHub Check: Lint
  • GitHub Check: Test (Python 3.14)
  • GitHub Check: Type Check
  • GitHub Check: Build Web Assets (melange)
  • GitHub Check: Lighthouse Site
  • GitHub Check: CodSpeed Python benchmarks
  • GitHub Check: Build Preview
  • GitHub Check: Analyze (python)
  • GitHub Check: Analyze (go)
  • GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (2)
{README.md,docs/**/*.md}

📄 CodeRabbit inference engine (CLAUDE.md)

Numerics in README and public docs sourced from data/runtime_stats.yaml via <!--RS:NAME--> markers.

Files:

  • docs/reference/protocols-audit.md
docs/**/*.md

📄 CodeRabbit inference engine (CLAUDE.md)

Use d2 for architecture / nested containers, mermaid for flowcharts / sequence / pipelines. Markdown tables for tabular data. D2 theme 200 (Dark Mauve), D2 CLI pinned to v0.7.1 in CI.

Files:

  • docs/reference/protocols-audit.md
🪛 LanguageTool
docs/reference/protocols-audit.md

[style] ~410-~410: Consider using “who” when you are referring to a person instead of an object.
Context: ...` rationale comment naming the consumer that justifies the seam; the 7 deletions (5 ...

(THAT_WHO)

@Aureliolo Aureliolo merged commit af33ddb into main May 11, 2026
77 checks passed
@Aureliolo Aureliolo deleted the refactor/issue-1865-protocols-review-fold branch May 11, 2026 19:01
@Aureliolo Aureliolo temporarily deployed to cloudflare-preview May 11, 2026 19:01 — with GitHub Actions Inactive
Aureliolo pushed a commit that referenced this pull request May 12, 2026
<!-- HIGHLIGHTS_START -->
## Highlights

> _AI-generated summary (model: `openai/gpt-4.1-mini` via GitHub
Models). Commit-based changelog below._

### What you'll notice
- Password and secret fields now include an eye-toggle for easier
visibility control.
- Containers running without probes are shown as healthy in the doctor
command.
- Unloaded and missing PR-review agents are restored and available
again.

### What's new
- Gate baseline protection is enhanced to block em-dashes during
writing.

### Under the hood
- Replaced Atlas with yoyo-migrations for persistence management.
- Refactored codebase extensively, including context-bound user
authentication and registry pattern for enums.
- Improved linting by draining magic number usages and tightening mock
and constant checks.
- Updated CI to retry Docker pushes on network timeout errors.
- Updated apko lockfiles for dependency management.

<!-- HIGHLIGHTS_END -->

:robot: I have created a release *beep* *boop*
---


##
[0.8.3](v0.8.2...v0.8.3)
(2026-05-12)


### Features

* harden gate baseline protection + block em-dashes at write time
([#1860](#1860))
([b41f151](b41f151))
* **web:** eye-toggle on every password / secret field
([#1873](#1873))
([9070387](9070387))


### Bug Fixes

* **ci:** retry Docker push on Go net/http deadline + cancellation
errors ([#1877](#1877))
([23a0bfa](23a0bfa))
* **cli:** render running-no-probe containers as healthy in doctor
([#1870](#1870))
([6263795](6263795))
* restore unloaded and missing PR-review agents
([#1875](#1875))
([db004fd](db004fd)),
closes [#1871](#1871)


### Refactoring

* bind authenticated user via ContextVar
([#1858](#1858))
([57ed0b4](57ed0b4))
* code-structure cleanup (sub-tasks D + F + G + H + I)
([#1859](#1859))
([362e5c8](362e5c8))
* convert enum dispatch to registry pattern
([#1854](#1854))
([e90550e](e90550e))
* drain no_magic_numbers baseline to zero via Final hoists
([#1856](#1856) phase 2)
([#1872](#1872))
([ec8109e](ec8109e))
* drain pagination + loop-init + kill-switch baselines
([#1857](#1857))
([#1868](#1868))
([115c3c2](115c3c2))
* **persistence:** replace Atlas with yoyo-migrations
([#1876](#1876))
([1b7e975](1b7e975)),
closes [#1874](#1874)
* protocols audit follow-up (REVIEW + fold pass)
([#1869](#1869))
([af33ddb](af33ddb))
* protocols audit follow-up REMOVE pass
([#1867](#1867))
([dd1eebc](dd1eebc))
* tighten check_mock_spec gate, add mock_of[T], drain baseline
([#1862](#1862))
([240a253](240a253))
* tighten check_no_magic_numbers for named module constants
([#1856](#1856))
([#1866](#1866))
([90c933b](90c933b))


### CI/CD

* update apko lockfiles
([#1863](#1863))
([2bd32e6](2bd32e6))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: synthorg-repo-bot[bot] <279117679+synthorg-repo-bot[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CLEANUP] Protocols audit follow-up: REVIEW + fold pass

1 participant