Skip to content

Add Conversation-Level Analysis #1914

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

Add Conversation-Level Analysis #1914

wants to merge 16 commits into from
+899 −336

Conversation

ryan-arman
Copy link
Contributor

@ryan-arman ryan-arman commented Aug 6, 2025 •
edited
Loading

Description

This PR adds dual-level conversation analysis - enabling both message-level and conversation-level analysis of datasets.

Main changes:

  • SampleAnalyzer has a analyze_sample method (instead of analyze_message) which computes both message level and conversation level metrics and returns tuple[list[MessageAnalysisResult], ConversationAnalysisResult]
  • LengthAnalyzer implements SampleAnalyzer in a way that conversation level metrics are the aggregate of message level metrics for all of the metrics (char, word, sentence) except for token. For token, it tokenizes the conversation directly using the datast.tokenize
  • dataset is passed to the LengthAnalyzer so it can use it for tokenziation
  • we can remove the tokenizer in a follow up PR since we are going to use the dataset directly
  • All of the tests are updated accordingly
  • The check for sample_count has been moved to analyze_config

Related issues

Fixes OPE-1455

Before submitting

  • This PR only changes documentation. (You can ignore the following checks in that case)
  • Did you read the contributor guideline Pull Request guidelines?
  • Did you link the issue(s) related to this PR in the section above?
  • Did you add / update tests where needed?

Reviewers

At least one review from a member of oumi-ai/oumi-staff is required.

@ryan-arman ryan-arman self-assigned this Aug 6, 2025
ryan-arman added 14 commits August 7, 2025 10:07
@ryan-arman
feat: Add conversation-level analysis to sample analyzers
bfc5cbc 
- Add analyze_conversation() method to SampleAnalyzer base class
- Implement conversation analysis in LengthAnalyzer with conversation rendering
- Add ConversationAnalysisResult dataclass and update DatasetAnalysisResult
- Add conversation-level querying and DataFrame generation methods
- Update test files to implement analyze_conversation() in mock analyzers

This allows analyzers to compute both message-level and conversation-level metrics,
with conversation-level analysis potentially giving different results than
aggregating individual message metrics.
@ryan-arman
refactor: combine MessageAnalysisResult and ConversationAnalysisResul…
bfd967b 
…t into SampleAnalysisResult

- Create new SampleAnalysisResult class that combines message-level and conversation-level results
- Update SampleAnalyzer to return SampleAnalysisResult instead of DatasetAnalysisResult
- Refactor DatasetAnalysisResult to contain list of SampleAnalysisResult objects
- Update LengthAnalyzer and all test files to work with new structure
- Simplify data organization and make analyzer results more intuitive
- All tests passing and code formatting compliant
@ryan-arman
fix: automatically pass tokenizer from config to analyzers
7ca1f9b 
- Fix DatasetAnalyzer to automatically pass tokenizer from config to analyzer constructors
- Resolves issue where LengthAnalyzer with token_count=True failed without tokenizer
- Maintains backward compatibility for analyzers that don't need tokenizers
- Fix import issue in test file
- All tests passing and code formatting compliant
@ryan-arman
refactor: remove unused samples field from DatasetAnalysisResult
5cdd274
@ryan-arman
fix: restore copyright header in analyze __init__.py
4155028
@ryan-arman
refactor: move dataclasses from types.py to dataset_analyzer.py
0dc51a2
@ryan-arman
update comments
4033048
@ryan-arman
undo unintended changes
0e25a2a
@ryan-arman
move sample_count to config
079e508
@ryan-arman
add logging back
d22bcb0
@ryan-arman
refactor: rename compute_metrics to analyze_sample for better semanti…
12f1863 
…c clarity
@ryan-arman
refactor LengthAnalyzer
85232cb
@ryan-arman
refactor: return individual components instead of SampleAnalysisResul…
073bff0 
…t wrapper
@ryan-arman
git statusfix: validate sample_count during config creation instead o…
7c9d4db 
…f execution
@ryan-arman ryan-arman force-pushed the ryan-arman-conversation-analysis branch from b0944cd to 7c9d4db Compare August 7, 2025 17:14
oelachqar
oelachqar reviewed Aug 7, 2025
View reviewed changes
@ryan-arman ryan-arman requested a review from oelachqar August 8, 2025 00:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jgreer013 jgreer013 Awaiting requested review from jgreer013

@taenin taenin Awaiting requested review from taenin

@oelachqar oelachqar Awaiting requested review from oelachqar

At least 1 approving review is required to merge this pull request.

Assignees

@ryan-arman ryan-arman

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ryan-arman @oelachqar