docs(proposal): add How section to #214 token usage response headers#586
Conversation
praxis-bot
left a comment
There was a problem hiding this comment.
Review Summary
The How? section is well-structured and follows the proposal template conventions. The design decisions are clear and well-justified with cross-references to related PRs. A few items need attention before merging.
Findings
| Severity | Description |
|---|---|
| Large | X-Praxis-Token-* naming conflicts with reserved internal header convention (see inline) |
| Medium | Data flow diagram shows explicit Some(500) for total, but set_token_usage auto-computes it (see inline) |
| Small | Graduation criteria jump (see inline) |
| Nit | Goals list still claims streaming support but streaming is an open question (see inline) |
Key Observation
The most significant finding is the tension between X-Praxis-Token-* as a client-facing response header and the existing x-praxis-* reserved header convention, which is documented and enforced as proxy-internal only. The reserved header stripping in response_filter.rs runs before filters execute, so filter-injected x-praxis-* headers technically survive to the client. But this creates an architectural inconsistency that should be explicitly addressed in the proposal before implementation proceeds.
(Please disregard the earlier empty test review above.)
Reviewed with praxis-bot
9a801ec to
26bec8d
Compare
praxis-bot
left a comment
There was a problem hiding this comment.
PR Review: docs(proposal): add How section to #214 token usage response headers
Adds the implementation approach (How?) section to the token usage response headers proposal, resolving the header naming question and detailing requirements, design, data flow, and pipeline ordering.
Overall Assessment
Solid proposal that follows the established template structure (Requirements + Design). The header naming rationale is thorough and well-reasoned. A few technical inaccuracies in the data flow diagram and a missing context note about set_token_usage() should be addressed before this guides implementation.
Findings Summary
| Severity | Count |
|---|---|
| Medium | 2 |
| Small | 3 |
| Nit | 2 |
All findings were placed as inline comments.
Automated review by praxis-bot
4ff7ed0 to
aa77cc9
Compare
…nse headers Add implementation details covering requirements, design decisions (string-based metadata transport, all-or-nothing injection, compile-time header names, X-Praxis-Token-* prefix), pipeline ordering, and streaming open question. Signed-off-by: noalimoy <nlimoy@redhat.com>
1d0f4c1 to
bcecafa
Compare
Summary
Add the implementation approach section to the token usage response
headers proposal, completing the What/Why/How structure.
Changes
## How?section with requirements and designinjection, compile-time header names, pipeline ordering
X-Praxis-Token-*)Related Issues
Relates to #214