fix: Raise proper error message for invalid format functions#68
Merged
Ruari-Phipps merged 3 commits intomainfrom Apr 10, 2026
Merged
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Contributor
There was a problem hiding this comment.
Pull request overview
Improves ADK’s robustness when parsing function decorators and handling merge-conflicted files so poly status, poly diff, and --json error output behave predictably instead of crashing on edge cases.
Changes:
- Make
_extract_decoratorsraise clearValueErrors for missing/unsupported type annotations (instead of crashing). - Exclude merge-conflicted files from
modified_filesand avoid parsing conflicted content inproject_status/get_diffs. - Emit structured JSON errors (with traceback) from the CLI when
--jsonis set; update docs and tests accordingly.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/poly/resources/function.py |
Updates decorator extraction to validate annotations and raise ValueError on invalid formats. |
src/poly/resources/resource.py |
Renames status API to is_modified() and removes merge-conflict detection from the resource layer. |
src/poly/project.py |
Detects merge conflicts from raw file contents before parsing; adjusts status/diff behavior. |
src/poly/cli.py |
Wraps command dispatch to return structured JSON error payloads when --json is used. |
src/poly/docs/functions.md |
Documents that parameters must have supported type annotations. |
src/poly/tests/resources_test.py |
Adds unit tests covering new decorator-extraction error cases. |
src/poly/tests/project_test.py |
Updates merge-conflict status expectations (conflicts no longer count as modified). |
Contributor
Coverage Report
Changed file coverage
|
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.
Summary
Fixes two related issues: untyped or unsupported function parameters now raise a clear
ValueErrorinstead of crashing withAttributeError, and merge-conflicted files are correctly excluded frommodified_filesinproject_statusand handled without crashing inget_diffs. Also surfaces all CLI errors as structured JSON when--jsonis set.Motivation
In v0.6.x, functions with untyped parameters (e.g.
def f(conv, booking_ref)) caused_extract_decoratorsto crash withAttributeError: 'NoneType' has no attribute 'id'. Separately, files with merge conflicts were being passed toread_local_resource, which would now raise rather than silently fail. Both issues needed to be fixed together forpoly statusandpoly diffto be reliable after a branch pull with conflicts.Closes #
Changes
_extract_decorators: guardarg.annotationaccess before reading.id; raiseValueErrorwith a distinct message for missing vs unsupported type annotations; remove thetry/except SyntaxErrorwrapper so errors propagate to the userresource.py: renameget_status→is_modified, removing merge conflict detection from the resource itselfproject_status: check for merge conflicts on raw file content before callingread_local_resource; conflicted files now appear only infiles_with_conflicts, not also inmodified_filesget_diffs: same conflict-before-parse pattern; shows diff of conflict markers vs original; fixtype(resource_type)key bug and missing second arg toget_diffcli.py: wrap the command dispatch intry/except; when--jsonis set, errors are returned as{"success": false, "error": "...", "traceback": "..."}instead of raisingsrc/poly/docs/functions.md: note that all parameters must have a supported type annotationTest strategy
poly <command>)Checklist
ruff check .andruff format --check .passpytestpassespolyCLI interface (or migration path documented)Screenshots / Logs