Repository navigation
Drop reason from TechreadException - #598
Merged
jmtts merged 1 commit intoSep 30, 2026
Merged
Conversation
The owner decided that a READ_INCOMPLETE warning tells the customer that a read is incomplete, never why (core-reader#2453). core-reader stopped sending the field in core-reader#2452, and no production release ever sent it. Remove it from the model, its docstrings and the techread CLI, which printed it. A message that still carries "reason" parses as before: the model keeps pydantic's default of ignoring unknown keys, and a test pins that. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H5VZH5w3jzcmxijpeamRaB
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The focused removal is consistently applied, preserves legacy wire compatibility, and has appropriate regression coverage.
Review effort: Balanced
Findings: None
What changed in this PR
Removes the unused reason field from TechreadException while preserving compatibility with legacy payloads.
Changes:
- Removes the model field and CLI rendering.
- Updates documentation and tests.
- Confirms legacy
reasonkeys are ignored during parsing.
| File | Description |
|---|---|
werk24/models/v2/internal.py |
Removes reason from the model and documentation. |
werk24/cli/commands/techread.py |
Stops displaying the removed field. |
tests/test_read_incomplete_warning.py |
Tests field removal and legacy payload handling. |
tests/test_cli_techread_command.py |
Updates expected CLI exception output. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
jmtts
approved these changes
Sep 30, 2026
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.
The owner decided on 2026-09-30 to drop
reasoncompletely (core-reader#2453). AREAD_INCOMPLETEwarning tells the customer that a read is incomplete, never why.History of the field:
testbranch, for 54 minutes. core-reader#2452 replaced that code.The change:
werk24/models/v2/internal.py: remove the field and its docstring lines. TheREAD_INCOMPLETEdocstring now says the warning never says why.ask_typestays.werk24/cli/commands/techread.py: stop printing, reason <x>. That line came in with Clearer first-run experience: size check, key handling, readable errors, JSON CLI, notebooks #591, after #2453 was written. Without this change,werk24 techreadwould crash on any message carrying an exception.reason."reason"parses, with the key ignored and never dumped. The model keeps pydantic's default of ignoring unknown keys.Compatibility. Nothing changes on the wire, because the server never sent the field. It is a removal from a released public model, though. Code on 2.8.0 that reads
exception.reasondirectly would getAttributeErrorafter upgrading;getattr(exc, "reason", None)keeps working. The docs-v2 changelog PR announces it.Merge order:
Tests.
.venv/bin/python -m pytest -q -p no:cacheprovider(Python 3.13, fromrequirements.txt,tests/requirements.txtand-e .): 999 passed, 5 skipped. The 5 skipped are the credentialed live tests.Questions for the owner
One question. It does not block the merge.
🤖 Generated with Claude Code
https://claude.ai/code/session_01H5VZH5w3jzcmxijpeamRaB
Generated by Claude Code