Skip to content

[codex] cover verifier report test gaps#5

Draft
chefbc2k wants to merge 73 commits intomasterfrom
codex/verifier-report-test-gaps
Draft

[codex] cover verifier report test gaps#5
chefbc2k wants to merge 73 commits intomasterfrom
codex/verifier-report-test-gaps

Conversation

@chefbc2k
Copy link
Copy Markdown
Owner

@chefbc2k chefbc2k commented Mar 19, 2026

Issue

The recent coverage work around create-dataset-and-list-for-sale exercised ownership mismatch diagnostics when voice-hash lookup succeeds and when authorization introspection is absent, but it still skipped the failure mode where isAuthorized exists and throws. That left the workflow's commercialization diagnostics partially unproven in the exact path used when the chain lookup is flaky rather than unavailable.

Root cause

packages/api/src/workflows/create-dataset-and-list-for-sale.ts intentionally treats authorization introspection as best-effort during ownership mismatch handling. The workflow catches failures from getVoiceHashFromTokenId and isAuthorized and falls back to a 409 with actorAuthorized: null so callers still receive the current owner and any recovered voice hash. The surrounding tests covered the missing-method case, but not the thrown-error case, so this recovery branch could regress unnoticed.

Fix

This update keeps scope tight to the changed workflow test file and adds a focused regression in packages/api/src/workflows/create-dataset-and-list-for-sale.test.ts. The new test sets up an ownership mismatch, returns a valid voice hash, forces isAuthorized to reject, and asserts that the workflow still raises the expected 409 conflict with actorAuthorized: null while preserving the recovered voiceHash in diagnostics.

Validation

  • pnpm vitest run packages/api/src/workflows/create-dataset-and-list-for-sale.test.ts --maxWorkers 1
  • pnpm vitest run packages/api/src/workflows/create-dataset-and-list-for-sale.test.ts --coverage --maxWorkers 1

@gemini-code-assist
Copy link
Copy Markdown

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses critical test gaps in the scripts/verify-report.ts module. It introduces new test cases to validate the logic for determining the highest-severity summary from multiple report classifications and ensures the writeVerifyReportOutput function correctly handles file writing based on the presence of an output path. These additions enhance the robustness and reliability of the report generation and output mechanisms.

Highlights

  • Enhanced Test Coverage for Summary Logic: Added comprehensive test cases for the buildVerifyReportOutput function to ensure correct summary severity ordering, validating that higher-severity classifications are prioritized.
  • Validated JSON Output Writing: Introduced new tests for the writeVerifyReportOutput function, verifying its behavior when writing JSON output to a file and correctly handling scenarios where no output path is provided.
  • Improved Test Infrastructure: Implemented temporary directory management and cleanup using fs.mkdtempSync and afterEach for file system-related tests, ensuring a clean test environment.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request improves test coverage for the verifier report helpers, specifically for summary generation and file output logic. The changes are a good step forward. I've identified an opportunity to make the new summary preference test more comprehensive and maintainable by using a data-driven approach with it.each and adding missing test cases. This will ensure all summary logic branches are robustly covered.

Comment on lines +70 to +92
it("prefers the highest-severity summary branch", () => {
expect(
buildVerifyReportOutput({
clarified: makeReport("semantically clarified but not fully proven"),
}).summary,
).toBe("semantically clarified but not fully proven");

expect(
buildVerifyReportOutput({
proven: makeReport("proven working"),
clarified: makeReport("semantically clarified but not fully proven"),
blocked: makeReport("blocked by setup/state"),
}).summary,
).toBe("blocked by setup/state");

expect(
buildVerifyReportOutput({
proven: makeReport("proven working"),
deeper: makeReport("deeper issue remains"),
blocked: makeReport("blocked by setup/state"),
}).summary,
).toBe("deeper issues remain");
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

While this test covers several important cases for summary preference, it could be more comprehensive and maintainable. Specifically, it's missing a test case where proven working is the expected summary, and it doesn't explicitly test the precedence of semantically clarified... over proven working as described in the PR description.

I suggest refactoring this into a data-driven test using vitest's it.each. This makes the different scenarios clearer, improves readability, and ensures all levels of severity precedence are explicitly tested.

  it.each([
    [
      "only proven reports are present",
      { proven: makeReport("proven working") },
      "proven working",
    ],
    [
      "clarified reports are present with proven reports",
      {
        proven: makeReport("proven working"),
        clarified: makeReport("semantically clarified but not fully proven"),
      },
      "semantically clarified but not fully proven",
    ],
    [
      "blocked reports are present with clarified reports",
      {
        clarified: makeReport("semantically clarified but not fully proven"),
        blocked: makeReport("blocked by setup/state"),
      },
      "blocked by setup/state",
    ],
    [
      "deeper issue reports are present with blocked reports",
      {
        deeper: makeReport("deeper issue remains"),
        blocked: makeReport("blocked by setup/state"),
      },
      "deeper issues remain",
    ],
  ])("prefers the highest-severity summary branch when %s", (_caseName, reports, expectedSummary) => {
    expect(buildVerifyReportOutput(reports).summary).toBe(expectedSummary);
  });

chefbc2k added 30 commits April 8, 2026 06:06
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.

1 participant