Repository navigation
feat(*): open the deck engine route on an attached template, not on a tier - #574
Conversation
… tier The route from Raven-Design to the Raven-PPT template engine used to open on the max tier alone, so the same request came back in three unrelated styles across the tiers, and the top one was built on a template the user never asked for. Every tier now designs its deck on Raven-Design; the engine is reached only when the user handed over a .pptx, either a bundled template picked in the web UI or one they attached themselves. A route declares that with needsFile, a suffix that joins needs and minTier as a third independent condition. Handed over means attached in this turn: a direct chat's media, or an attachment of the turn that the spawn task names by path or file name. The attachments are frozen beside the tier on the turn path in a ContextVar, so a DAG node or a spawn reads the same set its user sent. Task text alone never opens the route: a deck brief spells the deck's destination path, the format in words, or a stray file in the directory exactly like a template, and the first scan of this kind sent a no-template run into the engine on its destination path. What a tier buys is now how hard the lane thinks. Medium asks for low effort and max for xhigh, the highest word a model may know; the baseline inherits the host's setting as before. The design lane also retries an LLM call that dropped after streaming output, which is how a run died at its thirtieth iteration to an upstream idle timeout. The deck skill says to use what the user handed over first: their documents and data are the material, their pictures the pictures, and an attached .pptx is the engine's work, routed before the skill is read.
gloryfromca
left a comment
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: make attachment-name matching reject suffixes of different filenames.
I found one blocking defect, marked inline.
I covered the full diff; AGENTS.md, CONTEXT-MAP.md, CONTEXT.md, and agents/README.md rules; spawn, DAG, and direct-chat callers; the prior tier/credential routing history; backward-compatible defaults and signatures; test changes (none were weakened to manufacture a pass); and the runtime/subagent architecture boundary.
Verification:
uv run pytest tests/test_subagent_routing_backend.py -q: 55 passed.- After
uv sync --frozen --all-extras, the broader affected suite (routing, design launcher, schemas, manager, direct chat, DAG runner, and turn loop): 662 passed. The first attempt before syncing extras stopped on two missing-design-engine fixture failures; both passed after the declared extras were installed. - Ruff check and format check on all changed Python files: passed.
- Source-language, large-file, commit-message, and
git diff --checkchecks: passed.
…ames it The match that decides whether a spawn task hands over an attachment was bounded only on the right, so the attachment brand.pptx was read as named by a task that wrote new_brand.pptx, and a brief that told the lane to ignore the attachment and save its own deck under a similar name opened the engine route anyway. The name is now bounded on the left as well, and the negative briefs include a prefixed output name and a hyphenated neighbour.
The decision log under the engine's docs folder was a working journal for the people building the deck lanes, written in one language for one team, and it grew past two hundred kilobytes. It is not documentation of the product and has no reader in a public repository; the maintainer keeps it outside the tree from here on. The two places in code that cited it by path say their reason in place instead.
gloryfromca
left a comment
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
The follow-up commit fixes the reported left-boundary false positive and adds focused negative regressions without weakening existing coverage. Exact absolute-path, basename-only, and punctuated attachment handoffs still work. I found no new findings.
I rechecked the full diff and delta; AGENTS.md and the relevant CONTEXT/agent documentation; spawn, DAG, and direct-chat callers; routing history; backward compatibility; test changes; and the runtime/subagent architecture boundary.
Verification:
- Affected suite: 662 passed.
- Direct boundary probes: reported underscore and hyphen false positives rejected; valid path/name handoffs accepted.
- Ruff check and format check: passed.
- Source-language, large-file, commit-message, and
git diff --checkchecks: passed.
gloryfromca
left a comment
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
The attachment-boundary fix remains correct, and the latest cleanup commit only removes the internal design journal and makes its two former references self-contained. No dangling references remain and I found no new findings.
I reviewed the complete latest diff and the new delta; AGENTS.md and relevant CONTEXT/agent documentation; callers and routing history; backward compatibility; test changes; and the runtime/subagent architecture boundary.
Verification:
- Latest-head targeted suite: 117 passed.
- Prior full affected suite on the functional fix: 662 passed.
- Direct attachment-boundary probes: passed.
- Ruff, source-language, large-file, commit-message, and
git diff --checkchecks: passed.
|
Not a blocker, two observations from the same pass as the thread on routing.py. Neither holds up acceptance; both are things a later reader would have to re-derive. (1) Max mode's user-facing text promises the top reasoning rung; the overlay declares the second one. This PR rewrites agents/raven-design/run.py:47 to "400 tool iterations at the highest reasoning effort the model offers" and repeats the claim in the source comment at run.py:191-192, while agents/raven-design/modes/max.json declares (2) The deleted design record is still cited 22 times, by number. a8685c8 removes plugins-dist/ppt-engine/docs/ppt-raven-design.md, which defined D24-D64 (its own line 3: "from D24 on, the ppt engine's design decisions are recorded here"; D1-D23 live in the retired fork tree and are unaffected). The commit cleans up exactly the two citations that spelled the file PATH -- plugins-dist/ppt-engine/raven_ppt/profiles/registry.py:23 and tests/test_ppt_engine_quiet_and_tier.py:3 -- and leaves every citation that names only a decision NUMBER. Four such numbers are still cited (D35, D38, D41, D52), 22 times across 13 files, of which 6 files are shipped engine source: plus 13 more in tests/_ppt_engine_skill_claims.py, tests/test_agents_ppt_launcher.py, tests/test_ppt_engine_gates_house_style.py, tests/test_ppt_engine_gates_registry.py, tests/test_ppt_engine_measure_adherence.py, tests/test_ppt_engine_plugin.py and tests/test_ppt_engine_template.py. At the merge base every one of them resolved to a heading in the repository; at this head none of them does. Nothing breaks -- these are comments and docstrings -- but "(D52)" as the whole justification for a gate's severity is only useful while D52 can be read. For the record on the third one I checked and did not file: the ppt-engine's own |
The right bound on the attachment-name match put the dot in the class of characters that continue a name, so a brief ending its sentence with brand.pptx and an ASCII full stop read as not naming the attachment, and the template the user attached never reached the engine. A dot now closes the name unless it opens a longer one: brand.pptx. and brand.pptx... name the file, brand.pptx.bak, brand.pptx-based and brand.pptx_copy do not. The spelling tests carry both sides. The Max tier's label says what it buys, xhigh, instead of promising the top of the ladder; xhigh is the rung kept because the providers this lane runs against spend the whole output window thinking at max.
|
(1) Fixed in 88028db: the Max label now says "at xhigh reasoning effort", and the source comment says why that rung and not (2) Agreed on making the choice once. The record leaves the repository by the maintainer's decision, so the 22 citations of D35/D38/D41/D52 will be replaced by the reason inline where they are load-bearing (the gate severities and the adapt removal) and dropped where they are not. That is a comment-only sweep across six engine files and seven test files, and it will land as its own chore change rather than widen this one. |
gloryfromca
left a comment
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
I independently verified the latest delta. Sentence-final punctuation after an attached filename now opens the route, longer filename suffixes and extensions remain rejected, and the Max label accurately describes the configured xhigh effort. The new cases extend coverage without weakening existing assertions. I found no new findings.
I rechecked the complete diff and latest delta; AGENTS.md and relevant CONTEXT/agent documentation; spawn, DAG, and direct-chat callers; routing history; backward compatibility; test changes; and the runtime/subagent architecture boundary. The other reviewer’s thread remains theirs to resolve.
Verification:
- Affected suite: 662 passed.
- Ruff check and format check: passed.
- Source-language, large-file, commit-message, and
git diff --checkchecks: passed.
3 participants
Summary
The route from Raven-Design to the Raven-PPT template engine used to open on the max tier alone. The same request then came back in three unrelated styles across the tiers, and the top one was built on a template the user never asked for. This PR moves the decision to the one thing that actually calls for the engine: a .pptx the user handed over.
SubagentRouteConfiggainsneedsFile(a suffix), a third independent condition besideneedsandminTier. Raven-Design's route to Raven-PPT now declaresneedsFile: ".pptx"and dropsminTier: "max". Every tier designs its deck on Raven-Design; the engine is reached only when a .pptx was handed over, either a bundled template picked in the web UI (companion PR feat(*): deck template picker and full-frame deck preview for the web ui #573) or one the user attached.turn_attachmentsContextVar is frozen besideturn_tieron the turn path. A direct chat's media all count; a spawn or DAG node counts an attachment only when its task text names that attachment's path or file name. Task text alone never opens the route: a deck brief spells the deck's destination path, the format in words, or a stray file in the directory exactly like a template, and the first text-only scan sent a no-template run into the engine on its destination path.llmRetryAfterOutput: trueon the design lane's defaults, matching the ppt lane. A max run died at its thirtieth iteration to an upstream idle timeout after the body had already streamed to the host; the lane now retries instead of failing the turn.Docs:
CONTEXT.mdadmission paragraph,agents/README.md, regeneratedschemas/subagent.schema.json.Also removed: the engine's internal decision log (
plugins-dist/ppt-engine/docs/ppt-raven-design.md), a single-language working journal for the team building the deck lanes that has no reader in a public repository. The maintainer keeps it outside the tree; the two code comments that cited it by path now state their reason in place.Type
Verification
uv run --frozen pytest tests/test_subagent_routing_backend.py tests/test_agents_design_launcher.py tests/test_config_schema.py tests/test_subagent_registry.py -q-> 130 passed (after rebase onto main).ruff check/ruff format --checkon the changed Python files -> clean.End to end on a full gateway: a high-tier request with a picked template logged
classified for 'Raven-PPT'; routing thereand the engine delivered a 20-page deck on that template in about 60 minutes; the same request without a template loggedno .pptx the user attached was handed over; running hereand stayed on Raven-Design. A brief that only spelled the deck's destination path stayed on Raven-Design.Effort labels probed against
z-ai/glm-5.3-flashover OpenRouter:low,high,xhighall accepted;maxspent the whole output window on reasoning, which is why the top tier is xhigh.Relevant tests pass locally
Relevant lint / type checks pass locally
User-facing docs or screenshots are updated when needed
Risk
User-visible: medium and high tier deck requests no longer differ in product, only in effort; a deck on a template now requires attaching one. A bound direct-chat handle to Raven-Design stays on Raven-Design even with a .pptx attached (the route is evaluated at dispatch), a known boundary.
needsFileis additive; routes that do not declare it are dispatched exactly as before.Rollback: revert the squash commit; no schema migration or storage change.
Security impact considered
Backward compatibility considered
Rollback path is clear for risky changes
Related Issues
N/A