Skip to content

fix(scoring): surface snapshot warnings in previews - #985

Merged
JSONbored merged 4 commits into
mainfrom
codex/fix-stale-scoring-preview-warnings-propagation
Jun 22, 2026
Merged

fix(scoring): surface snapshot warnings in previews#985
JSONbored merged 4 commits into
mainfrom
codex/fix-stale-scoring-preview-warnings-propagation

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Motivation

  • A staleness warning was appended to served scoring snapshots but preview responses did not include snapshot-level warnings because buildScorePreview constructed warnings only from warningsFor(...).

Description

  • Merge args.snapshot.warnings into the preview warnings array inside buildScorePreview so snapshot warnings propagate to preview callers.
  • Add a unit test surfaces snapshot warnings in score previews (#810) to verify snapshot warnings appear in preview results.
  • Files changed: src/scoring/preview.ts, test/unit/scoring.test.ts.

Testing

  • Ran the targeted unit tests with npm exec vitest run test/unit/scoring.test.ts and the scoring test suite passed (43 tests passed).
  • Ran npm run typecheck (tsc --noEmit) and it succeeded.
  • An initial broad npm run test:unit run hit unrelated failures in test/unit/queue.test.ts, so I re-ran the targeted scoring tests to validate the change which passed.

Codex Task

@dosubot dosubot Bot added the size:XS label Jun 21, 2026
@ghost ghost added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jun 21, 2026
@ghost

ghost commented Jun 21, 2026

Copy link
Copy Markdown

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

🛠️ Maintainer review — Held for maintainer review

Reviewed 2 changed file(s) — two independent AI reviewers, synthesized. 🛠️ Request changes. Address the suggestions below before merging.

ℹ️ Recommend CLOSE — A required check is failing. CI failures are closed, not iterated in place — fix it and open a new focused PR. (your PR — not auto-closed; your call.)

Merge readiness: ❌ CI is red — approval withheld until it's green. Failing: see the checks tab.

Summary

The change introduces snapshot warnings into private context output and adds a corresponding test. However, the CI reports failures (validate and test suite), likely due to type or runtime issues with spreading args.snapshot.warnings which may be undefined. This constitutes a concrete blocker that must be resolved before merging.

🛑 Concerns raised — review before merging (reviewers were not unanimous)

  • Potential undefined args.snapshot.warnings causing a runtime/type error, leading to CI validation and test failures.
  • The CI test (2) check failed — the change must not introduce test failures.

💡 Nits — non-blocking, optional

  • Test description could reference the issue number more explicitly (e.g., "feat(scoring): sync hardening — staleness gate + robust constant parser #810").
  • Ensure args.snapshot.warnings is always an array (e.g., default to [] in the type definition or use args.snapshot.warnings ?? []).
  • Run the full test suite locally after fixing the type to confirm all tests pass.
  • Consider adding a comment or JSDoc explaining the purpose of concatenating snapshot warnings.
  • Consider adding a comment explaining the purpose of the new test case for better maintainability.

@ghost

ghost commented Jun 21, 2026

Copy link
Copy Markdown

Note

Gittensory Gate skipped

PR closed before full evaluation. No late first comment was created.

Signal Result Evidence Action
Gate result ⚠️ Skipped #985 is no longer open. No action.

💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

@ghost ghost added reviewbot-review labels Jun 21, 2026
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jun 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.61%. Comparing base (3b0af4a) to head (4d9416b).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #985   +/-   ##
=======================================
  Coverage   96.61%   96.61%           
=======================================
  Files         110      110           
  Lines       15079    15079           
  Branches     5451     5451           
=======================================
  Hits        14569    14569           
  Misses        106      106           
  Partials      404      404           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JSONbored JSONbored self-assigned this Jun 21, 2026

@ghost ghost 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.

Recommend close (ci_red) — held for your decision

@JSONbored
JSONbored merged commit 96f6067 into main Jun 22, 2026
19 checks passed
@JSONbored
JSONbored deleted the codex/fix-stale-scoring-preview-warnings-propagation branch June 22, 2026 08:21
@github-actions github-actions Bot mentioned this pull request Jun 22, 2026
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant