fix: address peqy review of v1.2.0 (v1.2.1) - #9
Merged
Conversation
- deals set-role: never POST an explicit null role type — omit opportunity_contact_role_type_id when unset (Apollo may reject null). - people search: merge `people` + `contacts` result lists instead of keeping only the first non-empty one (no silent drop). - conversations detail formatter: read fields via a dict-or-model helper so the rich sections render for raw dicts too, not only models. - resolve_stage_id: cap the "Available:" list at 15 names (+N more) so a large stage list can't produce a huge error message. - Document that conversations `--query` -> q_keywords is unverified for the (undocumented) conversations/search endpoint. Tests added for each. Bumps to 1.2.1. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![peqy[bot]](https://avatars.githubusercontent.com/in/1930020?s=60&v=4){kind=small}
apollo-api 0.3.1 typed update_opportunity_roles' payload as list[RoleAssignment]; set-role builds entries dynamically (conditional keys + pop) so they're plain dicts — cast at the boundary and require qodev-apollo-api>=0.3.1. (Local mypy had missed this against a 0.3.0 venv.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-ups from the automated review of #8 (v1.2.0).
Correctness
deals set-rolenever sends an explicitnullrole type (review Q on deals.py:151). Adding a contact without--role-typenow omitsopportunity_contact_role_type_identirely instead of postingnull(which Apollo may reject). Existing roles stored without a type are likewise sent without the key.people searchno longer drops results (people.py:56). When Apollo returns bothpeopleandcontacts, both are shown — previously only the first non-empty list survived.Robustness / polish
conversations getyields models or dicts.--stage-nameerrors (util.py:23). Unknown names list at most 15 available names (… (+N more)).Documented, not changed
conversations --query→q_keywords(review Q on conversations.py:20). The conversations/search endpoint is undocumented and I couldn't confirm keyword filtering server-side (the transcripts only ever paginated it).q_keywordsis Apollo's universal keyword param, so it's the best default; added a code comment + CHANGELOG note flagging it as unverified. Needs a live smoke test with a real key to confirm.Verification
ruff check✓ ·ruff format --check✓ ·mypy✓ ·pytest78 passed ✓🤖 Generated with Claude Code