Skip to content

feat(weave): scorer_* typed columns on feedback#6908

Open
nikumar1206 wants to merge 5 commits into
masterfrom
nikhil/scorer-feedback-columns-wiring
Open

feat(weave): scorer_* typed columns on feedback#6908
nikumar1206 wants to merge 5 commits into
masterfrom
nikhil/scorer-feedback-columns-wiring

Conversation

@nikumar1206
Copy link
Copy Markdown
Contributor

@nikumar1206 nikumar1206 commented May 20, 2026

Description

Wires the four `scorer_*` columns from #6906 through the trace-server so they round-trip via the existing feedback endpoints.

  • `orm.py`: new column types `array_string`, `map_string_string`, `map_string_float`. ClickHouse passes native list/dict; SQLite encodes as JSON text and decodes on read.
  • `feedback.py`: adds the four columns to `TABLE_FEEDBACK`. `format_feedback_to_row` defaults `None` → `[]` / `{}`.
  • `trace_server_interface.py`: four optional fields on `FeedbackCreateReq` (inherited by `Feedback` / `FeedbackReplaceReq`).

Testing

Unit tests.

Copy link
Copy Markdown
Contributor Author

nikumar1206 commented May 20, 2026

@wandbot-3000
Copy link
Copy Markdown

wandbot-3000 Bot commented May 20, 2026

@codecov
Copy link
Copy Markdown

codecov Bot commented May 20, 2026

Codecov Report

❌ Patch coverage is 86.36364% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
weave/trace_server/orm.py 85.71% 1 Missing and 1 partial ⚠️
weave/trace_server/sqlite_trace_server.py 50.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@nikumar1206 nikumar1206 force-pushed the nikhil/scorer-feedback-columns-migration branch from 3165f98 to 7d85f02 Compare May 20, 2026 18:58
@nikumar1206 nikumar1206 force-pushed the nikhil/scorer-feedback-columns-wiring branch from bb33ae8 to 33c36ec Compare May 20, 2026 18:58
@nikumar1206 nikumar1206 force-pushed the nikhil/scorer-feedback-columns-migration branch from 7d85f02 to cf341ff Compare May 20, 2026 19:17
@nikumar1206 nikumar1206 force-pushed the nikhil/scorer-feedback-columns-wiring branch from d80d70b to 39e2cad Compare May 20, 2026 19:17
@nikumar1206 nikumar1206 force-pushed the nikhil/scorer-feedback-columns-wiring branch 2 times, most recently from 1cb0e5e to 63cb0b4 Compare May 20, 2026 23:47
@nikumar1206 nikumar1206 force-pushed the nikhil/scorer-feedback-columns-migration branch from cf341ff to 34b26b1 Compare May 20, 2026 23:47
@nikumar1206 nikumar1206 marked this pull request as ready for review May 21, 2026 15:44
@nikumar1206 nikumar1206 requested a review from a team as a code owner May 21, 2026 15:44
Copy link
Copy Markdown
Contributor

@jtschoonhoven jtschoonhoven left a comment

Choose a reason for hiding this comment

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

Approving to unblock, but see comments. Lmk if you want to discuss more.

Comment thread weave/trace_server/orm.py Outdated
Comment thread weave/trace_server/orm.py Outdated
Comment thread weave/trace_server/trace_server_interface.py Outdated
@nikumar1206 nikumar1206 force-pushed the nikhil/scorer-feedback-columns-migration branch from 1a7832a to 94d6f83 Compare May 21, 2026 20:36
@nikumar1206 nikumar1206 force-pushed the nikhil/scorer-feedback-columns-wiring branch from 63cb0b4 to dab1855 Compare May 21, 2026 20:36
@nikumar1206 nikumar1206 force-pushed the nikhil/scorer-feedback-columns-migration branch from 94d6f83 to 74e66c5 Compare May 21, 2026 21:48
@nikumar1206 nikumar1206 force-pushed the nikhil/scorer-feedback-columns-wiring branch from dab1855 to 55dd6ab Compare May 21, 2026 21:48
@jtschoonhoven jtschoonhoven changed the base branch from nikhil/scorer-feedback-columns-migration to graphite-base/6908 May 22, 2026 17:22
@jtschoonhoven jtschoonhoven force-pushed the nikhil/scorer-feedback-columns-wiring branch from 55dd6ab to 7d46052 Compare May 22, 2026 17:22
@graphite-app graphite-app Bot changed the base branch from graphite-base/6908 to master May 22, 2026 17:23
@jtschoonhoven jtschoonhoven force-pushed the nikhil/scorer-feedback-columns-wiring branch from 7d46052 to 200a56f Compare May 22, 2026 17:23
@nikumar1206 nikumar1206 force-pushed the nikhil/scorer-feedback-columns-wiring branch from b3d41e6 to b48e754 Compare May 22, 2026 18:44
@nikumar1206 nikumar1206 force-pushed the nikhil/scorer-feedback-columns-wiring branch from b48e754 to 39a1b35 Compare May 22, 2026 19:06
Copy link
Copy Markdown
Contributor

@jtschoonhoven jtschoonhoven left a comment

Choose a reason for hiding this comment

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

One comment to address, otherwise LGMT.

Let's not merge this until after the migration has been deployed just in case.

Comment thread weave/trace_server/orm.py Outdated
Co-authored-by: Jonathan Schoonhoven <jschoonhoven@wandb.com>
@nikumar1206 nikumar1206 force-pushed the nikhil/scorer-feedback-columns-wiring branch from 39a1b35 to dd558dc Compare May 22, 2026 22:18
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.

2 participants