Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions reviewer/noema_reviewer/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

from .agent import PydanticAIReviewAgent, ReviewAgent, build_agent
from .manifest import ReviewManifest
from .models import Confidence, Finding, ReviewVerdict, Severity, Verdict
from .models import Finding, ReviewVerdict, Severity, Verdict
from .patch_image_validation import (
DockerPatchValidatorImageRunner,
PatchValidatorImageProfile,
Expand All @@ -32,7 +32,6 @@


__all__ = [
"Confidence",
"DockerPatchValidationRunner",
"DockerPatchValidatorImageRunner",
"Finding",
Expand Down
21 changes: 13 additions & 8 deletions reviewer/noema_reviewer/agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,12 @@
"pull request: its diff, changed-file context, workflow logs, SARIF "
"summary, dependency findings, prior review comments, and current check "
"conclusions. Judge correctness, security, maintainability, and behavioral "
"regressions from that evidence only. Approve when no blocking issue is "
"regressions from that evidence only. Approve only when no unresolved finding is "
"supported by the evidence. Use request_changes only for concrete, "
"evidence-backed blocking issues, and cite the log, SARIF, test, or source "
"line for each finding. Use blocked when required evidence is missing rather "
"than guessing. Never approve while an unresolved MEDIUM-or-higher "
"dependency finding is present; require a package bump instead."
"than guessing. Never approve while any unresolved dependency or scanner "
"finding is present; require remediation evidence instead."
)


Expand Down Expand Up @@ -101,13 +101,15 @@ def build_prompt(manifest: ReviewManifest) -> str:
class PydanticAIReviewAgent:
"""A ``ReviewAgent`` backed by a PydanticAI ``Agent`` with a typed verdict."""

def __init__(self, model: Model | str) -> None:
"""Build the agent around an injected model (a real model or a test model)."""
def __init__(self, model: Model | str, *, zdr_only: bool = False) -> None:
"""Build the agent without local retries and with exact request privacy policy."""
model_settings = {"extra_body": {"zdr_only": True}} if zdr_only else None
self._agent: Agent[None, ReviewVerdict] = Agent(
model,
output_type=ReviewVerdict,
system_prompt=SYSTEM_PROMPT,
retries=3,
retries=0,
model_settings=model_settings,
)

def review(self, manifest: ReviewManifest, *, strict: bool = False) -> ReviewVerdict:
Expand All @@ -125,5 +127,8 @@ def build_agent(config: ReviewerConfig | None = None) -> PydanticAIReviewAgent:
fails loudly when the model provider or credential is unavailable — the
reviewer never degrades to a silent approval.
"""
model = resolve_model(config)
return PydanticAIReviewAgent(model)
from .config import resolve_config

resolved = config or resolve_config()
model = resolve_model(resolved)
return PydanticAIReviewAgent(model, zdr_only=resolved.zdr_only)
108 changes: 43 additions & 65 deletions reviewer/noema_reviewer/config.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,11 @@
"""Reviewer model/credential resolution.

Per the repo ``AGENTS.md`` rule, secrets are not read ad hoc from the process
environment at runtime: they come from a KV / credential registry. This module
centralises that read into one place. A ``credential_getter`` (the KV) is the
source of truth; the process environment is only the bootstrap *transport* the
CI step uses to hand secrets to the KV, so the env fallback is explicit and
documented rather than scattered ``os.getenv`` reads.

The reviewer talks to an OpenAI-compatible endpoint (the
``contextual-orchestrator`` gateway in production). Upstream model selection
stays in that gateway; leftover sequential ``NOEMA_FALLBACK_*`` settings fail
closed instead of trying the next model inside Noema.
"""Reviewer model/credential resolution for the exact orchestrator/free pool.

Secrets are resolved from the credential registry with environment variables as
bootstrap transport only. Noema does not allocate provider retries, wall-clock
model attempt budgets, candidate fallbacks, or a second routing policy. Those
choices belong to ContextualWisdomLab/contextual-orchestrator. Private-target
privacy is a trusted workflow-derived boolean and is forwarded as request-level
``zdr_only`` evidence rather than inferred from model/provider names.
"""

from __future__ import annotations
Expand All @@ -25,17 +20,21 @@

CredentialGetter = Callable[[str], str | None]
_LOOPBACK_MODEL_HOSTS = frozenset({"localhost", "127.0.0.1", "::1"})
_FREE_ROUTING_ALIAS = "orchestrator/free"
_LEGACY_ATTEMPT_CONTROLS = (
"NOEMA_LLM_REQUEST_TIMEOUT_SECONDS",
"NOEMA_LLM_MAX_RETRIES",
)


@dataclass(frozen=True)
class ReviewerConfig:
"""Resolved settings for a production review agent."""
"""Resolved settings for one production review request."""

model_name: str
base_url: str
api_key: str
request_timeout_seconds: float = 5400.0
max_retries: int = 1
zdr_only: bool = False


def _read(name: str, credential_getter: CredentialGetter | None) -> str:
Expand All @@ -47,39 +46,32 @@ def _read(name: str, credential_getter: CredentialGetter | None) -> str:
return (os.environ.get(name) or "").strip()


def _bounded_int(
name: str,
default: int,
minimum: int,
maximum: int,
credential_getter: CredentialGetter | None,
) -> int:
"""Read a bounded integer setting and fail with a non-secret reason."""
raw = _read(name, credential_getter)
if not raw:
return default
try:
value = int(raw)
except ValueError as exc:
raise RuntimeError(f"{name} must be an integer") from exc
if not minimum <= value <= maximum:
raise RuntimeError(f"{name} must be between {minimum} and {maximum}")
return value
def _read_zdr_policy(credential_getter: CredentialGetter | None) -> bool:
"""Parse the trusted workflow-derived request privacy policy exactly."""
raw = _read("NOEMA_LLM_ZDR_ONLY", credential_getter)
if raw in ("", "false"):
return False
if raw == "true":
return True
raise RuntimeError("NOEMA_LLM_ZDR_ONLY must be exactly true or false")


def _require_single_routing_alias(name: str, value: str) -> None:
"""Reject sequential candidate lists and direct-provider model prefixes."""
if any(character.isspace() for character in value) or "," in value:
def _reject_legacy_attempt_controls(credential_getter: CredentialGetter | None) -> None:
"""Fail closed if Noema-local model timeout/retry allocation is reintroduced."""
configured = [name for name in _LEGACY_ATTEMPT_CONTROLS if _read(name, credential_getter)]
if configured:
raise RuntimeError(
f"{name} must be one routing alias; sequential model candidates are not allowed"
)
if value.startswith(("nvidia-nim/", "openai/", "github-models/")):
raise RuntimeError(
f"{name} must be the contextual-orchestrator routing alias, "
"not a direct provider model"
", ".join(configured)
+ " is not allowed; model attempt allocation belongs to contextual-orchestrator"
)


def _require_single_routing_alias(name: str, value: str) -> None:
"""Require the exact governed free-pool alias and reject local routing choices."""
if value != _FREE_ROUTING_ALIAS:
raise RuntimeError(f"{name} must equal {_FREE_ROUTING_ALIAS}")


def _require_safe_model_endpoint(name: str, value: str) -> None:
"""Reject credential-bearing model endpoints that use unsafe remote transport."""
try:
Expand All @@ -95,20 +87,12 @@ def _require_safe_model_endpoint(name: str, value: str) -> None:


def resolve_config(credential_getter: CredentialGetter | None = None) -> ReviewerConfig:
"""Resolve reviewer configuration from the KV getter or env transport.

Raises:
RuntimeError: when the model name, base URL, or API key is not
configured, so a misconfiguration fails loudly instead of letting
the reviewer silently skip its verdict.
"""
"""Resolve the exact free-pool reviewer configuration and fail closed on drift."""
model_name = _read("NOEMA_LLM_MODEL", credential_getter)
base_url = _read("NOEMA_LLM_API_URL", credential_getter)
api_key = _read("NOEMA_LLM_API_KEY", credential_getter)
request_timeout_seconds = _bounded_int(
"NOEMA_LLM_REQUEST_TIMEOUT_SECONDS", 5400, 60, 7200, credential_getter
)
max_retries = _bounded_int("NOEMA_LLM_MAX_RETRIES", 1, 0, 8, credential_getter)
_reject_legacy_attempt_controls(credential_getter)
zdr_only = _read_zdr_policy(credential_getter)
leftover_fallback = [
name
for name in (
Expand Down Expand Up @@ -137,26 +121,20 @@ def resolve_config(credential_getter: CredentialGetter | None = None) -> Reviewe
raise RuntimeError(
"Noema sequential model fallback is not allowed; unset "
+ ", ".join(leftover_fallback)
+ ". contextual-orchestrator selects min-cost / max-performance."
+ ". contextual-orchestrator owns routing."
)
_require_single_routing_alias("NOEMA_LLM_MODEL", model_name)
_require_safe_model_endpoint("NOEMA_LLM_API_URL", base_url)
return ReviewerConfig(
model_name=model_name,
base_url=base_url,
api_key=api_key,
request_timeout_seconds=float(request_timeout_seconds),
max_retries=max_retries,
zdr_only=zdr_only,
)


def resolve_model(config: ReviewerConfig | None = None) -> Model:
"""Build an OpenAI-compatible PydanticAI model from resolved configuration.

The reviewer routes every model call through an OpenAI-compatible endpoint
(the ``contextual-orchestrator`` gateway in production), so the OpenAI
provider is a required dependency rather than an optional extra.
"""
"""Build one OpenAI-compatible gateway model with no Noema-local retries/time budget."""
from openai import AsyncOpenAI
from pydantic_ai.models.openai import OpenAIChatModel
from pydantic_ai.providers.openai import OpenAIProvider
Expand All @@ -168,8 +146,8 @@ def resolve_model(config: ReviewerConfig | None = None) -> Model:
client = AsyncOpenAI(
base_url=resolved.base_url,
api_key=resolved.api_key,
timeout=resolved.request_timeout_seconds,
max_retries=resolved.max_retries,
timeout=None,
max_retries=0,
)
return OpenAIChatModel(
resolved.model_name,
Expand Down
Loading
Loading