feat: add research input manifest contract - #272
Conversation
Co-Authored-By: Codex <noreply@openai.com>
|
@codex review Initial review request 1/1. Exact immutable 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: d79e19bd64
ℹ️ 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".
Co-Authored-By: Codex <noreply@openai.com>
|
@codex review Final review request 1/1 after the single consolidated remediation. Exact immutable head: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d1e82553b6
ℹ️ 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".
| exact = _Decimal(delta.days * 86_400 + delta.seconds) | ||
| fraction = match.group("fraction") | ||
| if fraction is not None: | ||
| exact += _Decimal(f"0.{fraction}") |
There was a problem hiding this comment.
When a timestamp has more fractional digits than the active decimal context can retain, adding the fraction to the epoch-second integer rounds it before comparison. With the default precision, for example, as_of = 2026-07-30T09:00:00.00000000000000000001Z and observed_at = 2026-07-30T09:00:00.00000000000000000009Z both become Decimal('1785402000.000000000000000000'), so the later observation is accepted and can leak future data. Fresh evidence against the earlier remediation is that its new Decimal addition remains context-limited despite the unrestricted \d+ fraction; compare a lossless representation or reject excess precision.
Useful? React with 👍 / 👎.
| parsed = _datetime.fromisoformat( | ||
| text[:-1] + "+00:00" if text.endswith("Z") else text | ||
| ) |
There was a problem hiding this comment.
On Python 3.10, which the package declares as supported, datetime.fromisoformat() only accepts fractional seconds with three or six digits, while this contract's regex and schema permit any non-empty digit sequence. Consequently valid timestamps such as .1Z, .12Z, or nanosecond .123456789Z are rejected on Python 3.10 but accepted on Python 3.11, making manifest validity interpreter-dependent; validate the whole-second portion separately from the unrestricted fraction.
Useful? React with 👍 / 👎.
| "path": { | ||
| "type": "string", | ||
| "minLength": 1, | ||
| "pattern": "^(?!/)(?!.*//)(?!.*/$)(?!.*\\\\)(?!.*\\u0000)(?!.*(?:^|/)\\.{1,2}(?:/|$)).+$" |
There was a problem hiding this comment.
When a member path contains a POSIX-valid line terminator, such as data/a\nb, _member_path() accepts and canonicalizes it, but this schema pattern rejects it because . does not match line terminators. Consumers that prevalidate canonical manifests against the bundled schema will therefore reject evidence accepted by the public Python validator; either reject these characters in _member_path() or express the same accepted character set in the schema.
Useful? React with 👍 / 👎.
|
Final current-head Codex review on |
1 participant
Scope
research_input_manifest.v1Python contractExact three-file change only. No F1/P3, provider, data capture, materialization, publication, consumer, broker, order, account, capital, position, leverage, cloud, deploy, or live behavior.
Evidence
5a83f508552bb68115123aa378795325094a7b52