Conversation
|
|
|
||
| class AIMetadata(BaseModel): | ||
| provider: Optional[str] = None | ||
| models: Optional[list[str]] = None |
There was a problem hiding this comment.
Multiple models may be involved in fix generation
| fixStatus: FixStatus | ||
| changeSets: list[ChangeSet] | ||
| fixMetadata: Optional[FixMetadata] = None | ||
| fixQuality: Optional[FixQuality] = None |
There was a problem hiding this comment.
Fix quality evaluation belongs to the entire result, not just to a single file/diff.
| class FixResult(BaseModel): | ||
| """Result corresponding to a single finding""" | ||
|
|
||
| finding: Finding |
There was a problem hiding this comment.
There is now a strict 1:1 mapping between finding and fix.
| fixMetadata: Optional[FixMetadata] = None | ||
| fixQuality: Optional[FixQuality] = None | ||
| # A description of the reasoning process that led to the fix | ||
| reasoningSteps: Optional[list[str]] = None |
There was a problem hiding this comment.
Potentially each entry here should be something richer than a flat string: we might want to incorporate steps that represent user input/feedback.
There was a problem hiding this comment.
I could quibble that we have no idea what form that would take, and whether we should design for anything beyond a str now... but if you have short term thoughts that this is a promising data type, I'm okay with it.
|
|
||
| class CodeTF(CodeTFWriter, BaseModel): | ||
| run: Run | ||
| results: list[FixResult] |
There was a problem hiding this comment.
My expectation is that each finding in the input will have a corresponding result. I don't believe that's enforceable in the spec, and it probably should not be since we want to allow for the possibility of streaming subsets of results.
There was a problem hiding this comment.
I don't believe that's enforceable in the spec, and it probably should not be since we want to allow for the possibility of streaming subsets of results.
I'm not sure why these requirements are at odds. Aren't we only giving it 1 input as well? We had discussed as an input a single SARIF, and a single result ID (or something.) In this case, we'd expect 0 or 1 FixResult.
There was a problem hiding this comment.
Aren't we only giving it 1 input as well? We had discussed as an input a single SARIF, and a single result ID (or something.)
As a practical matter I'm still not convinced that's the best approach although we can work towards that setup. In any case I don't think we want to preclude the possibility of batch processing.
In this case, we'd expect 0 or 1 FixResult.
I would argue that the absence of any FixResult should be an exceptional condition. We should potentially consider making FixResult required. The fact that this would encode fix status means we should return a failure status if processing is failed or skipped
|
|
||
| class PackageAction(BaseModel): | ||
| action: Action | ||
| result: PackageResult |
There was a problem hiding this comment.
I'm not sure I ever fully understood the rationale for the result field: it might be worth discussing whether it's still necessary.
There was a problem hiding this comment.
I think associating a change to a dependency can allow downstream users to tell users add a dependency (like if it failed), or that this increases reliance on a dependency, etc.
|
I am merging this as it is still currently in draft status and does not affect any production codepaths. However making it available will be useful for prototyping new features and functionality. |
3 participants
Overview
Introduces new fix-oriented CodeTF data model as implementation for v3