conformance: provider-tool inventory rows, not_run disclosure, smoke coverage vocabulary - #62
conformance: provider-tool inventory rows, not_run disclosure, smoke coverage vocabulary#62Coldaine wants to merge 2 commits into
Conversation
…coverage vocabulary
🤖 CodeAnt AI — Review Status
|
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing |
| ), | ||
| "native Tavily map" | ||
| ); | ||
| const items = (data.urls || []).map((url) => ({ title: "", url, content: "", source: "tavily" })); |
There was a problem hiding this comment.
Suggestion: The Tavily Map response is read from urls, but the provider tool contract and the existing substrate summary use raw.results for this endpoint. Valid Tavily Map responses therefore produce zero native items and zero ColdSearch items, causing the conformance row to fail despite a successful provider call. Read the endpoint's actual result array consistently in both native and ColdSearch normalization. [api mismatch]
Severity Level: Major ⚠️
- ❌ Tavily Map conformance rows falsely fail.
- ⚠️ Full-matrix runs report zero valid Map results.
- ⚠️ Native-versus-ColdSearch parity cannot be measured.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** scripts/provider-pass-through.mjs
**Line:** 787:787
**Comment:**
*Api Mismatch: The Tavily Map response is read from `urls`, but the provider tool contract and the existing substrate summary use `raw.results` for this endpoint. Valid Tavily Map responses therefore produce zero native items and zero ColdSearch items, causing the conformance row to fail despite a successful provider call. Read the endpoint's actual result array consistently in both native and ColdSearch normalization.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| function makeColdSearchToolResult(target, output) { | ||
| const raw = output.raw && typeof output.raw === "object" ? output.raw : {}; | ||
| const items = toolRawItems(target, raw); | ||
| return { | ||
| ok: output.ok === true, | ||
| catalogued: output.catalogued === true, | ||
| result_count: items.length, | ||
| sample: sampleToolItems(items), | ||
| items, | ||
| provider: output.provider, | ||
| tool: output.tool, | ||
| }; |
There was a problem hiding this comment.
Suggestion: The tool-call result drops the CLI's raw provider payload before evidence is written. src/tools/substrate.ts explicitly returns raw provider detail for tool calls, but this object retains only derived items and identity fields, so publicResult and results.jsonl cannot preserve or inspect the raw pass-through response. Include the raw payload, subject to the existing redaction path, in the returned result. [incomplete implementation]
Severity Level: Major ⚠️
- ❌ Tool evidence loses raw provider payloads.
- ⚠️ results.jsonl cannot inspect provider-specific fields.
- ⚠️ Debugging provider-tool parity requires rerunning live calls.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** scripts/provider-pass-through.mjs
**Line:** 1027:1038
**Comment:**
*Incomplete Implementation: The tool-call result drops the CLI's `raw` provider payload before evidence is written. `src/tools/substrate.ts` explicitly returns raw provider detail for tool calls, but this object retains only derived `items` and identity fields, so `publicResult` and `results.jsonl` cannot preserve or inspect the raw pass-through response. Include the raw payload, subject to the existing redaction path, in the returned result.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fixThere was a problem hiding this comment.
Pull request overview
This PR expands the live-provider conformance and smoke reporting surface by adding catalogued provider-tool inventory rows, making “not run” coverage explicit, and publishing a smoke-only coverage table (including tools) into the scheduled canary workflow summary.
Changes:
- Add
REQUIRED_PROVIDER_TOOLSinventory rows and support selecting/running tool rows (--tool) in the Gate 0 pass-through harness, withnot_rundisclosure and per-row JSONL output. - Extend the smoke script to emit a provider/path/tool coverage table that keeps conformance status at
not_runwhile reporting smoke outcomes. - Add/extend tests and documentation to codify the new status vocabulary and coverage semantics; update the canary workflow to tee smoke output into the GitHub step summary.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/smoke-coverage.test.mjs | Adds unit coverage for the smoke-only coverage table semantics and row seeding. |
| test/provider-pass-through-script.test.mjs | Expands harness tests for tool rows, not_run, JSONL emission, redaction, and canary workflow triggers. |
| scripts/validate-docs.mjs | Adjusts baseline summary validation to tolerate status-vocabulary evolution (e.g., not_run). |
| scripts/smoke.mjs | Adds coverage row construction and rendering for smoke-only reporting, with explicit not_run conformance semantics. |
| scripts/provider-pass-through.mjs | Implements provider-tool inventory rows, selection (--tool), tool-native runners, not_run disclosure, and updated evidence rendering. |
| docs/contributing/testing.md | Documents scoped manual conformance commands for path and tool rows and explains not_run. |
| docs/contributing/ci.md | Updates CI/canary documentation to describe smoke-only coverage reporting and not_run semantics. |
| .github/workflows/canary.yml | Publishes smoke output (including coverage table) into $GITHUB_STEP_SUMMARY while preserving exit semantics. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| function inputForTarget(target) { | ||
| if (target.tool) { | ||
| return TOOL_INPUTS[`${target.provider}.${target.tool}`](); | ||
| } | ||
| if (target.path === "search") { | ||
| return { query: SEARCH_QUERY }; | ||
| } |
| let nativeResult; | ||
| try { | ||
| nativeResult = await nativeRunners[target.provider](target); | ||
| nativeResult = isTool | ||
| ? await nativeToolRunners[targetId(target)](target) | ||
| : await nativeRunners[target.provider](target); |
| - name: Live smoke test | ||
| run: node scripts/smoke.mjs | ||
| # Publish totals and the smoke-only provider/path coverage table in the | ||
| # workflow summary, even when a check fails (the job still goes red), so | ||
| # a green workflow cannot be read as "all supported integrations passed". | ||
| run: | | ||
| set +e | ||
| node scripts/smoke.mjs 2>&1 | tee smoke-output.txt | ||
| status=${PIPESTATUS[0]} | ||
| cat smoke-output.txt >> "$GITHUB_STEP_SUMMARY" | ||
| exit $status |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f9d7fdc18c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const items = (data.urls || []).map((url) => ({ title: "", url, content: "", source: "tavily" })); | ||
| return makeNativeToolResult(target, items, { | ||
| native_shape: ["urls[]"], |
There was a problem hiding this comment.
Parse Tavily Map results from the correct field
When a keyed tavily.map conformance row runs, both new legs read urls, but the existing Tavily tool implementation treats Map responses as raw.results (src/tools/substrate.ts:91-94). A successful response therefore becomes two empty result sets and is always reported as fail, so the newly documented scoped Tavily Map check cannot produce valid evidence. Parse data.results/raw.results using the response's actual item shape.
AGENTS.md reference: AGENTS.md:L16-L16
Useful? React with 👍 / 👎.
| hasUrlOrTitleOverlap(nativeResult.items, coldResult.items), | ||
| "native and ColdSearch tool results overlap by URL or title" | ||
| )); | ||
| detailLoss.push("ColdSearch tool call output preserves provider raw detail; the tool summary is a lossy derived view."); |
There was a problem hiding this comment.
Verify raw preservation before declaring tool conformance
For any provider-tool response that retains URL/title data but loses other provider-specific fields, this comparison still passes: makeColdSearchToolResult reduces output.raw to selected items, the native runner also discards its raw payload, and the checks only compare counts plus URL/title overlap. This unconditional note then records that raw detail was preserved even though the harness never tested it, allowing precisely the raw-detail regression these conformance rows are meant to detect. Retain and compare the native and ColdSearch raw payloads, or add an explicit preservation check before emitting this claim.
AGENTS.md reference: AGENTS.md:L16-L16
Useful? React with 👍 / 👎.
| const rows = buildCoverageRows(checkResults); | ||
| const lines = [ | ||
| "## Provider/Path Coverage (smoke-only)", | ||
| "", | ||
| "These are CLI-only smoke results. Each check executes the ColdSearch CLI with no", | ||
| "provider-native leg and no comparison, so no row below is a native-vs-ColdSearch", | ||
| "conformance pass: every supported row stays `not_run` in conformance vocabulary.", | ||
| "The Smoke column reports the CLI-only outcome: pass / fail / skip / not covered.", | ||
| "", | ||
| "| Row | Conformance | Smoke |", | ||
| "|---|---|---|", | ||
| ...rows.map((entry) => `| ${coverageRowId(entry.row)} | ${entry.conformance} | ${entry.smoke} |`), |
There was a problem hiding this comment.
Derive published totals from the coverage rows
When the agent check runs or skips, the preceding counters include that coverage: null check, while they omit every unsupported-by-smoke inventory row, including all provider-tool rows. For example, with zero secrets the summary reports 1 passed, 9 skipped, but the table contains 17 inventory rows whose actual smoke breakdown is 1 pass, 8 skips, and 8 not covered. Because the workflow now advertises these as the coverage-table totals, readers cannot reconcile them with the rows; calculate totals from buildCoverageRows() or explicitly label the existing counters as check totals and add inventory-row totals.
AGENTS.md reference: AGENTS.md:L13-L14
Useful? React with 👍 / 👎.
| checks.push(check(nativeResult.result_count > 0, "native returned non-empty results")); | ||
| checks.push(check(coldResult.ok, "ColdSearch tool call succeeded")); | ||
| checks.push(check(coldResult.result_count > 0, "ColdSearch tool call returned non-empty raw results")); |
There was a problem hiding this comment.
Require provider-tool rows to remain catalogued
If one of these inventory profiles is removed or renamed, generic tool call dispatch still forwards the now-uncatalogued provider endpoint and can return ok, non-empty results, and matching URLs, so this conformance row still reports pass even though makeColdSearchToolResult captured catalogued: false. This defeats the stated purpose of catalogued provider-tool inventory rows and would let registry/dispatch drift go unnoticed; include coldResult.catalogued === true in the comparison.
AGENTS.md reference: AGENTS.md:L9-L10
Useful? React with 👍 / 👎.
| const usedStatuses = [...new Set(rows.map((row) => row.status))]; | ||
| for (const status of usedStatuses) { |
There was a problem hiding this comment.
Keep validating the baseline's established status vocabulary
When the historical summary accidentally drops a zero-count status such as fail, blocked_provider, or waived_by_user, this new loop no longer reports drift because none of those values occurs in the current JSONL rows. Only the newly introduced not_run status is legitimately absent from this historical baseline; restricting validation to used statuses also removes coverage for the established vocabulary that the baseline already enumerates. Validate the pre-existing statuses unconditionally and exempt only statuses introduced after the snapshot.
AGENTS.md reference: AGENTS.md:L14-L14
Useful? React with 👍 / 👎.
- Tavily Map rows parse the endpoint's result array from raw.results, matching the substrate map summary; a valid Map response no longer reports zero items (CodeAnt Major + Codex P1). - Tool rows disclose raw_preserved and compareToolTarget checks it, so the raw-detail claim is verified instead of assumed. Provider raw detail stays at the CLI/redaction boundary and is not re-persisted into evidence, preserving the no-scraped-bodies evidence isolation rule (CodeAnt Major, Codex P1). - compareToolTarget also requires coldResult.catalogued, so a registry row removed or renamed can no longer pass by generic dispatch (Codex P2). - inputForTarget and native runner dispatch throw clear errors for missing entries instead of TypeErrors (Copilot). - canary.yml pins shell: bash for the PIPESTATUS step (Copilot). - smoke.mjs publishes coverage-row totals via coverageTotals() and labels the raw check counters, so the summary reconciles with the table (Codex P2). - validate-docs.mjs validates the baseline's established status vocabulary unconditionally and exempts only post-baseline statuses, so a dropped status cannot hide (Codex P2).
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a02ae62661
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| raw_preserved: | ||
| Boolean(output.raw) && | ||
| typeof output.raw === "object" && | ||
| Object.keys(output.raw).length > 0, |
There was a problem hiding this comment.
Compare raw payloads before marking them preserved
When provider-tool dispatch drops provider-specific fields but retains any non-empty object—such as only results—this check still marks raw_preserved true, and the remaining URL/title checks can report the row as passing. Fresh evidence in this revision is the new raw_preserved implementation, which tests only Object.keys(output.raw).length > 0 while the native runners discard their original payloads, so it cannot verify the documented requirement that raw contain the provider-native payload or a faithful subset; retain both payloads and compare their relevant fields/schema before declaring conformance.
AGENTS.md reference: AGENTS.md:L16-L16
Useful? React with 👍 / 👎.
| const links = data.links || data.data || []; | ||
| const items = links.map((url) => ({ title: "", url, content: "", source: "firecrawl" })); |
There was a problem hiding this comment.
Unwrap Firecrawl map link objects
When Firecrawl v2 returns its successful Map response with links entries shaped as { url, title, description }, this code stores each entire entry object in the normalized url field. The ColdSearch-side parser does the same at toolRawItems, so both result counts are nonzero but normalizedUrl rejects the object values and the URL/title overlap check always fails; extract link.url and the optional title instead so a valid firecrawl.map run can produce conformance evidence.
AGENTS.md reference: AGENTS.md:L16-L16
Useful? React with 👍 / 👎.
| checks.push(check(nativeResult.result_count > 0, "native returned non-empty results")); | ||
| checks.push(check(coldResult.ok, "ColdSearch tool call succeeded")); | ||
| checks.push(check(coldResult.result_count > 0, "ColdSearch tool call returned non-empty raw results")); |
There was a problem hiding this comment.
Validate the tool result provenance envelope
When tool call returns the correct raw results but mislabels provider or tool, or omits meta, this comparison still passes because it checks only ok, result content, raw presence, and catalogue membership. The documented provider-tool contract requires provider, tool, ok, raw, and meta, and these fields provide the provenance needed to audit the evidence; preserve meta in makeColdSearchToolResult and check all three envelope fields against the selected target before assigning pass.
AGENTS.md reference: AGENTS.md:L16-L16
Useful? React with 👍 / 👎.
Code Review SummaryStatus: 3 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
WARNING
Files Reviewed (8 files)
Commit Fix these issues in Kilo Cloud Reviewed by laguna-s-2.1:free · Input: 705.9K · Output: 85.5K · Cached: 2.5M |
User description
Summary
Implements the unchecked tasks of
plans/2026-08-10-live-provider-conformance.md:scripts/provider-pass-through.mjsnow carriesREQUIRED_PROVIDER_TOOLS(tavily.map,brave.webSearch,exa.contents,firecrawl.map) with native runners, exercised through the ColdSearchtool callCLI leg and compared against native HTTP, using the same Gate 0 status vocabulary.results.jsonl.not_run: added toALLOWED_STATUSES; coverage summaries (harnesssummary.mdand the smoke reporter) explicitly list supported-but-omitted rows asnot_runwith totals. A green workflow never implies all supported integrations passed.not_runin conformance vocabulary and are never reported as a native-vs-ColdSearchpass. Exit semantics unchanged (non-zero only when a check that ran fails).canary.ymltees the smoke coverage table into$GITHUB_STEP_SUMMARY; still scheduled/manual-only, non-gating.docs/contributing/testing.mdgains the scoped manual commands (path- and tool-scoped);docs/contributing/ci.mddescribes the smoke-only table andnot_run.not_run), summary discloses blocked/not_run without passes, secret/signed-URL redaction, smoke coverage vocabulary, canary non-gating.Validation
npm run typecheck— passnpm run test:docs— passnpm test— 251/251 passnode scripts/provider-pass-through.mjs --list— full matrix (13 paths + 4 tools)not_run), exit 0, baseline untouchedCloses the remaining tasks of the live-provider conformance plan.
CodeAnt-AI Description
Expand provider conformance coverage and disclose untested integrations
What Changed
not_runinstead of silently omitting or passing them.Impact
✅ Four provider tools included in conformance checks✅ Clear distinction between tested, skipped, blocked, and untested integrations✅ Safer conformance evidence without exposed credentials or signed URLs💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.