Skip to content

Conversation

@MeghnaAllamudi
Copy link
Collaborator

@MeghnaAllamudi MeghnaAllamudi commented Jun 6, 2025

Summary

We implemented two negotiation workers: a Single Issue Worker for straightforward price negotiations and a Multi Issue Worker for handling complex negotiations involving multiple aspects (financing, warranty, color, price). The Multi Issue Worker includes advanced features like KDE (Kernel Density Estimation) for analyzing offer patterns, utility calculations for both buyer and seller, and a monitoring system to track resolved issues. Both workers use MessageState for conversation tracking and share a consistent slot structure with proper metadata. Recent improvements focused on streamlining the code by removing redundant history tracking, fixing conversation flow issues (like double ice breakers), implementing proper JSON handling for complex data in slots, and ensuring clean integration with the orchestrator through a single-node task graph configuration. The result is a robust negotiation system that can handle both simple and complex negotiations while maintaining natural conversation flow and effective state management.

Description

This is needed to help scale the multi and single issue bots in the future.

Tests

I ensured that the run.py command works as expected and all parts of the bots are executing properly.

Here are the commands I used:
python run.py --input-dir ./examples/negotiation_agent_multi_issue (for the multi issue bot)
python run.py --input-dir ./examples/negotiation_agent (for the single issue bot)

Here is a screenshot of the multi issue conversation:
(insert screenshot here)

Reviewers

  • @ The reviewer who responsible for this PR

@MeghnaAllamudi MeghnaAllamudi marked this pull request as ready for review June 8, 2025 21:59
Copy link
Contributor

@yongwhan yongwhan left a comment

Choose a reason for hiding this comment

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

could you put some comments (e.g., docstrings) in your code?

Copy link
Collaborator

@luyunan0404 luyunan0404 left a comment

Choose a reason for hiding this comment

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

Could you put config and prompts under - negotiation_workers folder as well? So eveything related to negotiation are in one folder.
Please add the command to run your testing and paste your test result if possible.

@MeghnaAllamudi
Copy link
Collaborator Author

Could you put config and prompts under - negotiation_workers folder as well? So eveything related to negotiation are in one folder. Please add the command to run your testing and paste your test result if possible.

Yes will do. @luyunan0404 where should I add the command? Should I just add it to the description for this PR?

@MeghnaAllamudi
Copy link
Collaborator Author

could you put some comments (e.g., docstrings) in your code?

I've added them in the single issue and multi issue workers, let me know if you think there are any other places I should add them!

Copy link
Collaborator

@luyunan0404 luyunan0404 left a comment

Choose a reason for hiding this comment

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

lgtm without breaking the current logic.

Copy link
Contributor

@yongwhan yongwhan left a comment

Choose a reason for hiding this comment

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

lgtm

@yongwhan
Copy link
Contributor

make sure to update the branch; then, should be good for merging? @luyunan0404

Copy link
Contributor

@yongwhan yongwhan left a comment

Choose a reason for hiding this comment

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

make sure to add some unit tests

Copy link
Contributor

@yongwhan yongwhan left a comment

Choose a reason for hiding this comment

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

lgtm

@yongwhan yongwhan added the run-coverage-tests Label a pull request before merging to trigger test coverage label Jun 27, 2025
@github-actions
Copy link
Contributor

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  arklex/env/workers
  message_worker.py 228
Project Total  

This report was generated by python-coverage-comment-action

@MeghnaAllamudi MeghnaAllamudi changed the title Moving Negotiation Bots to Arklex feat: moving negotiation bots to arklex Jun 30, 2025
@yongwhan yongwhan marked this pull request as draft July 11, 2025 20:08
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we remove the changes here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think there was a ruff error which is why when I ran pre-commit it fixed this file, but will double check

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run-coverage-tests Label a pull request before merging to trigger test coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants