Repository navigation
docs(policy): clarify HTTP budget recording time - #449
kevin-orellana wants to merge 3 commits into
Conversation
## Problem The HTTP byte-budget example can read as a cap on bytes as they leave Box. Request bytes enter history only when their exchange ends, so pending exchanges and a newly admitted request can exceed the threshold. ## Solution Clarify recording time and overshoot in the sample comments and policy references. Distinguish definite failed delivery from partial or uncertain delivery, and define the recorded byte count. Add an editable timing figure for the review. Runtime code, policy expressions, public interfaces, thresholds, and stored history remain unchanged. ## Tests All 42 focused tests passed with --locked --all-features: temporal_egress_budget (6), temporal_egress_mitm_e2e (3), readme_examples (6), and site_manifest (27). They cover budget decisions, completion-time history, reference examples, and publication metadata. Policy expressions and Markdown code blocks are unchanged; git diff --check passed. The full workspace test command stopped at two unchanged mcp_alias_fallback fixture spawns with Text file busy. An independent control at the same base70eb3af reproduced that failure. Full workspace validation is not green locally; no launcher repair is included. The timing figure passed layout checks and visual inspection. The pinned draw.io renderer reproduced the existing statfs control image byte-identically.
github-actions
Bot
added
Area - policy-engine
documentation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| the ten-minute window. The gateway records the outcome when the exchange ends, including failure | ||
| or timeout. A refused request or a request that definitely failed to reach upstream does not | ||
| count. Pending exchanges do not count yet, so several requests can pass before earlier exchanges | ||
| end. Those requests can make the total exceed the limit. |
There was a problem hiding this comment.
Suggestion (non-blocking): This bullet is accurate, but the lead paragraph of the same section (line 186) still describes the rule the way the PR says it shouldn't be read.
Example: Line 186 says "At most sixty completed requests to one host in ten minutes. The sixty-first is refused…" Take 61 concurrent requests to api.github.com with nothing recorded yet. All 61 pass, because formerly within 10m sees zero ::response events, and each one records ::response when its exchange ends. That matches what this bullet now says. Also, "completed" in that lead (and in the @description) doesn't match the rule. The count has no output.status or delivery filter. So an exchange that timed out after a partial write (Partial/Indeterminate → ::response, see delivery_of in crates/policy/src/adapters/egress.rs) counts too. This bullet says that ("including failure or timeout"). The write_budget section above can say "completed" because its rule filters output.result: "completed". This rule has no such filter.
Suggestion: Reword the lead to describe recorded outcomes. For example: "A request to one host is refused once sixty exchanges with it have ended in the last ten minutes." Then the first thing a reader sees doesn't contradict the bullet. If you'd rather keep this PR to the bullets, that's fine too. Then please note it as a follow-up.
There was a problem hiding this comment.
Addressed in 148b2fa. The lead and @description now describe the threshold as "at least sixty response outcomes recorded", which matches the count over http:request::response with no status filter, and they say pending exchanges don't count. Nothing else in the repo copies the old description string, and the policy predicates are unchanged. This can be resolved.
|
Assessment: Approve (with one non-blocking suggestion inline) I checked the new wording against the gateway and policy code at Assumptions and rules checked that held
Optional follow-up, not required by the root The rewrite is clear and correctly separates recorded outcomes from enforcement strength. |
## Problem The rate-limit guide still describes a strict limit on completed requests in its opening paragraph and example description, although pending exchanges do not count and some failed exchanges produce response outcomes. ## Solution Describe the threshold in terms of recorded response outcomes and state that pending exchanges do not count. Keep the policy predicates unchanged. ## Tests All 27 site-manifest tests pass. The example code is unchanged except for its description string. git diff --check passes.
|
Assessment: Approve Re-reviewed at Checked at this head
The optional "at reply time" follow-up from the earlier summary is still open, and it doesn't block the merge. |
## Problem The review-only diagram appeared as repository files in the pull request. ## Solution Keep the image and editable source in GitHub review attachments and remove the repository copies. The four documentation changes remain unchanged. ## Tests Verified the attachment renders in the PR, the four prose blobs are unchanged, and git diff --check passes. No runtime or documentation text changed.
|
Assessment: Approve Re-reviewed at Checked at this head
The optional "at reply time" follow-up ( |
Human readable section
Problem: The HTTP byte-budget example can read as a cap on bytes as they leave Box. Box records request bytes when the exchange ends, so requests still waiting for a response can exceed the threshold.
Solution: Clarify when request bytes enter history, how the time window works, and why pending requests and the next request's size can take the total over the threshold.
Trade-off: The example remains a budget over recorded outcomes. This documentation change does not strengthen enforcement or change policy expressions.
Existing HTTP budget timing. Pending exchanges contribute no bytes until Box records their outcomes. This documentation clarification leaves runtime behavior unchanged.
Editable diagram.
Description
Clarify the byte-budget fixtures, policy reference, and HTTP rate-limit guide. The reference also distinguishes definite delivery failures, which produce error events, from completed, partial, or uncertain deliveries, which produce response events. The recorded byte count is the number of request bytes known to have reached the operating system.
Runtime code, policy expressions, event names, thresholds, and stored history remain unchanged.
Related Issues
No public issue. This clarifies the existing counting-rule contract.
Type of Change
Documentation update
Testing
Linux ARM64, based on
70eb3afc, with--locked --all-features:42 focused tests passed: 6 byte-budget, 3 gateway/streaming, 6 README examples, and 27 site-manifest tests.
cargo test --workspace --locked --all-features -- --show-outputran but stopped at two unchangedmcp_alias_fallbacktests withText file busywhile spawning a copied test executable. An independent control at the same unchanged base reproduced that fixture failure. The full workspace suite is not green locally; this PR does not change the launcher code or tests.The same nine launcher-fixture tests passed when run serially using the existing binary. This does not replace the failed full-workspace result.
git diff --checkpassed. Policy predicates are unchanged; the guide example's description now uses recorded-outcome wording. All 27 site-manifest tests passed again after that wording update.The diagram passed layout checks and visual inspection. Export used
rlespinasse/drawio-desktop-headless:v1.75.0, scale 2, border 20. A control export of the existing statfs figure was byte-identical.I ran the relevant suites (
just check, orcargo test --workspace --all-features)If I touched Rust, I ran
cargo fmt --allandcargo clippy --workspace --all-targets --all-features -- -D warningsChecklist
Prepared with AI assistance.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.