Skip to content

feat: add synchronous embeddings and KV-only secret bootstrap - #66

Draft
seonghobae wants to merge 11 commits into
claude/contextualwisdomlab-audit-governance-fb7470from
codex/sync-embeddings-file-ontology
Draft

feat: add synchronous embeddings and KV-only secret bootstrap#66
seonghobae wants to merge 11 commits into
claude/contextualwisdomlab-audit-governance-fb7470from
codex/sync-embeddings-file-ontology

Conversation

@seonghobae

@seonghobae seonghobae commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Purpose

Add a standard synchronous OpenAI-compatible embeddings endpoint and make KV-backed stdin bootstrap the only accepted provider/API-auth secret-value entry path.

Product behavior

  • POST /v1/embeddings shares the existing embeddings engine, validation, request limits, timeout behavior, model routing, and response contract with the batch API.
  • Boolean values are rejected as embedding numbers rather than being accepted through Python's bool-is-int relationship.
  • The response reports the backend model actually used.
  • Internal request-channel metadata is not exposed in the provider payload.
  • Provider and API-auth values enter through stdin JSON and are resolved from the KV; runtime argv and environment secret-value paths are removed.
  • The container startup and OpenAI example configuration use the same contract.

Exact-head verification

Current head: e7020795c6c5cbaac884dbcee3e0a37c409ab360

  • Tests: passed
  • Fuzz: passed
  • Security: passed
  • Security Scan: passed
  • SAST Semgrep: passed
  • CodeRabbit review threads: all resolved

Integration order

This PR remains Draft because it changes orchestrator.py, cost_ledger.py, batch_routing.py, package exports, and adjacent contracts also touched by the security and coverage stack. The required order is:

  1. merge fix(security): pin provider egress to validated public addresses #76's DNS-pinned provider-egress security base;
  2. integrate test(coverage): full test coverage across 6 modules + two latent-bug fixes; keep the Semgrep gate green #75's runtime bug fixes and coverage evidence;
  3. update this branch to the resulting main, preserve the reviewed synchronous-embeddings/KV behavior, and rerun exact-head tests, statement/branch coverage, docstrings, package build/install smoke tests, SAST, security, and independent review;
  4. only then return this PR to Ready.

No branch-protection, exact-head, or independent-review gate may be bypassed.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds a synchronous OpenAI-compatible embeddings endpoint to contextual-orchestrator while tightening the secret-handling story so runtime/provider/auth secret values are bootstrapped via stdin into the KV and resolved by KV name (not argv/env). This aligns the gateway with downstream consumers that expect /v1/embeddings and enforces KV-only secret resolution at the CLI/server boundary.

Changes:

  • Introduces POST /v1/embeddings with shared validation/limits and sync-wait behavior over the existing embeddings batch backend.
  • Extends embeddings plumbing to support optional dimensions, deadline-aware backend calls, and reconciliation metadata for ambiguous submissions.
  • Removes runtime secret-value paths via argv/env and updates docs/tests/Docker defaults to stdin JSON bootstrap + KV-name flags.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/test_kv_credentials.py Adds regression tests ensuring runtime secrets are not accepted via env/argv and stdin bootstrap resolves KV credentials correctly.
tests/test_batch_embeddings.py Adds sync embeddings endpoint tests plus deadline/timeout/reconciliation coverage for embedding backends.
tests/test_api_contract.py Asserts OpenAPI exposes /v1/embeddings with inference bearer auth and correct operationId.
README.md Updates local run and credential bootstrap instructions to KV-name + stdin JSON flow; documents sync embeddings behavior.
examples/agents.openai.json Refreshes the OpenAI example model configuration.
docs/rest_api_design.md Documents the new /v1/embeddings endpoint behavior and error semantics.
docs/kv-credentials.md Updates credential resolution/bootstrapping guidance to stdin-only value transport and KV-only runtime lookup.
Dockerfile Changes container default startup to bootstrap credentials from stdin into KV before serving.
contextual_orchestrator/server.py Adds /v1/embeddings handler, dimensions validation, and consistent error mapping for sync/batch embeddings.
contextual_orchestrator/cost_router.py Implements sync embeddings orchestration via polling the batch backend, adds dimension limits, and strengthens result validation.
contextual_orchestrator/batch_routing.py Makes embedding backends deadline-aware; adds dimensions to embedding requests; adds reconciliation support for pg-llm-batch submissions.
contextual_orchestrator/api_contract.py Extends OpenAPI contract to include /v1/embeddings and dimensions for embeddings endpoints.
contextual_orchestrator/main.py Removes runtime secret flags/env lookups; adds KV-name token flags and stdin bootstrap modes.
contextual_orchestrator/init.py Exposes EmbeddingSubmissionTimeout in the public package exports.
CLAUDE.md Updates developer commands/examples to use stdin bootstrap + KV-name tokens.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread contextual_orchestrator/__main__.py
Comment thread contextual_orchestrator/cost_router.py
Copilot AI review requested due to automatic review settings July 21, 2026 10:46

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

contextual_orchestrator/main.py:71

  • register-credential still exposes --value-stdin but the flag is not used (the command always reads from stdin). This makes the CLI misleading: callers can omit the flag and still hit stdin parsing errors, and the help implies the flag changes behavior.
    parser.add_argument("--name", required=True, help="Credential name, e.g. OPENAI_API_KEY.")
    parser.add_argument(
        "--value-stdin",
        action="store_true",
        help="Read the secret value from stdin (the only supported secret transport).",
    )
    args = parser.parse_args(argv)

    value = _read_stdin_credential()

Comment thread contextual_orchestrator/server.py
Copilot AI review requested due to automatic review settings July 21, 2026 10:50

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

contextual_orchestrator/cost_router.py:584

  • The vector validation treats JSON booleans as valid embedding components because bool is a subclass of int (so True/False pass the (int, float) check). This can let invalid embeddings through and be cost-recorded as successful.
                isinstance(component, (int, float)) and math.isfinite(float(component))

Comment thread contextual_orchestrator/server.py Outdated
@seonghobae

Copy link
Copy Markdown
Contributor Author

Consumer linkage: ContextualWisdomLab/noema#19 now requires this PR’s KV-backed inference-token contract for production reviews, verifies /healthz service identity, and refuses direct external-provider bypass. Both PRs remain draft; no live gateway deployment or credential/organization setting was changed.

@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@seonghobae, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 6 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: f1dc9ff4-6583-439c-8333-8db851cd2c20

📥 Commits

Reviewing files that changed from the base of the PR and between 6841b71 and e702079.

📒 Files selected for processing (18)
  • CLAUDE.md
  • Dockerfile
  • README.md
  • contextual_orchestrator/__init__.py
  • contextual_orchestrator/__main__.py
  • contextual_orchestrator/api_contract.py
  • contextual_orchestrator/batch_routing.py
  • contextual_orchestrator/cost_ledger.py
  • contextual_orchestrator/cost_router.py
  • contextual_orchestrator/orchestrator.py
  • contextual_orchestrator/server.py
  • docs/kv-credentials.md
  • docs/rest_api_design.md
  • examples/agents.openai.json
  • tests/test_api_contract.py
  • tests/test_batch_embeddings.py
  • tests/test_kv_credentials.py
  • tests/test_pr66_review_regressions.py

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

Comment thread .github/workflows/pr66-review-repair.yml Fixed
Comment thread .github/workflows/pr66-review-repair.yml Fixed
@seonghobae
seonghobae marked this pull request as ready for review August 4, 2026 00:17
@seonghobae
seonghobae enabled auto-merge (squash) August 4, 2026 00:17

@opencode-agent opencode-agent Bot left a comment

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.

Pull request overview

OpenCode cannot approve yet because required coverage evidence did not pass.

Review outcome

1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence

  • Problem: The required coverage-evidence job result was failure, so OpenCode cannot establish approval sufficiency for this head.

  • Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.

  • Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports success with required evidence or explicit no-source not-applicable evidence.

  • Regression test: Keep the approval branch checking needs.coverage-evidence.result == success before posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present.

  • Result: REQUEST_CHANGES

  • Reason: coverage-evidence result was failure, so required test/docstring evidence was not proven for current head e7020795c6c5cbaac884dbcee3e0a37c409ab360.

  • Head SHA: e7020795c6c5cbaac884dbcee3e0a37c409ab360

  • Workflow run: 30868407263

  • Workflow attempt: 1

Coverage evidence

Coverage evidence job did not run or did not publish coverage evidence.

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Changed file (12 files)"]
  S1 --> I1["repository behavior"]
  I1 --> R1["Review risk: Changed file (12 files)"]
  R1 --> V1["required checks"]
  Evidence --> S2["Docs (2 files)"]
  S2 --> I2["operator or user guidance"]
  I2 --> R2["Review risk: Docs (2 files)"]
  R2 --> V2["docs review"]
  Evidence --> S3["Test (4 files)"]
  S3 --> I3["regression suite"]
  I3 --> R3["Review risk: Test (4 files)"]
  R3 --> V3["targeted test run"]
Loading

@opencode-agent

opencode-agent Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

OpenCode Review Overview

  • Head SHA: e7020795c6c5cbaac884dbcee3e0a37c409ab360
  • Workflow run: 30868407263
  • Workflow attempt: 1
  • Gate result: REQUEST_CHANGES (approval step)

Pull request overview

OpenCode cannot approve yet because required coverage evidence did not pass.

Review outcome

1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence

  • Problem: The required coverage-evidence job result was failure, so OpenCode cannot establish approval sufficiency for this head.

  • Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.

  • Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports success with required evidence or explicit no-source not-applicable evidence.

  • Regression test: Keep the approval branch checking needs.coverage-evidence.result == success before posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present.

  • Result: REQUEST_CHANGES

  • Reason: coverage-evidence result was failure, so required test/docstring evidence was not proven for current head e7020795c6c5cbaac884dbcee3e0a37c409ab360.

  • Head SHA: e7020795c6c5cbaac884dbcee3e0a37c409ab360

  • Workflow run: 30868407263

  • Workflow attempt: 1

Coverage evidence

Coverage evidence job did not run or did not publish coverage evidence.

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Changed file (12 files)"]
  S1 --> I1["repository behavior"]
  I1 --> R1["Review risk: Changed file (12 files)"]
  R1 --> V1["required checks"]
  Evidence --> S2["Docs (2 files)"]
  S2 --> I2["operator or user guidance"]
  I2 --> R2["Review risk: Docs (2 files)"]
  R2 --> V2["docs review"]
  Evidence --> S3["Test (4 files)"]
  S3 --> I3["regression suite"]
  I3 --> R3["Review risk: Test (4 files)"]
  R3 --> V3["targeted test run"]
Loading

@opencode-agent
opencode-agent Bot disabled auto-merge August 4, 2026 04:46
@seonghobae
seonghobae marked this pull request as draft August 4, 2026 05:58

Copy link
Copy Markdown
Contributor Author

Temporarily Draft to preserve stacked integration order. This feature touches orchestrator.py, cost_ledger.py, batch_routing.py, and adjacent exports also changed by the security and coverage slices. Merge #76 first, then #75, update this branch to the resulting main, retain the reviewed sync-embeddings/KV behavior, and rerun exact-head tests, coverage, packaging, security, and review before returning it to Ready.

@seonghobae
seonghobae changed the base branch from main to claude/contextualwisdomlab-audit-governance-fb7470 August 5, 2026 02:09
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.

3 participants