Files
SuperBizAgent-java/openspec/changes/session-run-trace-isolation/decisions.md
T

15 KiB

Decisions: session-run-trace-isolation

sm-flow State

  • Checkpoint: Apply / Phase 5 ready
  • Scale: complex
  • Capability source: sm-flow built-in protocol for context/proposal; grill decisions are recorded from the confirmed user discussion in the issue thread.
  • Change slug: session-run-trace-isolation

Entry Summary

Problem: the same sessionId currently represents both multi-turn conversation context and one persisted diagnosis trace. Multi-turn Chat E2E proved that Redis context behaves correctly, but MySQL trace rows from different rounds are mixed under one session_id.

Expected result: split session metadata from per-run execution state, expose runId as the official run identifier, and make trace, feedback, evaluation, demo scripts, Trace UI, and AIOps read/write by run.

Known modules: Flyway/JPA entities/repositories, ChatService, ChatController, AiOpsService, DiagnosisTraceService, EvaluationService, FeedbackService, CaseLibraryService, AgentLoggingHook, ToolInvocationRecorder, SessionContextHolder, demo scripts, static Trace UI, MVP docs.

Context Collection

devflow/index.md: relevant entries found.

Relevant historical decisions:

  • session-storage: current trace persistence is diagnosis_session + agent_step + tool_invocation; sessionId propagation uses RunnableConfig.metadata with SessionContextHolder fallback for tools; AIOps records child agents only.
  • confidence-feedback: FeedbackService writes feedback, useful feedback creates case_library, and feedback must not change execution status.
  • mvp-demo-trace-acceptance: Trace API is read-only and demo artifacts/scripts are part of acceptance.
  • aiops-traceable-diagnosis-entry: /api/ai_ops is an SSE entry point that accepts optional alert payload and exposes sessionId.
  • data-model.md: current case_library.diagnosis_id maps to diagnosis_session.session_id; this must be treated as legacy data after the change.
  • session-trace-lifecycle.md: current docs already list run id as a future enhancement for multi-run sessions.

OpenSpec inputs that must be carried forward:

  • Current specs mention diagnosis_session directly in trace, verifier, evidence, and demo requirements; new specs must either supersede or preserve compatibility for those requirements.
  • GET /api/diagnosis/{sessionId}/trace must remain read-only.
  • Baseline/eval checks must expect changed counts only when the change is explained by run isolation.

Question Pool

# Question Mode Status
Q1 Should this be an added field on existing diagnosis_session, or split tables? user-interview confirmed: split chat_session and diagnosis_run, keep existing trace detail tables
Q2 What is metadata in chat_session? user-interview confirmed: session directory fields only, not full message body
Q3 Is "message body" the full conversation history? user-interview confirmed: full conversation history stays in Redis SessionContext.messageHistory for now
Q4 Should runId be an official API field? user-interview confirmed: yes
Q5 How should trace work without runId? user-interview confirmed: default to latest run for compatibility
Q6 Should Feedback without runId fail or fall back? user-interview confirmed: short-term fallback to latest run, long-term may tighten
Q7 Should simple Chat Q&A create a run? user-interview confirmed: yes, every valid /api/chat execution creates a run
Q8 Should AIOps be included? user-interview confirmed: yes, same run isolation semantics
Q9 Should there be a separate trace table? user-interview confirmed: no, current agent_step and tool_invocation are enough for this phase
Q10 What should happen to old diagnosis_session? user-interview confirmed: keep it for history/rollback, new code stops writing it after migration
Q11 Does the issue need demo/Trace UI support? user-interview confirmed: yes, minimal runId support
Q12 What extra risks were found by document review? evidence-driven reported and patched into ISS-010

Evidence-Driven Findings

  • Code evidence: SessionContext contains messageHistory and getMessagePairCount(), supporting the decision that Redis holds hot conversation history while MySQL stores auditable per-run query/answer.
  • Code evidence: CaseLibraryService.createFromSession currently deduplicates by session.getSessionId() and maps answer/query from DiagnosisSession; this must change for new run-based data.
  • Documentation evidence: mvp/architecture/data-model.md states case_library.diagnosis_id = diagnosis_session.session_id; this becomes transitional legacy semantics.
  • Documentation evidence: existing trace OpenSpec requires GET /api/diagnosis/{sessionId}/trace to be read-only; run resolution must preserve that invariant.
  • E2E evidence from ISS-010: two Chat rounds with the same sessionId resulted in one overwritten diagnosis_session row and mixed step/tool rows.

Confirmed Decisions

  • runId format: run- + full UUID.
  • Latest run ordering: diagnosis_run.created_at DESC, id DESC, not updated_at.
  • chat_session stores metadata: session_id, status, message_pair_count, created_at, last_active_at, expires_at.
  • Per-run long-term audit stores query and answer in diagnosis_run.
  • agent_step and tool_invocation retain session_id and add run_id.
  • Missing feedback runId returns fallbackToLatestRun=true plus actual bound runId.
  • Historical mixed data is not split into multiple true runs.

OpenSpec Backwrite Log

  • Created proposal.md with problem, scope, non-goals, context constraints, interface impact, and risks.
  • ISS-010 patched to clarify phase boundary, migration order, AIOps same-change requirement, feedback fallback response, case-library transitional semantics, and Redis/MySQL TTL boundary.
  • Glossary patched to include SessionContext.messageHistory and its persistence boundary.
  • Created design.md, specs/session-run-trace-isolation/spec.md, specs/mvp-demo-trace-acceptance/spec.md, and tasks.md.
  • Architecture audit found that ToolTraceSummaryService, ExecutorGatekeeperService, and EvaluationService also read tool rows by sessionId; Phase 2 tasks were updated to cover run-scoped reads.

Cross-Artifact Alignment

Check Result Evidence
issue/context -> proposal aligned proposal carries E2E problem, split-table solution, compatibility, AIOps, feedback, demo/UI, and baseline scope
proposal -> design aligned design records data model, API impact, migration plan, rollback, and key decisions
design -> specs aligned specs cover session/run split, Chat, trace, feedback, AIOps, migration, demo/UI, E2E, and baseline behavior
specs -> tasks aligned tasks implement schema, Chat write path, trace reads, feedback/case, AIOps, demo/UI/docs, and verification gates

Architecture Audit

Input to output chain:

Chat/AIOps request
  -> ChatController / AIOps endpoint
  -> ChatService / AiOpsService
  -> execution context(sessionId, runId)
  -> AgentLoggingHook -> agent_step
  -> ToolInvocationRecorder -> tool_invocation
  -> verifier/gatekeeper/evaluation summary reads
  -> diagnosis_run answer/self_evaluation/status/counts
  -> Trace API / Feedback / CaseLibrary / Demo / Trace UI

Audit conclusions:

  • Data ownership is clearer with chat_session owning conversation metadata and diagnosis_run owning execution state; agent_step and tool_invocation remain trace details owned by one run.
  • The highest coupling risk is execution-context propagation because hooks and tools currently use both RunnableConfig.metadata and SessionContextHolder.
  • The main read-path risk is missing one of the session-scoped consumers (ToolTraceSummaryService, ExecutorGatekeeperService, EvaluationService, trace, feedback, case creation).
  • Migration is additive and rollback-friendly until constraints are tightened; historical mixed data must be treated as compatibility data.
  • AIOps must complete before final archive because otherwise the system would still have one production entry point with mixed trace semantics.

Commit Gate Preflight

  • Interface impact level: L4.
  • Required artifacts: proposal, design, specs, tasks.
  • Strict OpenSpec validation: passed with openspec validate session-run-trace-isolation --strict.
  • .committed marker: created after successful commit gate.

Pre-apply Research

  • Reference migrations: V005__create_session_storage.sql, V008__add_answer_to_diagnosis_session.sql, V010__add_relevance_level_to_tool_invocation.sql.
  • Entity style: JPA entities use Lombok @Data, @Builder, @NoArgsConstructor, @AllArgsConstructor, @PrePersist, and @PreUpdate where timestamps need maintenance.
  • Repository style: Spring Data JPA repository interfaces with derived query methods returning Optional<T> or List<T>.
  • Test style: repository tests use @DataJpaTest, @AutoConfigureTestDatabase(replace = NONE), Flyway enabled, and ddl-auto=validate.

Phase 1 Apply Notes

  • Implemented additive migration V011__add_session_run_isolation.sql.
  • Implemented ChatSession / DiagnosisRun entities and repositories.
  • Added nullable runId fields to AgentStep and ToolInvocation.
  • Added run-scoped repository methods for step/tool lookup and counts.
  • Added DiagnosisRunRepositoryTest.
  • Verification evidence is recorded in phase-1-evidence.md.
  • Historical DB inspection found one orphan tool_invocation row without a matching diagnosis_session; it remains run_id = NULL because no reliable compatibility run can be inferred.

Document Review Follow-up

  • Clarified that Chat creates runs for the effective resolved sessionId, including requests where the server generates a session id.
  • Tightened legacy feedback fallback so fallbackToLatestRun=true and the actual bound runId are response fields, not log-only evidence.
  • Clarified AIOps SSE compatibility: emit a metadata message containing sessionId and runId before report content while preserving the existing content stream shape.
  • Clarified that run/session ownership is enforced by service-layer validation and indexed lookup in this change; database foreign keys are intentionally deferred to preserve compatibility with historical orphan detail rows and rollback.
  • Clarified that chat_session.expires_at is nullable directory metadata / best-effort TTL snapshot, not mandatory persisted conversation history.
  • Clarified document review findings before continuing Phase 2: OpenSpec task phases are authoritative over the older active issue phase sketch, and AIOps rule evaluation is stored in diagnosis_run.self_evaluation.aiops_rule_evaluation, not a separate table.

Document Review Follow-up Before Phase 3 Gate

  • Clarified AIOps SSE compatibility before Phase 5: keep SSE event name message, emit a JSON SseMessage with type=metadata, and preserve existing content message shape for report streaming.
  • Clarified Feedback API before Phase 4: request runId is preferred, response always includes the bound runId and fallbackToLatestRun, and wrong-session run binding uses the existing failed feedback response path.
  • Clarified run-list API before Phase 3 gate: GET /api/chat/session/{sessionId}/runs returns ApiResponse<List<RunSummary>>, returns an empty list for an existing session with no runs, and uses existing missing-session error behavior when no session/run data exists.

Phase 2 Apply Notes

  • Capability source: openspec-apply-change + sm-flow apply protocol. codebase-retrieval and LSP tools were not available in this session, so call-chain confirmation used OpenSpec context, rg, targeted file reads, compilation, focused tests, E2E, DB inspection, and logs.
  • Implemented unified Chat execution context carrying sessionId and runId through RunnableConfig.metadata and SessionContextHolder.
  • Switched valid Chat writes to create/update chat_session metadata and create one diagnosis_run per request.
  • Switched Chat run completion, failure, self-evaluation, metrics, verifier support reads, gatekeeper validation, and evidence scoring to run-scoped data.
  • Added /api/chat response runId and focused tests for valid run creation, invalid request no-run behavior, run-scoped trace consumers, and same-session multi-turn run creation.
  • Phase 2 gate evidence is recorded in phase-2-evidence.md.

Phase 3 Apply Notes

  • Capability source: openspec-apply-change + sm-flow apply protocol. The committed OpenSpec remained the execution source; a document review follow-up tightened DTO/SSE/listing contracts before the Phase 3 gate.
  • Implemented latest-run trace resolution using diagnosis_run.created_at DESC, id DESC.
  • Implemented exact trace lookup for sessionId + runId with run/session ownership validation.
  • Changed trace details to read agent_step and tool_invocation by run_id, while retaining a legacy diagnosis_session fallback for historical compatibility.
  • Added trace response fields for resolved runId, chat session metadata, run metadata, and per-row runId.
  • Added lightweight GET /api/chat/session/{sessionId}/runs backed by diagnosis_run summaries.
  • Added focused tests for latest trace, exact first trace, exact second trace, wrong-session rejection, missing session, read-only trace behavior, run listing, and legacy fallback.
  • Phase 3 E2E used Maven startup with profile mvp-demo; evidence is recorded in phase-3-evidence.md.

Phase 4 Apply Notes

  • Capability source: openspec-apply-change + sm-flow apply protocol. codebase-retrieval and LSP tools were still unavailable; call-chain confirmation used OpenSpec context, rg, targeted file reads, focused tests, API E2E, DB inspection, and logs.
  • Added FeedbackRequest.runId and FeedbackResponse.runId/fallbackToLatestRun.
  • Changed FeedbackController to pass runId through to FeedbackService.
  • Changed FeedbackService so new feedback prefers exact sessionId + runId, validates ownership, falls back to latest run when runId is omitted, and writes new feedback to diagnosis_run.feedback.
  • Preserved a legacy DiagnosisSession fallback only when no diagnosis_run exists, so old data can still receive feedback during the migration window.
  • Added CaseLibraryService.createFromRun, using diagnosis_run.run_id as the new automatic case_library.diagnosis_id; createFromSession remains the legacy session-id path.
  • Phase 4 evidence is recorded in phase-4-evidence.md.
  • Document review follow-up: clarified that the legacy DiagnosisSession feedback path only applies when no diagnosis_run exists for the session. It returns no bound runId and is not the same as latest-run fallback.