Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(ui): basic human feedback rendering in calls table #2991

Merged
merged 29 commits into from
Nov 19, 2024

Conversation

gtarpenning
Copy link
Member

@gtarpenning gtarpenning commented Nov 14, 2024

Description

MVP pr to combine feedback into the main calls data object.

https://wandb.atlassian.net/browse/WB-21666

Screenshot 2024-11-18 at 10 05 26 AM

Testing

How was this PR tested?

@gtarpenning gtarpenning changed the title Griffin/feedback column query chore(ui): basic human feedback rendering in calls table Nov 15, 2024
@gtarpenning gtarpenning marked this pull request as ready for review November 15, 2024 16:00
@gtarpenning gtarpenning requested review from a team as code owners November 15, 2024 16:00
const field = inputField.startsWith('summary.weave.feedback')
? inputField.replace('summary.weave.feedback.', '')
: inputField;
const deBracketed = field.replace(/\[.*\]/g, '');
Copy link
Collaborator

Choose a reason for hiding this comment

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

the fact that this is looking for a bracket does not make sense to me given the comment above

Copy link
Collaborator

Choose a reason for hiding this comment

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

i think this is just a no-op right now

Copy link
Member Author

Choose a reason for hiding this comment

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

ill update the comment, but I am using this fn in one place where we go from backend filter -> displayName

export const getFieldLabel = (field: string): string => {
  if (field.startsWith('feedback.')) {
    // Here the field is coming from convertFeedbackFieldToBackendFilter
    // so the field should start with 'feedback.' if feedback
    const parsed = parseFeedbackType(field);
    if (parsed === null) {
      return field;
    }
    return parsed.displayName;
  }
  return FIELD_LABELS[field] ?? field;
};

Copy link
Collaborator

Choose a reason for hiding this comment

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

hmmm, i would probably split those, but sure. It seems like a sufficiently different case

Copy link
Collaborator

Choose a reason for hiding this comment

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

hmmm, i would probably split those, but sure. It seems like a sufficiently different case

@gtarpenning gtarpenning enabled auto-merge (squash) November 19, 2024 01:54
@gtarpenning gtarpenning merged commit 21812f6 into master Nov 19, 2024
113 of 115 checks passed
@gtarpenning gtarpenning deleted the griffin/feedback-column-query branch November 19, 2024 02:05
@github-actions github-actions bot locked and limited conversation to collaborators Nov 19, 2024
@@ -108,11 +109,12 @@ export const useCallsForQuery = (
{
skip: calls.loading,
includeCosts: true,
includeFeedback: true,
Copy link
Member Author

Choose a reason for hiding this comment

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

includeFeedback: true,

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants