Repository navigation
M4 ultra-low-cost review engine + M5 external dependency discovery and contract registry - #65
Merged
Conversation
…ier budgets Add a PR-level change/risk classifier (patchfrog.change_risk) that sorts a diff into no_ai/tiny/normal/elevated/high_risk from deterministic signals only (docs/comment/generated/lockfile-only, size, cross-module, manifests, CI, public interface, schema, migrations, security-sensitive paths, static findings). Comment detection is conservative and verified exactly with the Python tokenizer when file content is available. With the cost-aware strategy (operator default via PATCHFROG_REVIEW_STRATEGY): - no_ai runs make zero provider calls but are still persisted SUCCEEDED with a reason, and the check run explains the deterministic result; - tiny/normal/elevated/high-risk runs make one single-pass UNIFIED call per candidate batch with shared, de-duplicated context, then escalate sequentially to a Security/Correctness specialist only with a typed reason; the unchanged validation/critic/confidence pipeline decides what survives; - per-tier provider-call ceilings sit on the existing ReviewBudget ledger, with a verification reserve so review work can never starve a mandatory critic call (tiny = one reviewer call plus a critic-only reserve); - tiny/normal runs narrow context and never expand adaptively; - the cost policy fingerprint joins run identity, and --force / force_review bypasses exact-head reuse. Risk tier, signals, escalation reasons, context cost and budget status are persisted (migration 0033), exported in telemetry (schema v12), exposed as Prometheus counters, and summarized by ReviewRunSummary.cost_report(). The pre-M4 specialist fan-out is kept as ReviewStrategy.SPECIALIST_FANOUT and remains the service's behavior when no cost policy is passed.
- patchfrog.evaluation.cost_benchmark + `eval cost-benchmark`: ten
deterministic scenarios (comment/docs-only, tiny, normal bug,
cross-module, auth, schema+migration, public API, test-only, exact-head
repeat) run through the real review service on private in-memory
databases, pre-M4 fan-out vs cost-aware, with deterministic token
estimates of the exact prompts and a synthetic rate card. Committed
result: evaluation_baselines/m4_cost_benchmark.{json,md} -- 23 -> 10
provider calls, every target met, no finding lost. It measures call
shape and cost, never model quality.
- The evaluation runner evaluates the production cost-aware path by
default; `eval run --review-strategy specialist_fanout` keeps the
pre-M4 arm. EvaluationIdentity records the strategy. The oracle answers
for every target in a multi-target prompt. Tests that exercise per-role
provenance now request the specialist strategy explicitly.
- CLI review wires the operator cost policy, adds --force, and prints a
cost line; --dry-run prints the risk tier and reasons.
- docs/cost-aware-review.md documents tiers, budgets, the verification
reserve, escalation reasons and observability.
…racts Add patchfrog.dependencies: given a repository, identify the external APIs/SDKs it depends on from static evidence only -- manifests and lockfiles (PyPI, npm, Go), imports, SDK constructors and call chains (Python via ast, JS/TS via a string-aware lexical scanner), endpoint hosts, environment variable NAMES and local OpenAPI/Swagger specs. - Provider-agnostic domain: ExternalDependency, DependencyProvider, DependencyVersion (declared/resolved), DependencyUsageSite (file, line, enclosing symbol from the index's own parser), DiscoveryEvidence, DependencyContract/ContractSource/ContractFingerprint, confidence. - DependencyProviderAdapter interface; OpenAI, Stripe and GitHub are declarative ProviderSpecs of one adapter (a new provider is one spec), plus a generic OpenAPI adapter and generic declared packages. - OpenAPI 3.x/Swagger 2.0 normalization (paths, methods, parameters, bodies, responses, security, components) with fingerprints that ignore format/order/prose but change on real path/schema/auth changes. - Secret-value discipline: secret-store-like files are never opened, only tracked files are read in a git checkout, env values and literal defaults are never captured, URLs keep host + sanitized path only, and URL-shaped version specs are reduced to url:<host>. - Dependency graph on the existing GraphNode/RepositoryEdge primitives, and text/JSON inventory reports. - Fixture repositories (mixed multi-provider demo, false-positive traps) and tests for versions, scanners, no-secret handling, fingerprints and provider extensibility.
- external_dependencies / external_dependency_usage_sites / external_contract_snapshots (migration 0034, all cascading from repositories): latest state per dependency, current usage sites, and a contract history with one row per distinct fingerprint. - DependencyRegistry.record_inventory is idempotent (identical re-discovery writes no rows), updates versions/contracts in place while adding a new snapshot, marks vanished dependencies removed and reactivates them, rewrites usage sites only when they change, and serializes per-repository writes with an advisory lock. - `python -m patchfrog.cli dependencies discover <repo> [--json] [--persist --full-name owner/repo] [--all] [--output]`. - docs/architecture.md: the new product architecture (discovery -> registry -> [M6] upstream change detection -> impact -> migration -> verification -> PR), with generic review marked secondary and M6+ marked future. docs/dependency-discovery.md, roadmap and cost-guard cross-links updated.
CI resolves sqlalchemy[asyncio]>=2.0,<3.0 fresh and picked up 2.1.0, whose stubs type a selected nullable column as `X | None` (even when the query filters `.is_not(None)`) and no longer infer an element type for `text()` results. Local runs were still on 2.0.52. Narrow the nullable columns explicitly (the SQL filter is unchanged, the Python check only narrows the type) and annotate the two raw `text()` results in tests. No ignores, no strictness change; passes on both 2.0.52 and 2.1.0.
CI (and the Docker images) resolve dependencies fresh on every run, so the open `>=2.0,<3.0` range pulled in SQLAlchemy 2.1.0 while local environments were on 2.0.x. Beyond the stricter result typing (fixed in the previous commit), 2.1 moved aiosqlite onto the generic asyncio cursor adapter, which keeps cursors open until soft-close. Under the shared-connection StaticPool SQLite test fixture, concurrent review sessions then hit "cannot commit transaction - SQL statements in progress" (4 integration tests). Production (asyncpg, real pool) is unaffected, but a runtime-behavior minor bump must be an explicit change, not resolver drift.
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.
First product-transition milestone: PatchFrog becomes an API/SDK-compatibility product. This PR covers M4 (ultra-low-cost review engine) and M5 (external dependency discovery + contract registry). M6 is not started. Nothing in
patchfrog-cloudchanged.Full audit, plan and results:
validation/m4_m5_cost_engine_dependency_discovery/latest-summary.md. Architecture:docs/architecture.md,docs/cost-aware-review.md,docs/dependency-discovery.md.M4: cost-aware review (operator default
PATCHFROG_REVIEW_STRATEGY=cost_aware)patchfrog.change_risk): no_ai / tiny / normal / elevated / high_risk, each with typed signals and reasons.UNIFIEDcall per candidate batch, with shared de-duplicated context. Escalation to a specialist is sequential, and every extra call carries a reason. The critic stays on-demand, with the policy unchanged.ReviewBudgetledger, with a verification reserve. One deliberate deviation from the suggested budgets: tiny = 1 reviewer call + 1 critic-only call, instead of 1 total. With 1 total, the existing safety rule would suppress a tiny PR's HIGH/security finding.{"tiny":1}restores the strict setting.--force, migration 0033, telemetry schema v12, and Prometheus counters.specialist_fanout.Cost benchmark (
eval cost-benchmark, fake provider, synthetic prices): 23 → 10 provider calls, input tokens 42.9k → 20.4k. Every target is met and no finding is lost.Beta-readiness guard (20 cases, oracle): quality metrics are identical before and after (all 1.0 / 0.0, variance 0). Reviewer calls 84 → 22, critic calls 10 → 10. This is a pipeline-correctness benchmark, not a measure of real-model quality.
M5: dependency discovery + registry
repositories). Usage sites carry enclosing symbols, and the dependency graph uses the existingGraphNode/RepositoryEdgetypes.python -m patchfrog.cli dependencies discover <repo> [--json] [--persist]Validation
mypy --strictclean.PATCHFROG_REQUIRE_POSTGRES=1.Do not merge without review.