Skip to content

plan: #3134 - #348

Merged
PolyphonyRequiem merged 1 commit into
plan/3131from
plan/3131-3134
May 13, 2026
Merged

plan: #3134#348
PolyphonyRequiem merged 1 commit into
plan/3131from
plan/3131-3134

Conversation

@PolyphonyRequiem

Copy link
Copy Markdown
Owner

requests_parent_change: false
ancestor_plan_generations:
root: 1

Plan for #3134 (root #3131)

Promotes plan/3131-3134 into plan/3131.

AB#3134

@PolyphonyRequiem PolyphonyRequiem left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Polyphony plan reviewer (advisory)

Summary: Tight, well-scoped issue plan for the sufficiency-judge + deep-tier escalation. It correctly inherits schemas from issue-2, names the cap default and override path, and lists harness scenarios that map 1:1 to the routing branches. Main concerns are around (a) where the per-topic escalation counter physically lives in a stateless sub-workflow, (b) cite-both-sources being asserted only on the deep path while AC #3 is phrased unconditionally, and (c) the absence of an explicit failure mode if deep-researcher tools are unavailable.

Strengths

  • Clear disjoint-surface note vs issue-4, with an explicit rebase-not-merge instruction — exactly the kind of guidance the executor wave needs.
  • Profile schema extension is minimal and additive: optional field, sensible default, 0-disables semantics called out, negative-value validation specified.
  • Harness scenarios enumerate the four meaningful routing paths (sufficient / insufficient-under-cap / cap-exhausted / cap-zero) — good coverage of the state machine.
  • Honors the parent's "no tier-pinned models" decision and defers rigor-bar work to issue-6, so the scope stays inside one PG.

Concerns

  • Per-topic counter location is under-specified. Step 5 says "track escalation count keyed by topic id within a single sub-workflow invocation" but doesn't say where: workflow state.*, an agent-emitted field threaded through routes, or a script node. In a conductor sub-workflow this matters for re-entry semantics and for the route condition in step 2 to be expressible without a hard-coded state name. Pin the mechanism.
  • AC #3 ("findings cite both archive and live sources when needed") is phrased as a global criterion but is only verifiable on the escalation path. Either scope it to the deep-tier output explicitly, or add the symmetric criterion that archive-sufficient outputs cite archive sources only — otherwise the test matrix is ambiguous.
  • No failure-mode AC for the deep tier. What happens if web/GitHub/MCP tools are unreachable, rate-limited, or return empty? The plan should say whether that collapses to archive-only with a marker (mirroring cap-exhausted) or surfaces an error to the architect. Today the harness scenarios don't cover this.
  • Verdict schema is named but not pinned. { sufficient, gaps, rationale } is good shape, but gaps element type is unspecified. Issue-2 defined the findings-document schema; do the same here so the deep-tier agent has a contract to consume.
  • "Recorded 'cap reached' marker" in the cap-exhausted scenario is the only place this marker appears. If downstream consumers (architect in issue-5, archivist in issue-4) need to distinguish capped-archive-only from naturally-sufficient-archive-only, name the marker field in the findings-document schema extension, not just in the test description.

Suggested changes

  • Add a one-line "Counter mechanism" bullet under Proposed Approach naming the conductor primitive (e.g. workflow state field, agent-output passthrough) used to carry the per-topic count, and reference it from the route condition in step 2.
  • Split AC #3 into two ACs: "deep-tier output cites at least one live source in addition to any archive citations" and "archive-sufficient output is unchanged from issue-2's contract."
  • Add an AC + harness scenario for deep-tier tool failure (e.g. tool throws / returns empty) — degrade-to-archive-with-marker is the simplest contract and matches cap-exhausted.
  • Pin the verdict schema (gaps: string[] or richer) and the "cap reached" marker as a named field in the findings document, so issue-4/issue-5 have a stable contract to read.
  • Confirm none of the new YAML/script introduces hard-coded ADO state name strings ("Done", "Active", etc.) — the routing here is intra-workflow so it should be naturally clean, but worth an explicit no-op check given the recent regression history.

This is an advisory comment from the polyphony plan_reviewer agent. It does not vote — human review is the merge gate.

@PolyphonyRequiem

Copy link
Copy Markdown
Owner Author

polyphony:approve 0f49d41

@PolyphonyRequiem
PolyphonyRequiem merged commit 151af43 into plan/3131 May 13, 2026
@PolyphonyRequiem
PolyphonyRequiem deleted the plan/3131-3134 branch May 13, 2026 20:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant