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

Sessions Feedback #2730

Open
wants to merge 27 commits into
base: main
Choose a base branch
from
Open

Sessions Feedback #2730

wants to merge 27 commits into from

Conversation

kavinvalli
Copy link
Collaborator

@kavinvalli kavinvalli commented Oct 7, 2024

Screen.Recording.2024-10-08.at.4.03.13.pm.mov

Copy link

vercel bot commented Oct 7, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
helicone ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 10, 2024 5:21pm
helicone-bifrost ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 10, 2024 5:21pm
helicone-eu ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 10, 2024 5:21pm

@kavinvalli
Copy link
Collaborator Author

kavinvalli commented Oct 8, 2024

@chitalian @maamalama I feel like this is good enough to merge now, filters is i think big enough to create a separate pr on, and it will probably take longer than i expected 🥲

@kavinvalli
Copy link
Collaborator Author

3f8404f and 199ecca implements session feedback

  • Implementation Details
    • Creates a property on the first request of the sessions (Helicone-Session-Feedback) and sets it to 1 or 0 depending on the feedback
    • In the frontend, I just find the request with the property and set that to the value (suggested by Stefan)
      • The reason, I didn't just get the first request in the frontend as well was because the sorting was working a little differently for some reason and was giving a different request id
Screen.Recording.2024-10-09.at.11.09.24.am.mov

Comment on lines +373 to +379
const query = `
SELECT request_id
FROM request_response_rmt
WHERE properties['Helicone-Session-Id'] = '${sessionId}'
ORDER BY request_created_at ASC
LIMIT 1
`;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you also select for the organzation_id = the user org?

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually can we select from postgres instead? We have better indexes on the JSONB column for custom properties and clickhouse is notoriously bad at point queries

@chitalian chitalian changed the base branch from main to sessions-redesign-ui2 October 9, 2024 20:18
@chitalian chitalian changed the title Sessions Page Redesign Sessions Feedback Oct 9, 2024
LIMIT 1
`;

console.log({ query });
Copy link
Contributor

Choose a reason for hiding this comment

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

rm please

Copy link
Contributor

Choose a reason for hiding this comment

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

and the other console logs below too

Base automatically changed from sessions-redesign-ui2 to main October 9, 2024 20:42
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.

4 participants