Skip to content

feat: evidence-grade NIM discovery + all-modality cost-quality benchmark - #90

Draft
seonghobae wants to merge 199 commits into
fix/atheris-interpreter-lockfrom
claude/nim-all-models-support-fecb0b
Draft

feat: evidence-grade NIM discovery + all-modality cost-quality benchmark#90
seonghobae wants to merge 199 commits into
fix/atheris-interpreter-lockfrom
claude/nim-all-models-support-fecb0b

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Purpose

Implement issue #86 as a provider-neutral, evidence-grade NVIDIA NIM discovery and benchmark harness while preserving standalone operation and the modular CWL MSA boundary.

Exact integration identity

  • Current exact head: a9a37e28c1790be43cbdbd472d8e1017762b71f5
  • Stacked base branch: PR fix(security): pin provider egress and repair the Atheris lock #96 fix/atheris-interpreter-lock
  • Current exact base tip: 2df63e080737d5aa29d40871af99aa13411c3d76
  • Current GitHub merge-test SHA: 07fe73b1e9f8c3b3c564a9a3d4fdc47dfb36d2c1
  • State: Draft / mergeable

The base branch is independently maintained and may move. Before every mutation and acceptance decision, GitHub PR metadata is authoritative. Any check, status, review, approval, or synthetic merge associated with another head/base pair is historical only.

Current-head verification state

Repository workflows have materialized for exact head a9a37e28c1790be43cbdbd472d8e1017762b71f5 but are not yet passing evidence:

  • Tests run 31174602772: queued;
  • Security run 31174602823: queued;
  • Fuzz run 31174602869: queued;
  • CodeRabbit commit status: successful.

Queued evidence is not success. Security Scan, SAST Semgrep, trusted 100% statement/branch/docstring/package evidence, OpenCode, Noema, Strix, release acceptance, and qualifying independent non-author approval are absent for this exact head. One valid transactional-publication review thread remains intentionally unresolved until its current-head evidence requirement is actually satisfied.

The previous stacked-base merge conflict is repaired without weakening either side: repository-local workflows retain #96's arbitrary-base exact-contributor-head checkout contract, the exact-head regression matches the current base, and CHANGELOG.md preserves current provider-response and stacked-workflow evidence while moving the NIM feature record to a conflict-free Added section.

Implemented benchmark scope

  • dynamically discover the OpenAI-compatible GET /v1/models catalog without a hard-coded authoritative inventory;
  • deduplicate and provenance-hash catalog evidence;
  • compute the complete discovery, all-model capability-probe, and equal-budget evaluation plan before capability egress;
  • fail closed before the first capability probe when the hard request cap cannot cover the complete plan;
  • probe every discovered model under deterministic bounded concurrency;
  • cover chat, completions, Responses, embeddings, image, valid deterministic one-frame H.264 video, audio understanding, transcription, and speech contracts;
  • classify supported, unsupported, throttled, timeout, unavailable, failed, and malformed outcomes with machine-readable reasons;
  • compare every eligible direct single-model arm, route_once, bounded conduct, and an optional reviewed cheapest-worker scenario under common total token and call envelopes;
  • separate reviewed actual free-to-caller access evidence from hypothetical paid pricing scenarios, never infer prices, and keep unsupported cost evidence unknown;
  • record provenance, configured and observed budgets, task completion, uncertainty, paired comparisons, evidence sufficiency, and Pareto frontiers in secret-redacted JSON/CSV/Markdown artifacts;
  • preserve every policy/task cell's exact step, role, agent, and model assignments in deterministic CSV evidence with JSON/CSV identity parity;
  • transactionally stage, validate, enrich, and publish the complete artifact set with ordinary-failure rollback, residue cleanup, bounded crash recovery, and final-path-only success output.

Strict scoring validity boundary

A test-first buyer-visible slice repairs construct-irrelevant score inflation in the locked task set.

  • RED contract head 337356117ede1b69ba1b47282038bfa0fcc9c360 required complete-answer scorers that did not yet exist.
  • Numeric scorer exact_number_match version 2 accepts only one complete finite decimal response; prose, negation, units, multiple values, NaN, and infinities do not earn credit.
  • Text scorer exact_text_match version 1 compares the complete NFC-normalized, whitespace-normalized, case-folded response against explicit declared alternatives; substrings and undeclared aliases do not match.
  • malformed answer keys, ambiguous locked scorer contracts, and scorer identity collisions fail before provider egress;
  • the supported CLI explicitly derives a private versioned strict manifest, while historical authoring-manifest and exploratory scorer behavior remain isolated from headline comparisons;
  • the derived manifest version and SHA-256 are written into existing provenance;
  • ordinary import contextual_orchestrator does not import the benchmark, CSV adapter, or strict-scoring adapter;
  • the strict scorer and supported publication path are included in permanent 100% statement/branch/docstring, package build/install/import, and end-to-end dry-run contracts.

An isolated networkless harness exercised all 142 strict-scoring production statements and 54 branches and passed; this diagnostic validates the bounded module logic but is not accepted as repository CI or merge evidence. Authoritative doctoring is in docs/doctoring/nim-benchmark-strict-answer-scoring.md.

Provider and credential security boundary

  • provider sockets dial only validation-time globally routable addresses;
  • original hostname authority, TLS SNI, and certificate verification are preserved;
  • ambient proxies and redirects are disabled;
  • credentials are never forwarded outside the validated origin or serialized;
  • provider responses are bounded to 8 MiB;
  • dry-run jobs receive no NVIDIA_NIM_API_KEY; only the bounded live benchmark step receives the GitHub Secret;
  • no COPILOT_GITHUB_TOKEN path is introduced;
  • optional benchmark adapters remain lazy and do not mutate the normal package at import time;
  • no one-shot, self-modifying, encoded-patch, branch-local repair, or temporary write-capable workflow exists in the current PR tree.

Central dependencies — read-only

Central .github#759 is closed without merge at exact head dcb87ea3421ec87ce6acc62b04beef56758aa33e; none of its predecessor evidence transfers.

The active protected-main prerequisites are owned exclusively by the central maintenance path:

  • .github#812: Draft at exact head 2137012a2bf57b19a36f20ff6dcaac3e3e3b8cba, repairing attempt-scoped coverage artifact recovery control flow;
  • .github#816: Draft at exact head 71f74df36bbe322b1022d6eb3fa5aa7ac53d0643, repairing LLVM 19 propagation and revalidation across the isolated Rust coverage runtime.

This repository neither writes those branches nor dispatches repair agents there. PR #96 must remain Draft until the required central changes reach protected .github main, then regenerate fresh exact-head central coverage and review evidence and merge first. This PR must subsequently retarget to integrated main and rerun every gate on the exact resulting head/base pair.

Evidence contracts

Permanent regressions cover complete-plan admission, one-request-short zero-partial egress, deterministic bounded concurrency, valid media fixtures, DNS rebinding and non-global egress, redirects, proxy and credential isolation, bounded provider bodies, equal token/call budgets, reviewed pricing expiry, strict scoring, complete assignment evidence, Pareto exclusion labels, schema completeness, transactional publication, rollback/crash recovery, package import isolation, exact contributor-head workflow checkout, fuzz instrumentation, and buyer-readable evidence-status semantics.

Stack order and merge acceptance

  1. central .github#812 and .github#816 must reach protected central main under their own maintenance path;
  2. PR fix(security): pin provider egress and repair the Atheris lock #96 must regenerate all exact-head central and repository evidence, obtain qualifying independent approval, and merge without bypass;
  3. this PR must retarget to the integrated main, reconcile once if its branch or base moved, and rerun all exact-head gates;
  4. resolve the publication thread only after its stated current-head evidence requirement is actually satisfied;
  5. merge only when every required check, security gate, repository rule, and qualifying independent non-author approval applies to one exact integrated head.

Queued, pending, skipped-required, cancelled, absent, stale-head, predecessor-head, failed, status-only, author-only, or synthetic-merge evidence is never success.

Refs #86.
Depends on #96 and the protected-main central repairs tracked by ContextualWisdomLab/.github#812 and #816.

…ark (#86)

Optional stdlib-only benchmark harness (contextual_orchestrator/nim_benchmark.py):

- Dynamic catalog discovery from the OpenAI-compatible GET /v1/models —
  no hard-coded inventory; deduplicated, sorted (response-order-drift
  immune), with machine-readable duplicate/invalid hygiene lists.
- Capability probes for every contract NIM can host: chat completions,
  text completions, Responses API, embeddings, image understanding,
  video understanding, omni-style audio understanding, audio
  transcription, and audio speech — omni_capable derived, skipped
  probes always carry a machine-readable reason, bounded concurrency
  under one shared hard request budget.
- Fair policy comparison on a locked task split: per-worker direct
  baselines (source of best-single-worker-in-hindsight), route_once,
  conduct capped at five steps, cheapest-eligible-worker; identical
  scorers, caps, timeouts, and token budgets across systems.
- Honest cost accounting: actual cost 0 while the hosted catalog is
  free; hypothetical paid cost only from an explicit versioned pricing
  scenario, "unknown" otherwise; the two never mix.
- Paired-bootstrap CIs, quality-latency and quality-hypothetical-cost
  Pareto frontiers, full provenance (git SHA, run id, catalog/manifest/
  pricing hashes, parameters), schema-validated JSON/CSV/Markdown
  artifacts with a secret-leak refusal guard.
- Fail closed: missing KV credential (NVIDIA_NIM_API_KEY, bootstrap
  env->KV only, never argv), incomplete discovery, exceeded budget,
  missing provenance, schema violations.
- Deterministic --dry-run drives the whole pipeline against an
  in-process synthetic provider covering every modality class — zero
  network, byte-identical artifacts.
- Tests: 100% statement+branch coverage of the new module, adversarial
  cases (malformed catalogs, duplicate ids, non-finite tokens/costs,
  rate limits, timeouts, order drift, secret redaction); new fuzz seam
  (Hypothesis + Atheris) for the catalog parser.
- CI: manual + conservative monthly scheduled workflow, single-flight
  concurrency, hard budgets, pinned actions, 90-day artifact retention.
- Docs: docs/nim_benchmark.md, architecture/tracks pointers, HELM
  (arXiv:2211.09110) added to docs/papers.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown

Review Change Stack

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 72d0a628-b3fb-4c38-9535-c97d6cfa9a9b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

NVIDIA NIM 비용·품질 벤치마크 하네스를 추가했습니다. 카탈로그 탐색, modality 검증, 정책 평가, 비용·증거 검증, 결정적 dry-run, live CI 실행, artifact 생성 및 품질 테스트를 포함합니다.

Changes

NIM 벤치마크 기능과 검증

Layer / File(s) Summary
벤치마크 엔진과 실행 입력
contextual_orchestrator/nim_benchmark.py, contextual_orchestrator/__main__.py, examples/nim_*.json
NIM 모델 카탈로그 탐색, 전 modality probe, 요청 예산, 가격 검증, 정책 비교, 통계, provenance, artifact 생성 및 nim-benchmark CLI를 추가했습니다.
CI 실행과 카탈로그 퍼징
.github/workflows/*, fuzz/*, tests/fuzz/*, .gitignore
dry-run 및 live benchmark 작업을 추가했습니다. pull request 대상 제한을 제거했습니다. NIM 카탈로그 퍼징과 관련 Hypothesis 테스트를 추가했습니다.
벤치마크 계약 및 릴리스 검증
tests/test_nim_benchmark*.py, tests/test_pr_workflow_oidc_boundary.py, tests/test_repository_security_metadata.py
보안 경계, 요청 계획, 예산, capability 결과, 가격 증거, token usage, MP4 fixture, provenance, secret 격리 및 workflow 계약을 검증합니다.
증거 정책과 운영 문서
README.md, docs/*, CHANGELOG.md, docs/papers/README.md, conductor/tracks.md
실행 방법, 보안 제약, 증거 등급, 비용 처리, 운영 승인 규칙, 평가 방법론 및 변경 기록을 문서화했습니다.

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related PRs

Sequence Diagram(s)

sequenceDiagram
  participant Operator
  participant GitHubActions
  participant NIMBenchmark
  participant NIMProvider
  participant ArtifactStore

  Operator->>GitHubActions: dry-run 또는 live 실행 요청
  GitHubActions->>NIMBenchmark: 고정된 예산·manifest·provenance 전달
  NIMBenchmark->>NIMProvider: 카탈로그 조회 및 capability probe
  NIMProvider-->>NIMBenchmark: 모델·probe 결과 반환
  NIMBenchmark->>NIMProvider: 평가 task 요청 전송
  NIMProvider-->>NIMBenchmark: 응답·usage 반환
  NIMBenchmark->>ArtifactStore: JSON·CSV·Markdown artifact 업로드
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 55.94% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 NIM 동적 탐색과 전 모달리티 비용·품질 벤치마크라는 PR의 주요 변경 사항을 정확하고 간결하게 요약합니다.
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
⚔️ Resolve merge conflicts 💡
  • Resolve merge conflict in branch claude/nim-all-models-support-fecb0b
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/nim-all-models-support-fecb0b

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

Comment thread contextual_orchestrator/nim_benchmark.py Outdated

Copy link
Copy Markdown
Contributor Author

Exact-head maintainer audit of 064973ea3ac3dfd8975342f8453483fa16a09aa6 found three release-blocking gaps. This is not approval evidence.

  1. Provider egress is not DNS-pinned and redirects/proxies are not closed. build_default_transport() validates DNS once, then calls urllib.request.urlopen() against the hostname. That creates a second DNS resolution, follows redirects by default, and honors environment proxy configuration. A changed DNS answer or redirect can therefore forward the NVIDIA bearer credential outside the validated address set. The live policy-evaluation path also uses this branch's pre-fix(security): pin provider egress to validated public addresses #76 ModelClient transport. Stack this work on fix(security): pin provider egress to validated public addresses #76 and reuse its validation-time-address pinning, original-host TLS verification, direct no-proxy connection, and redirect rejection for both catalog/probe calls and model evaluation. Add adversarial tests for DNS rebinding, redirect credential forwarding, proxy bypass, non-global addresses including RFC 6598, and pinned-IP/original-host SNI behavior.

  2. The compared policies do not have an equal adjusted token budget. Each request receives the same max_output_tokens, while conduct_bounded may issue up to five calls and route_once/direct baselines issue one. Conduct can therefore consume roughly five times the output-token allowance, contrary to issue [Product Gap] Evidence-grade NVIDIA NIM model discovery and cost-quality benchmark #86. Introduce a per-policy/task total token allowance and the same declared maximum-call envelope, decrement it across every step, stop or fail cells when exhausted, and record configured versus observed budgets in every cell and the provenance parameters. Tests must prove identical total allowances across direct, route-once, and conduct arms.

  3. Actual-free cost is asserted without versioned evidence provenance. The report hard-codes actual_cost_usd = 0.0 and a static honesty label. NVIDIA currently documents Developer Program hosted endpoints as free for prototyping/research/testing, but evidence-grade artifacts must record the reviewed source identity, observation/review date, access/program context, and uncertainty or require a versioned run input. Do not silently carry the zero-cost assertion forward if access terms change.

Do not mark this PR ready or resolve these findings from stale checks. Re-run all required workflows and independent review on the exact repaired head.

@seonghobae
seonghobae changed the base branch from main to claude/inkspan-pr-audit-ci-q1u4uj August 4, 2026 08:15

@seonghobae seonghobae left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Blocking security finding

CRITICAL — benchmark HTTPS transport reintroduces DNS-rebinding SSRF

require_public_https_endpoint() validates one DNS answer, but build_default_transport() then calls urllib.request.urlopen(), which resolves the hostname again. The validated_hosts cache widens the gap by skipping validation on later calls. A provider hostname can therefore pass public-address validation and later connect to loopback, private, RFC 6598, link-local, or otherwise non-global infrastructure. Redirects are also delegated to urllib's default handler, so credentials may be forwarded to an unvalidated destination.

This is the same TOCTOU class repaired by PR #76. Keep this PR Draft until #76 is in main, then reuse or generalize its DNS-pinned transport so the socket dials only validation-time public IPs while preserving the original hostname for HTTP authority, TLS SNI, and certificate verification. Reject redirects, bypass environment proxies, require is_global, clear stale pins before each validation, and deterministically close failed sockets/responses.

Required regression evidence:

  • no transport-time DNS re-resolution;
  • public IPv4/IPv6 pinning and hostname/SNI preservation;
  • RFC 6598/private/loopback/link-local/multicast/reserved/unspecified rejection;
  • redirect rejection with no authorization propagation;
  • approved-IP fallback and all-address failure;
  • exact-head statement/branch coverage and full repository checks.

Do not merge based on the current local 100% claim; it does not cover this security invariant.

@seonghobae
seonghobae marked this pull request as draft August 4, 2026 08:16
@seonghobae
seonghobae changed the base branch from claude/inkspan-pr-audit-ci-q1u4uj to main August 4, 2026 08:32

@seonghobae seonghobae left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Exact-head review — still blocked

The latest head addresses the three audit themes, but the implementation is not yet mergeable.

1. HIGH — production security behavior is coupled to a test monkeypatch

nim_benchmark_hardening._build_secure_transport() deliberately falls back to urllib.request.urlopen() whenever that global has been replaced. This keeps the old offline tests green by changing production behavior instead of replacing the obsolete tests. It reintroduces the hostname-resolving, proxy-aware, redirect-capable path that this repair is supposed to eliminate, and requires a nosemgrep waiver on the exact sink.

Remove the compatibility branch entirely. Rewrite the transport tests to inject the pinned connection/resolver seam and prove the real direct transport. Do not let test instrumentation select a less-secure production path.

2. HIGH — the new 531-line hardening module has no direct regression suite

The current tests/test_nim_benchmark.py still asserts the old urlopen behavior and contains no evidence for EqualBudgetModelClient, expiry enforcement, actual-cost evidence validation, configured/observed budget fields, or the installed wrappers. A 100% repository claim cannot be accepted while the newly shipped module and its branches are not directly exercised.

Add behavior tests that fail without each contract, then prove exact-head statement and branch coverage at 100% for both nim_benchmark.py and nim_benchmark_hardening.py, plus 100% public docstrings.

3. HIGH — cited cost source does not support the recorded claim

ACTUAL_COST_EVIDENCE names https://docs.nvidia.com/nim/large-language-models/latest/faq.html, but the current NVIDIA FAQ location is different and the available FAQ content does not establish that the API Catalog hosted endpoint used by this run is free to the caller. Current build.nvidia.com model pages do label prototype endpoints as free, while NIM offering/licensing documentation distinguishes free exploratory offerings from NVIDIA AI Enterprise production support. Record the exact reviewed page/version or immutable evidence artifact that supports the hosted endpoint claim; do not cite a generic or moved FAQ as proof.

4. MEDIUM — optional adapter boundary is lost

contextual_orchestrator.__init__ now imports the entire benchmark and monkeypatch installer for every package import. Keep the NIM evaluator optional: integrate the fix directly into its module, or install it only from the benchmark entrypoint. Runtime gateway users should not import benchmark-only code or mutate benchmark globals as a side effect of import contextual_orchestrator.

5. MEDIUM — temporary privileged workflow remains in the PR tree

.github/workflows/temporary-nim-transport-fix.yml has top-level contents: write, persists checkout credentials, and triggers on every branch push. It must be removed from the final tree. Any one-shot repair must use read-only defaults, isolate credentials to the publication step, validate exact changed paths, and disappear before review.

Keep the PR Draft. Replace the compatibility installer with direct, test-first implementation; remove the temporary workflow; update the PR body to the actual head; then rerun all exact-head checks and independent review.

Copy link
Copy Markdown
Contributor Author

Exact-head maintainer follow-up for the new hardening commits:

  1. Keep the existing GHAS thread unresolved until contextual_orchestrator/nim_benchmark.py itself no longer contains the hostname-re-resolving urllib.request.urlopen implementation. Installing a replacement through package import does not remove the vulnerable/static-scanned implementation and forces the optional benchmark adapter into every package import.
  2. Correct the cost-evidence source to the current official NVIDIA NIM General FAQ: https://docs.api.nvidia.com/nim/docs/product. That page explicitly states free Developer Program access to NIM API endpoints for prototyping and distinguishes production use, which requires NVIDIA AI Enterprise. The current docs.nvidia.com/nim/large-language-models/latest/faq.html identity and the docs.nvidia.com-only validator do not match the reviewed current source.
  3. Preserve lazy/optional adapter loading: install benchmark-specific behavior from the benchmark module or integrate it directly, rather than eagerly importing the full benchmark and compatibility installer from package __init__.

Do not resolve the security thread or mark the PR ready until these are implemented and verified on the exact repaired head.

@seonghobae seonghobae reopened this Aug 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants