Repository navigation
Close the Greptile gaps from the prospecting PR - #38
Merged
Merged
Conversation
Making ProspectingMessageRequest.text optional for intake answers
dropped the local guard against an empty message. The API still
rejects a body with neither text nor intake answers (422 from its
text_or_intake validator), but the spec cannot express that rule, so
the generated model let {} and {"intake": {}} through to a wasted
request. The base request class now applies the same rule, the way
it already mirrors the platform's geo shape rules.
answer_intake shares the messages route with message(), and the
contract check kept only the first method per route, so it never
looked at answer_intake. Its signature carries a hand-written intake
key Literal and IntakeAnswer values that nothing compared to the spec.
The check now keeps every decorated method and checks answer_intake's
arguments against the body fields they fill: intake keys both ways,
IntakeAnswer fields both ways, and that the text field still exists.
Comment on lines
+236
to
+237
| spec_keys = set(variant.get("propertyNames", {}).get("enum", [])) | ||
| if spec_keys and typing.get_origin(key_type) is Literal: |
There was a problem hiding this comment.
Missing key constraints go unchecked If the API schema removes
propertyNames.enum from intake, spec_keys is empty and this check skips the key comparison. The SDK could then reject a question key the API accepts without the contract check reporting the drift. Treat a missing key constraint as a mismatch rather than a passing check.
Knowledge Base Used: Contract and release workflows
Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/check_contract.py
Line: 236-237
Comment:
**Missing key constraints go unchecked** If the API schema removes `propertyNames.enum` from `intake`, `spec_keys` is empty and this check skips the key comparison. The SDK could then reject a question key the API accepts without the contract check reporting the drift. Treat a missing key constraint as a mismatch rather than a passing check.
**Knowledge Base Used:** [Contract and release workflows](https://app.greptile.com/discolike/-/custom-context/knowledge-base/discolike/discolike-python/-/docs/contract-and-release-workflows.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Contributor
Author
There was a problem hiding this comment.
Fixed in a follow-up commit: a Literal key type against a spec with no propertyNames enum is now reported ("spec accepts any key ... but the SDK restricts them"), with a test.
If the spec stops listing allowed intake keys, the SDK's Literal would reject keys the API accepts while the key comparison silently skipped, since it only ran when the spec had an enum.
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
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.
Two Greptile P2s left open on #37.
ProspectingMessageRequestwith neithertextnor anyintakeanswer is rejected locally. The API already 422s it (itstext_or_intakevalidator); the spec can't express the rule, so the generated model let it through.scripts/check_contract.pykept only the first method per route, soanswer_intake(same route asmessage) was never checked. It now checksanswer_intake's arguments against the body fields they fill: intake keys,IntakeAnswerfields, andtext.The PR appears safe to merge; the contract-check coverage gap is non-blocking.
Fix with agent prompt
Summary
The PR rejects prospecting messages with neither text nor intake answers and adds contract coverage for the
answer_intakemethod on its shared route.Reviews (1) · Last reviewed commit: "Close the Greptile gaps from the prospec..."