Repository navigation
feat: observation criteria with taxonomy and run/iteration subject (#1156) - #1206
Draft
Cedric Vidal (cedricvidal) wants to merge 26 commits into
Conversation
…config, docs Add kind:gate|observation discriminator + hard-coded taxonomyElementId to criteria; record-only per-iteration boolean observations evaluated post-run by the pp-taxonomy handler (reuses judge via JudgeClient, dependsOn pp-atif) and stored as turn.observationResults. Request-level opt-in via observations[]. CLI, migration 021, deploy wiring, config seeds, and docs included. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…bservations tab Carry criterion kind + taxonomyElementId, request observations, and turn.observationResults through the portal: kind selector + taxonomy picker on criteria pages, observations multi-select on Submit Run, and a Run-detail Observations tab grouped by dimension. Includes Storybook stories. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Proves the judge consumes atifUrl: a trajectory-only observation (dependency_added_then_removed) resolves passed:true WITH the synthetic agent_trajectory_atif tool call and passed:false WITHOUT it. Excluded from default vitest; runs via vitest.integration.config.ts with a Copilot token. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…nomy registers
The Dockerfile copied only handler.yaml, so the register-handler-taxonomy
job (HANDLER_YAML_PATH=apps/workers/post-processor/handler.taxonomy.yaml)
failed with ENOENT in the container, exit 1, gating the post-processor
(hard depends_on service_completed_successfully). Use a handler*.yaml glob
in both builder and prod stages so every handler manifest is baked in.
Verified in compose: register-handler-taxonomy now exits 0 ("Registered
pp-taxonomy successfully") and the post-processor loads both the atif and
taxonomy handlers.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…aches judge The post-processor service used the *common-env anchor, which omits JUDGE_SERVICE_URL (only *worker-env defines it). pp-atif never called the judge so the gap was invisible, but pp-taxonomy (issue #1156) evaluates observation criteria via JudgeClient and fell back to localhost:3002 inside the container. Set JUDGE_SERVICE_URL=http://judge:80 explicitly. k8s is unaffected: post-processor.yaml already pulls JUDGE_SERVICE_URL from the worker-config ConfigMap via envFrom. Verified in compose: the containerized post-processor reached the judge container, which loaded the ATIF and wrote run.turns[].observationResults (dependency_added_then_removed -> passed:true). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… vs k8s In k8s the post-processor inherits JUDGE_SERVICE_URL from the worker-config ConfigMap via envFrom; in docker-compose the service uses the common-env anchor (not worker-env), so JUDGE_SERVICE_URL is set explicitly on the post-processor service. Reflects the compose fix in 3f2e98cc. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…not run.observations The API submit path persists the selected observation-criteria ids at the request document top-level (RequestDocument.observations, types.ts), but the TaxonomyHandler read them from doc.run.observations. They never met, so pp-taxonomy no-op'd on every real submitted run and never wrote turn.observationResults. Read doc.observations (top-level) instead; turns still come from doc.run.turns. Rename the local RunDoc interface to RequestDoc and document the placement. Update the handler unit tests to put observations at the canonical top-level, and add a regression test asserting that legacy run.observations is ignored (no-op), which would have caught this. Verified end-to-end on a real submitted run through the containerized stack: turn.observationResults is written with passed:true and feedback citing the mid-run install/uninstall trajectory (the judge consumed atifUrl). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ervations subsection The run-detail tab is now labeled "Taxonomy" and its content nests an "Observations" sub-section (heading + description) around the per-dimension ObservationResultsPanel, so the tab can host the broader taxonomy view with observations as one section within it. The internal tab value stays "observations" to preserve routing/state. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ervations tab URL to taxonomy The Enrichment badge read the legacy single `run.postProcessorStatus` scalar, which every post-processor handler overwrites on its own success. It therefore flipped to "Enriched" as soon as pp-atif finished, while pp-taxonomy was still computing observations. Derive an aggregate enrichment status from `run.handlerStatus` (the #1147 per-handler DAG state): stay "Enriching…" until every handler is terminal, treating pp-taxonomy as pending whenever observations are configured (which also covers the gap before the scheduler dispatches it). Use the derived status for both badge placements and the detail poller so taxonomy results auto-appear. Falls back to the legacy scalar for pre-DAG runs. Also rename the run-detail tab slug "observations" -> "taxonomy" so the URL reads /runs/:id/taxonomy, with an in-place redirect for legacy /observations deep-links. Proven live on the containerized stack: a real run held "Enriching…" from pp-atif done (legacy scalar already "done") through pp-taxonomy queued, and flipped to "Enriched" only once pp-taxonomy completed ~32s later. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… set (#1156 P10) Fixes a non-deterministic false negative on trajectory observations (e.g. `dependency_added_then_removed`): pp-taxonomy fed the judge the entire ~86 KB ATIF as ONE opaque synthetic tool call, so `read_tool_outputs`' 2000-char preview only ever showed the ~26 KB system prompt and the mid-run npm calls were buried tens of KB deep — the LLM found them only by luck. Replace the opaque blob + the two legacy tools (`read_tool_outputs`/ `get_tool_output`) with ONE normalized `AgentTrajectory` model and a unified, source-agnostic tool set used by BOTH gates (HAR `ToolCall[]`) and observations (parsed ATIF): - new `agent-trajectory.ts`: `fromToolCalls`/`fromAtif` normalizers + five `defineTool`s — two symmetric pairs (definitions: `list_agent_tools`/ `get_agent_tools`; calls: `list_agent_tool_calls`/`get_agent_tool_calls`) plus `get_trajectory_overview`. Outputs larger than INLINE_LIMIT (16 KB) spill to a per-evaluation temp dir created OUTSIDE the workspace and are returned as an absolute `responseFile`/`descriptionFile` the judge reads with `read_file`. - `index.ts`: build the trajectory at the `/evaluate` boundary (ATIF wins as superset; tool-calls otherwise); non-fatal on load failure. - `judge-strategies.ts`: thread `trajectory` through both strategies; extend `createFileTools` with `extraReadRoots` (read_file/file_exists may read the out-of-workspace spill dir; list_directory/search_files stay workspace-only); create+cleanup the spill dir in `runCopilotSession`; swap TOOL_OUTPUTS_GUIDANCE → TRAJECTORY_GUIDANCE. Gate-parity guard preserved (`trajectory.calls.length > 0`). - `judge-agent.ts`: `EvaluationInput.toolCalls` → `trajectory?`. Tests: new `agent-trajectory.test.ts` (normalizers, 5 tools, out-of-workspace spill, strict-superset-over-HAR contract); `judge-strategies.test.ts` (isWithinAnyRoot + extraReadRoots + new guidance/tool names); `observation-trajectory.integration.test.ts` rewritten to pass `fromAtif(...)`; `judge-evaluation.integration.test.ts` extended with a gate clear-PASS/clear-FAIL regression guard built from `fromToolCalls(...)`. Verified live on the containerized stack: the same observation that flipped non-deterministically now resolves passed:true 5/5 WITH the ATIF and false 3/3 WITHOUT it, citing `npm install left-pad` / `npm uninstall left-pad`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…vationResults (#1156) Introduce a kind-dependent `subject` discriminator on criteria so observation criteria can be evaluated either per-iteration (today) or once over the whole run. Gate criteria are always `iteration`. Adds: - CriterionSubject type + CRITERION_SUBJECTS + isCriterionSubject guard - subject on CriteriaConfig; run-level observationResults on RunState - zod schemas (optional, no flat default); kind-dependent default applied in the store (observation -> run, gate/legacy -> iteration) - subject threaded through criteria-store create/update/find + both providers - atifUrls[] on JudgeEvaluateRequest for merged whole-run trajectory - migration 022 backfills subject kind-aware (gate/legacy -> iteration, observation -> run) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- routes/criteria: reject subject:run for gate criteria (400); allow run| iteration for observations on create + update - llm: authoring assistant returns suggestedSubject; heuristic maps cross- iteration phrasing -> run, end-state/per-step -> iteration; defaults run for observations when the model is unavailable - endpoints test: subject create + run-subject-gate rejection - regenerated OpenAPI snapshot (subject enum + run-level observationResults) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- agent-trajectory: mergeTrajectories(list) concatenates calls in iteration order and unions tool definitions by id; fromAtifList helper - index: accept atifUrls[]; download all + merge into the single AgentTrajectory (single atifUrl path unchanged) - integration test: run-subject dependency_added_then_removed is false per iteration but true on the merged trajectory (GITHUB_TOKEN-gated) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…on (#1156) - taxonomy-handler: resolve selected observations with their subject; split into iteration vs run buckets. Iteration-subject -> per-turn observationResults (unchanged). Run-subject -> evaluate once (final snapshot + every turn's atifUrl merged + full conversationHistory) -> run-level run.observationResults. Runs once because the handler runs once per run (the scheduler never sets message.iteration), so no idempotency guard needed. - HandlerContext gains criteriaCollection so handlers can resolve criterion metadata by id; wired in post-processor + atif-handler test contexts Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…#1156) - wizard: Run/Iteration toggle (shown for observations, prefilled from AI suggestedSubject, user-overridable); subject in create payload + reset - kind copy reworded to convey STEERING vs record-only: gate steers the agent (pass/fail feedback shapes its next iteration), observation records evidence never shown to the agent — in both the wizard and the inline editor - RunDetail Taxonomy tab: new Whole-run observations subsection (from run.observationResults) above the per-iteration matrix, grouped by dimension - CriteriaSubjectBadge on list + detail; subject in types + api payloads - Storybook: subject-badge story Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…1156) - yaml-mappers: round-trip subject - criteria command: --subject <run|iteration> on create/update; show subject in list/get/export/import - run-get-action: render whole-run run.observationResults block alongside the per-iteration ones - updated cli-commands snapshot for --subject Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Document the new subject discriminator, the kind-dependent default and gate restriction, run-level run.observationResults storage, and the merged whole-run trajectory evaluation across post-processing.md, app-design.md, criteria-provider.md, and overview.md. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 247cbe24-e22c-4f4c-b3e4-e71ce03f4669
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 247cbe24-e22c-4f4c-b3e4-e71ce03f4669
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 247cbe24-e22c-4f4c-b3e4-e71ce03f4669
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 247cbe24-e22c-4f4c-b3e4-e71ce03f4669
Test Results (Node.js 22)test: Run #308
🎉 All tests passed! |
Cedric Vidal (cedricvidal)
added a commit
that referenced
this pull request
Oct 21, 2026
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 92d03055-789e-48b7-9a44-6de5b357f486 # Conflicts: # packages/shared/package.json
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 247cbe24-e22c-4f4c-b3e4-e71ce03f4669
Test Results (Node.js 22)test: Run #312
🎉 All tests passed! |
6 tasks done
Test Results (Node.js 22)test: Run #321
🎉 All tests passed! |
2 of 6 tasks
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 247cbe24-e22c-4f4c-b3e4-e71ce03f4669
Test Results (Node.js 22)test: Run #335
🎉 All tests passed! |
Test Results (Node.js 22)test: Run #348
🎉 All tests passed! |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 247cbe24-e22c-4f4c-b3e4-e71ce03f4669
Test Results (Node.js 22)test: Run #421
🎉 All tests passed! |
Cedric Vidal (cedricvidal)
added a commit
that referenced
this pull request
Oct 28, 2026
Normally merge exact published #1206 parent 2daf9af while preserving the original 15-path Wizard feature patch and both inherited shared subpath exports. No new UI or Taxonome integration behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: d0c36bff-9146-4622-aaf0-858de5876813
Cedric Vidal (cedricvidal)
changed the base branch from
cedricvidal-microsoft/pp-dag-orchestration
to
cedricvidal-taxonome-foundation
October 28, 2026 07:45
Test Results (Node.js 22)test: Run #430
🎉 All tests passed! |
2 of 6 tasks
Merge the latest fully green Taxonome foundation parent into #1206 while preserving the existing observation criteria changes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a050b3c1-0d66-4d8f-be86-04e0901f9a25
Test Results (Node.js 22)test: Run #512
🎉 All tests passed! |
This branch has not been deployed
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.
Summary
Preserves the existing record-only Observation criteria feature: gate/observation kind, static taxonomy labels, run/iteration subjects, Judge evaluation, and Portal/CLI parity. Production Taxonome evaluation and Taxonome-aware readout adoption remain deferred.
Fixes: #1156
Restack: now directly based on #1595,
cedricvidal-taxonome-foundationat2ef7ea8d5069e8d782c66886c515bdda8ed0001f, in native stack 1596. Head2daf9af5a2444feedcde5e5cd5a1d5707d786e56normally merged that parent into the preserved original observation head. The PR-only path set remains exactly 75 paths; all 73 nonoverlapping file entries are byte-identical to the frozen original feature, and the two shared-export/documentation overlaps are additive. No published history was rewritten.Original imported implementation description (historical, preserved verbatim; not current restack validation)
Demos
Before
N/A — parent synchronization introduces no new user-visible behavior; the imported source description contains no before recording.
After
N/A — no new recording was requested or produced for this publication-only synchronization.
Testing
Actual parent-synchronization validation at
2daf9af5a2444feedcde5e5cd5a1d5707d786e56:pnpm --filter shared build— passed.pnpm exec vitest run apps/api/src/endpoints.test.ts apps/api/src/prompt-request-transport.test.ts packages/shared/src/criteria/criteria-store.test.ts packages/shared/src/schemas/schemas.test.ts --reporter=default— passed, 364 tests / 4 files.pnpm exec vitest run apps/judge/src/agent-trajectory.test.ts apps/judge/src/judge-strategies.test.ts apps/judge/src/observation-trajectory.integration.test.ts --reporter=default— passed, 115 tests / 2 effective files. Default Vitest excludes the integration file; it is not counted here.pnpm exec vitest run --config vitest.integration.config.ts apps/judge/src/observation-trajectory.integration.test.ts --reporter=default— 1 harness test passed, 2 credential-gated tests skipped withoutGITHUB_TOKEN; no claim of new live/model acceptance.pnpm exec vitest run apps/workers/post-processor/src/handlers/taxonomy-handler.test.ts apps/workers/post-processor/src/post-processor.test.ts apps/workers/post-processor/src/handlers/atif-handler.test.ts --reporter=default— passed, 20 tests / 3 files.pnpm build:cliandnode apps/cli/dist/scope.mjs --version— passed; version0.0.0-dev.pnpm exec vitest run --config vitest.integration.config.ts apps/cli/src/bundle.integration.test.ts apps/cli/src/utils/update-check.integration.test.ts --reporter=default— the effective integration selection passed, 24 tests / 2 files. The original combined invocation also listedapps/cli/src/run-get.test.ts, which the integration config excluded; it was run separately below.pnpm exec vitest run apps/cli/src/run-get.test.ts --reporter=default— passed, 5 tests / 1 file.pnpm exec vitest run apps/portal/src/lib/enrichment.test.ts apps/portal/src/components/ui/stories.play.test.tsx --reporter=default && pnpm --filter portal exec tsc -b && pnpm --filter portal build— passed, 41 tests / 2 files, real Portal typecheck and Vite production build; existing Browserslist/chunk warnings only.pnpm --filter api build && pnpm --filter judge build && pnpm --filter post-processor build— all three builds passed.The imported description above retains its original historical validation/non-goal wording. These fresh checks do not claim a new paid/live semantic or CosmosDB acceptance. Native CI is tracked on exact heads and actual synthetic merge checkouts separately; current upper-stack upstream-main baseline failures are not concealed by changing this branch or claiming the whole stack is green.
Documentation and compatibility
Preserves the existing Observation API/CLI, run/iteration storage, migrations, and UI behavior.
docs/architecture/app-design.mdretains both the Observation feature and inherited standalone Taxonome documentation;packages/shared/package.jsonretains both./observation-taxonomyand the parent./schemas/requestexports. No new migration, domain model, service dependency, Taxonome production handler, or report-consumer integration is introduced by this parent synchronization. Existing migrations 021/022 are preserved; their original live-validation status is historical, not newly asserted. The direct PR base changed from #1147 to #1595; #1147 remains an ancestor.Checklist
up()/down()and keep it CosmosDB-compatible. No additional database change; original migrations remain unchanged.NOTICE/NOTICE-REVIEW.txtwithpnpm noticeas needed. Inherited Taxonome dependencies/NOTICE are owned and validated by feat(taxonome): add standalone foundation below production consumers #1595.