Conversation
|
@codex review — Please review the authored T89 same-payoff duration contract, required output validation, and reserved-ID admission before ordinary/FpML dispatch. This remains draft pending broader release validation. |
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. |
![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: 6ed08b255b
ℹ️ 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".
| try: | ||
| if is_t89_duration_proof: | ||
| from trellis.agent.task_manifest_validation import assert_executable_task_selection | ||
|
|
||
| # The reserved ID, not mutable provenance labels, identifies this | ||
| # proof. Direct callers cannot remove its required-output contract. | ||
| assert_executable_task_selection([{ |
There was a problem hiding this comment.
Move the reserved-ID validation ahead of the ordinary task setup. For a direct T89 call whose benchmark_contract is missing or whose call_dates are absent, task_to_semantic_contract() raises from _proof_legacy_semantic_contract() before execution reaches this try, so run_task() propagates an exception instead of returning its normal structured contract-failure result. This leaves part of the exact T89 contract outside the newly added admission boundary and can abort direct task runners rather than recording the rejected run.
Useful? React with 👍 / 👎.
| # OASDuration(None) has one implementation: parallel shifts, no OAS solve. | ||
| metadata.update({ | ||
| "measure": measure_name, | ||
| "derivative_method": "finite_difference", | ||
| "resolved_derivative_method": "parallel_curve_bump", | ||
| "bump_bps": bump_bps, |
There was a problem hiding this comment.
Do not manufacture the finite-difference provenance after accepting an unannotated result. OASDuration.compute() currently returns a plain float, so its empty metadata passes the optional conflict checks and these lines label any finite value as parallel_curve_bump; if that implementation regresses to a constant or delegates to the reference Duration calculation, T89 can still pass whenever the numbers agree, falsely certifying the intended independent OAS-duration lane. Have the measure return authoritative metadata and require it here before admitting the output.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🟡 Changes recommended
Require structured measure output validation before adding contract metadata.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds an executable T89 callable-bond duration comparison using the existing Hull–White fixture.
Changes:
- Adds strict T89 admission, runtime routing, and analytics comparison.
- Adds tests, manifest/baseline updates, and documentation.
- Moderate finding (2 votes): duration validation may accept unstructured finite results before adding provenance metadata.
File summaries
| File | Description |
|---|---|
trellis/agent/task_runtime.py |
Integrates T89 execution and comparison. |
trellis/agent/task_manifest_validation.py |
Enforces the T89 contract. |
trellis/agent/task_analytics.py |
Provides duration analytics and validation. |
trellis/agent/benchmark_contracts.py |
Recognizes the callable-bond fixture. |
tests/test_tasks/test_t89_callable_duration.py |
Covers T89 behavior and rejection cases. |
TASKS_PROOF_LEGACY.yaml |
Defines the executable T89 task. |
TASKS_PROOF_LEGACY_BASELINE.yaml |
Updates legacy debt fingerprints. |
LIMITATIONS.md |
Documents T89 scope and limitations. |
docs/user_guide/pricing.rst |
Documents callable duration behavior. |
docs/quant/differentiable_pricing.rst |
Documents duration semantics. |
docs/developer/task_and_eval_loops.rst |
Documents runtime validation and routing. |
doc/plan/active__legacy-task-migration-map.md |
Updates T89 migration status. |
Review details
- Files reviewed: 12/12 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if ( | ||
| metadata.get("output_unit", "years") != "years" | ||
| or metadata.get("unit", "years") != "years" | ||
| or metadata.get("status", "passed") != "passed" | ||
| ): |
T89 now authors an executable duration comparison on the existing named fixed-coupon USD callable-bond fixture. Both targets use Hull-White (a=0.1, sigma=0.01, 200 steps); the separate required output is effective duration in years at symmetric 25 bp discount-zero-rate shifts, holding OAS at zero.
The task analytics bridge delegates OASDuration(None, 25bp) and Duration(25bp) on the bound payoff. It preserves the current holder-PV anchor, checks units/status/finiteness/shock/derivative provenance, and requires the duration outputs even when the prices agree. This is a same-payoff, same-model identity, not an independent oracle or market-price OAS calibration.
Reserved normalized T89 admission rejects missing contracts independently of mutable corpus/manifest labels. A contradictory task kind cannot bypass that validation through the early FpML dispatcher; ordinary FpML requests are unchanged.
Validation:
Updated official quant/developer/user docs, L64, the migration map and debt baseline. Generated validation artifacts were archived outside the repository; only the 12 intended files are included.
Draft: full release validation and coordinated branch integration remain pending. QUA-1261 owns generic OAS state preservation, QUA-1264 owns generic comparison-output hardening outside this profile. QUA-1258 remains In Progress.