feat(w65): add bounded data-quality evaluator runtime - #250
Conversation
Signed-off-by: Ammar Heidari <ammar@arad-itc.org>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@codex review Please review exact head |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6798cf9fec
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| private static void ValidateScope( | ||
| DataQualityPolicyDefinition policy, | ||
| DataQualityEvaluationInput input) |
There was a problem hiding this comment.
When a policy configures a smaller EvaluationWindow, RecordsPerSecond, or BytesPerSecond, this validation checks only cluster/topic/partition while the loop trusts the independently supplied input.Budget. An input admitted under the much larger cycle hard caps can therefore be evaluated and reported under a restrictive policy, bypassing its configured resource ceilings; validate the window and derive effective record/byte limits from policy.Budget before evaluation.
Useful? React with 👍 / 👎.
| var recordViolation = | ||
| EvaluateRecord( | ||
| record, | ||
| policy.Rules, | ||
| counts); |
There was a problem hiding this comment.
If a record begins just before the deadline but JSON parsing and up to 128 rule evaluations finish after it, this call is followed by no deadline check when it is the final record, so the result is reported as Complete/Available despite exceeding MaxDuration. The same gap prevents cancellation requested during the final record from being reflected; add cooperative checks during expensive work or at least recheck before claiming complete progress.
Useful? React with 👍 / 👎.
| if (!int.TryParse( | ||
| segment, | ||
| NumberStyles.None, | ||
| CultureInfo.InvariantCulture, | ||
| out var arrayIndex) || |
There was a problem hiding this comment.
For an array value, int.TryParse accepts tokens such as "01" and resolves them as index 1, but JSON Pointer array indices must be 0 or a nonzero digit followed by digits. Consequently a rule like /items/01/id can incorrectly pass against the second element instead of failing as an invalid/unresolved pointer; validate the array-index token grammar before parsing it.
Useful? React with 👍 / 👎.
| private static string DecodePointerSegment( | ||
| string segment) => | ||
| segment |
There was a problem hiding this comment.
The policy contract currently admits pointer tokens containing escapes other than ~0 and ~1, and this decoder silently leaves them unchanged. Thus /a~2b can match a literal a~2b property even though it is not a valid JSON Pointer token, allowing a malformed rule to produce successful evidence rather than being rejected or treated as unresolved; decode with escape validation and fail closed on any other ~ sequence.
Useful? React with 👍 / 👎.
2 participants
Authority
W65 #214 is ACTIVE. Slice 1/2 contracts and evaluator/progress foundations are already protected-main through PRs #239/#243.
Slice 3
IDataQualityBatchEvaluatorruntime;~0/~1decoding and array indexes;Safety
Exact head:
6798cf9feca9cebf0736e41101700f0146958ac2.Refs #214 #209.