fix: collapse the evidence-key gate and resolver into one decision
Three fix rounds each rebuilt the gate as a separate expression that merely agreed with the resolver on the inputs that round's tests used. Different expressions cannot agree in general, so the defect class stayed open while each reported instance closed. `findResolvableAsset(assets, key)` is now the single place that decides which Asset an evidence key resolves to. Every gate is `Boolean(findResolvableAsset(...)) || legacyKey(key)` via one shared composition, and every resolver returns what it returns: - validate-working-copy: the key gate and the decorative lookup (a last-wins Map against the resolver's first-wins find, so alt could be judged against a different Asset than the one rendered) - adapters/mock/project-public-render-model: gate and resolver - instant-preview: gate and descriptor resolver - createAssetCatalogResolver: the pixels, a fourth expression nobody had listed -- one Asset's caption could sit over another Asset's image Duplicate assetKeys are a contract violation but reachable through a paged list, so the choice is total and order-independent: newest updatedAt wins, tie-broken by id. InstantPreview's gate is no longer looser than the others. The un-loaded-asset case it was loosened for blanks either way; all the looseness bought was catalog-only keys rendering an empty gap with no message while validation said EVIDENCE_UNSUPPORTED. The test that pinned that divergence now asserts the consistent behaviour, and the false comment claiming a fix that did not exist is gone. idempotent() now maps a deterministic content failure to VALIDATION_STALE/409 instead of offering a retry that fails identically, and no longer caches uncharacterized internal failures -- reporting one as retryable while freezing it in the ledger meant the retry could never re-run. Adds tests/features/tech-log/evidence-key-agreement.test.ts: 144 adversarial (asset list, key) combinations asserting the agreement itself rather than examples. It reported 53 disagreements against the previous code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
DongHyeonka
co-authored by
Claude Opus 5
Claude Opus 5
parent
783e9b2cf1
commit
073fda87eb
@@ -16,6 +16,7 @@ import {
|
||||
stateForUploaded,
|
||||
} from "../../../src/features/tech-log/presentation/studio/components/asset-upload-dialog.tsx";
|
||||
import { DocumentEditorScreen } from "../../../src/features/tech-log/presentation/studio/components/document-editor-screen.tsx";
|
||||
import { InstantPreview } from "../../../src/features/tech-log/presentation/studio/components/instant-preview.tsx";
|
||||
import { StudioProvider } from "../../../src/features/tech-log/presentation/studio/studio-provider.tsx";
|
||||
import { createTechLogFeatureInstalledInput } from "../../../src/features/tech-log/adapters/create-tech-log-feature-input.ts";
|
||||
import { createMockStudioGateway } from "../../../src/features/tech-log/adapters/mock/mock-studio-gateway.ts";
|
||||
@@ -914,7 +915,14 @@ test("a READY asset whose publicPath satisfies a different key's legacy conventi
|
||||
// re-evaluated fresh inside createPreview against the now-mutated assets,
|
||||
// correctly rejects -- and that internal ContentFormatError must not leave
|
||||
// the port unwrapped.
|
||||
test("idempotent() wraps a raw domain error into a StudioGatewayError, never lets it escape the port", async () => {
|
||||
//
|
||||
// Fix round 4 amends the expected code. Round 3 mapped this to
|
||||
// STUDIO_UNAVAILABLE/500 with `retryable: true`, which invites a retry that
|
||||
// fails identically -- the Asset is gone, the projection will refuse again.
|
||||
// A deterministic content failure between validate and preview is exactly
|
||||
// what VALIDATION_STALE/409 means, and it is not retryable without
|
||||
// re-validating first.
|
||||
test("idempotent() maps a deterministic content failure to VALIDATION_STALE, never lets a raw error escape the port", async () => {
|
||||
const asset = assetFixture({ assetKey: "race-check" });
|
||||
const assets = new Map<string, Asset>([[asset.assetKey, asset]]);
|
||||
const gateway = createMockStudioGateway({ assets });
|
||||
@@ -947,20 +955,35 @@ test("idempotent() wraps a raw domain error into a StudioGatewayError, never let
|
||||
|
||||
assets.delete("race-check");
|
||||
|
||||
const expectStale = (error: unknown) => {
|
||||
assert.ok(
|
||||
error instanceof StudioGatewayError,
|
||||
`expected a StudioGatewayError, got ${String(error)}`,
|
||||
);
|
||||
assert.equal(error.code, "VALIDATION_STALE");
|
||||
assert.equal(error.status, 409);
|
||||
assert.equal(error.retryable, false);
|
||||
return true;
|
||||
};
|
||||
|
||||
await assert.rejects(
|
||||
gateway.createPreview(
|
||||
created.id,
|
||||
{ expectedVersion: created.version, validationId: report.validationId },
|
||||
{ idempotencyKey: "race-preview" },
|
||||
),
|
||||
(error: unknown) => {
|
||||
assert.ok(
|
||||
error instanceof StudioGatewayError,
|
||||
`expected a StudioGatewayError, got ${String(error)}`,
|
||||
);
|
||||
assert.equal(error.code, "STUDIO_UNAVAILABLE");
|
||||
return true;
|
||||
},
|
||||
expectStale,
|
||||
);
|
||||
|
||||
// Deterministic outcomes stay in the ledger: a same-key retry replays the
|
||||
// same 409 rather than re-running work that cannot succeed.
|
||||
await assert.rejects(
|
||||
gateway.createPreview(
|
||||
created.id,
|
||||
{ expectedVersion: created.version, validationId: report.validationId },
|
||||
{ idempotencyKey: "race-preview" },
|
||||
),
|
||||
expectStale,
|
||||
);
|
||||
});
|
||||
|
||||
@@ -1010,25 +1033,37 @@ test("EVIDENCE_NOT_FOUND is reachable through validateWorkingCopy directly: the
|
||||
);
|
||||
});
|
||||
|
||||
// Low note (reviewer): round 2's assets-only gate 1 in InstantPreview made a
|
||||
// CASE referencing a document-catalog-backed key blank the WHOLE preview
|
||||
// behind the error panel whenever that key's Asset was not (yet) in the
|
||||
// loaded `assets` array -- outside the Picker's 50-item page, or simply
|
||||
// because `assets` is still `[]` while `listAssets` is in flight. Fix round
|
||||
// 3 reverted InstantPreview's gate 1 to the merged catalog; this pins that
|
||||
// such a document now degrades the one unresolved figure to a placeholder
|
||||
// instead of failing the whole preview.
|
||||
test("Instant Preview degrades to a placeholder for a document-catalog-backed key with no loaded Asset, instead of blanking the whole preview", async () => {
|
||||
const user = userEvent.setup();
|
||||
const gateway = createMockStudioGateway();
|
||||
const assetGateway = gatewayOf([]);
|
||||
// --- Fix round 4 ---
|
||||
//
|
||||
// Rounds 1-3 each rebuilt gate 1 as a *separate expression* that happened to
|
||||
// agree with the resolver on the inputs the round's tests used. They cannot
|
||||
// agree in general, because they were never the same function. Round 4
|
||||
// collapses "can this key be rendered" and "which Asset does it render" into
|
||||
// one exported decision, `findResolvableAsset`, and puts every caller
|
||||
// (validation's gate, validation's decorative lookup, the mock projection's
|
||||
// gate and resolver, InstantPreview's gate, resolver, and pixel resolver) on
|
||||
// it. The tests below are the shapes that broke the round-3 pair.
|
||||
|
||||
// `MockStudioDependencies.assets` is keyed by whatever the caller chooses --
|
||||
// the gateway only ever reads `.values()`. Keying by `id` here is what lets a
|
||||
// test express the shape the whole round is about: two Assets that share one
|
||||
// `assetKey`.
|
||||
function duplicateKeyAssets(...assets: Asset[]): Map<string, Asset> {
|
||||
return new Map(assets.map((asset) => [asset.id, asset]));
|
||||
}
|
||||
|
||||
async function previewFor(
|
||||
assets: Map<string, Asset>,
|
||||
bodyMarkdown: string,
|
||||
slugSuffix: string,
|
||||
) {
|
||||
const gateway = createMockStudioGateway({ assets });
|
||||
const created = await gateway.createDocument(
|
||||
{
|
||||
kind: "CASE",
|
||||
title: "카탈로그로만 뒷받침되는 근거 미리보기",
|
||||
slug: "catalog-only-evidence-preview-check",
|
||||
summary: "로드된 Asset 없이 카탈로그만으로 뒷받침되는 키의 미리보기 동작을 확인합니다.",
|
||||
title: `중복 키 확인 (${slugSuffix})`,
|
||||
slug: `duplicate-key-${slugSuffix}`,
|
||||
summary: "하나의 assetKey를 공유하는 Asset이 둘일 때의 동작을 확인합니다.",
|
||||
topicId: FIXTURE_IDS.topicJpa,
|
||||
projectId: FIXTURE_IDS.projectBackend,
|
||||
relations: [],
|
||||
@@ -1037,25 +1072,276 @@ test("Instant Preview degrades to a placeholder for a document-catalog-backed ke
|
||||
environment: "env",
|
||||
reproduction: "repro",
|
||||
lastVerifiedOn: "2026-08-14",
|
||||
bodyMarkdown: `## 제목\n\n:::evidence key="${FIXTURE_IDS.fetchJoinCase}" alt="근거" caption="근거" zoom="false"\n:::`,
|
||||
bodyMarkdown,
|
||||
},
|
||||
{ idempotencyKey: "catalog-only-evidence-preview-create" },
|
||||
{ idempotencyKey: `duplicate-key-create-${slugSuffix}` },
|
||||
);
|
||||
const report = await gateway.validateDocument(
|
||||
created.id,
|
||||
{ expectedVersion: created.version },
|
||||
{ idempotencyKey: `duplicate-key-validate-${slugSuffix}` },
|
||||
);
|
||||
const preview = await gateway.createPreview(
|
||||
created.id,
|
||||
{ expectedVersion: created.version, validationId: report.validationId },
|
||||
{ idempotencyKey: `duplicate-key-preview-${slugSuffix}` },
|
||||
);
|
||||
const model = preview.renderModel;
|
||||
assert.equal(model.kind, "CASE");
|
||||
const figure = model.kind === "CASE"
|
||||
? model.bodyBlocks.find((block) => block.type === "EVIDENCE_FIGURE")
|
||||
: undefined;
|
||||
assert.ok(figure && figure.type === "EVIDENCE_FIGURE", "expected an EVIDENCE_FIGURE block");
|
||||
return { report, asset: figure.asset };
|
||||
}
|
||||
|
||||
// The third live instance the reviewer found: two READY Assets share one
|
||||
// `assetKey`, the first carries `publicPath: null`, the second a real path.
|
||||
// `∃a. P(a) ∧ Q(a)` says yes (the second one). `Q(first a satisfying P)` says
|
||||
// no, and the resolver falls through to the generic placeholder -- so
|
||||
// `validateDocument` returns VALID with zero issues, `createPreview` succeeds,
|
||||
// and `publishDocument` would snapshot a render model carrying
|
||||
// `publicPath: ""` with nothing anywhere reporting an error.
|
||||
test("two READY assets sharing one assetKey: the gate and the resolver agree, and the preview never carries an empty publicPath", async () => {
|
||||
const withoutPath = assetFixture({
|
||||
id: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaa1",
|
||||
assetKey: "dup-null-first",
|
||||
publicPath: null,
|
||||
updatedAt: "2026-08-14T00:00:00.000Z",
|
||||
});
|
||||
const withPath = assetFixture({
|
||||
id: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaa2",
|
||||
assetKey: "dup-null-first",
|
||||
publicPath: "/media/dup-null-first.svg",
|
||||
updatedAt: "2026-08-14T00:00:01.000Z",
|
||||
});
|
||||
|
||||
const { report, asset } = await previewFor(
|
||||
duplicateKeyAssets(withoutPath, withPath),
|
||||
':::evidence key="dup-null-first" alt="근거" caption="근거" zoom="false"\n:::',
|
||||
"null-first",
|
||||
);
|
||||
|
||||
assert.equal(report.status, "VALID", JSON.stringify(report.issues));
|
||||
assert.equal(asset.publicPath, "/media/dup-null-first.svg");
|
||||
assert.equal(asset.assetId, withPath.id);
|
||||
});
|
||||
|
||||
// Order-independence. The same two resolvable Assets in either array order
|
||||
// must resolve to the same one; `find`-based first-wins picks whichever the
|
||||
// caller happened to list first.
|
||||
test("duplicate resolvable assets resolve to the same asset in either array order", async () => {
|
||||
const older = assetFixture({
|
||||
id: "bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbb1",
|
||||
assetKey: "dup-order",
|
||||
publicPath: "/media/dup-order-older.svg",
|
||||
updatedAt: "2026-08-14T00:00:00.000Z",
|
||||
});
|
||||
const newer = assetFixture({
|
||||
id: "bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbb2",
|
||||
assetKey: "dup-order",
|
||||
publicPath: "/media/dup-order-newer.svg",
|
||||
updatedAt: "2026-08-14T00:00:01.000Z",
|
||||
});
|
||||
const body = ':::evidence key="dup-order" alt="근거" caption="근거" zoom="false"\n:::';
|
||||
|
||||
const forward = await previewFor(duplicateKeyAssets(older, newer), body, "order-forward");
|
||||
const reverse = await previewFor(duplicateKeyAssets(newer, older), body, "order-reverse");
|
||||
|
||||
assert.deepEqual(forward.asset, reverse.asset);
|
||||
assert.equal(forward.asset.assetId, newer.id);
|
||||
assert.equal(forward.asset.publicPath, "/media/dup-order-newer.svg");
|
||||
});
|
||||
|
||||
// The third rule with the same disease: `readyAssetsByKey` was built with
|
||||
// `new Map(...)` (last-wins) while the resolver used `find` (first-wins), so
|
||||
// EVIDENCE_ALT_REQUIRED could be judged against a different Asset than the
|
||||
// one actually rendered. Here the rendered Asset is `decorative` (empty alt
|
||||
// is legitimate for it) but the last-wins map judges the other one.
|
||||
test("EVIDENCE_ALT_REQUIRED is judged against the asset that actually renders, not a different one sharing the key", async () => {
|
||||
const rendered = assetFixture({
|
||||
id: "cccccccc-cccc-4ccc-8ccc-ccccccccccc1",
|
||||
assetKey: "alt-mismatch",
|
||||
publicPath: "/media/alt-mismatch-rendered.svg",
|
||||
decorative: true,
|
||||
updatedAt: "2026-08-14T00:00:02.000Z",
|
||||
});
|
||||
const shadow = assetFixture({
|
||||
id: "cccccccc-cccc-4ccc-8ccc-ccccccccccc2",
|
||||
assetKey: "alt-mismatch",
|
||||
publicPath: "/media/alt-mismatch-shadow.svg",
|
||||
decorative: false,
|
||||
updatedAt: "2026-08-14T00:00:01.000Z",
|
||||
});
|
||||
|
||||
const { report, asset } = await previewFor(
|
||||
duplicateKeyAssets(rendered, shadow),
|
||||
':::evidence key="alt-mismatch" alt="" caption="근거" zoom="false"\n:::',
|
||||
"alt-mismatch",
|
||||
);
|
||||
|
||||
assert.ok(
|
||||
!report.issues.some((issue) => issue.code === "EVIDENCE_ALT_REQUIRED"),
|
||||
JSON.stringify(report.issues),
|
||||
);
|
||||
assert.equal(asset.assetId, rendered.id);
|
||||
assert.equal(asset.decorative, true);
|
||||
});
|
||||
|
||||
const PREVIEW_CATALOG = [
|
||||
{ id: "topic-jpa", type: "TOPIC", label: "JPA", publicPath: "/topics/jpa", dependencyRevision: "r1" },
|
||||
{
|
||||
id: "resolution-evidence",
|
||||
type: "EVIDENCE",
|
||||
label: "결론 근거",
|
||||
publicPath: "/cases/some-case",
|
||||
dependencyRevision: "r1",
|
||||
},
|
||||
] as never[];
|
||||
|
||||
function caseDraft(bodyMarkdown: string) {
|
||||
return {
|
||||
kind: "CASE",
|
||||
title: "미리보기 게이트 확인",
|
||||
slug: "preview-gate-check",
|
||||
summary: "요약",
|
||||
topicId: "topic-jpa",
|
||||
projectId: null,
|
||||
relations: [],
|
||||
problem: "문제",
|
||||
conclusion: "결론",
|
||||
environment: "env",
|
||||
reproduction: "repro",
|
||||
lastVerifiedOn: "2026-08-14",
|
||||
bodyMarkdown,
|
||||
} as never;
|
||||
}
|
||||
|
||||
function renderInstantPreview(bodyMarkdown: string, assets: readonly Asset[]) {
|
||||
render(
|
||||
<MemoryRouter initialEntries={[`/studio/documents/${created.id}/edit`]}>
|
||||
<StudioProvider createGateway={() => gateway} createAssetGateway={() => assetGateway}>
|
||||
<DocumentEditorScreen documentId={created.id} />
|
||||
<MemoryRouter>
|
||||
<StudioProvider createGateway={() => createMockStudioGateway()}>
|
||||
<InstantPreview draft={caseDraft(bodyMarkdown)} catalog={PREVIEW_CATALOG} assets={assets} />
|
||||
</StudioProvider>
|
||||
</MemoryRouter>,
|
||||
);
|
||||
}
|
||||
|
||||
await screen.findByLabelText("본문 Markdown");
|
||||
await user.click(screen.getByRole("tab", { name: "즉시 미리보기" }));
|
||||
const panel = screen.getByRole("tabpanel", { name: "즉시 미리보기" });
|
||||
// Round 3 loosened InstantPreview's gate back to the merged catalog, arguing
|
||||
// the un-loaded-asset case would otherwise blank the preview. It blanks
|
||||
// either way -- an un-loaded Asset contributes no catalog row either. All the
|
||||
// looseness bought was keys the fetched EVIDENCE catalog carries: those
|
||||
// rendered a caption with an empty gap and no message while validation said
|
||||
// EVIDENCE_UNSUPPORTED. Every caller is now on the same expression, so the
|
||||
// preview refuses exactly what validation refuses -- and says so.
|
||||
test("Instant Preview refuses a key only the document catalog carries, exactly as mock validation does", () => {
|
||||
const body = ':::evidence key="resolution-evidence" alt="근거" caption="근거" zoom="false"\n:::';
|
||||
renderInstantPreview(body, []);
|
||||
|
||||
assert.equal(within(panel).queryByRole("alert"), null);
|
||||
const alert = screen.getByRole("alert");
|
||||
assert.match(alert.textContent ?? "", /resolution-evidence/);
|
||||
|
||||
const report = validateWorkingCopy(
|
||||
{ ...(caseDraft(body) as object), id: "88888888-8888-4888-8888-888888888882", version: 1, updatedAt: "2026-08-14T00:00:00.000Z" } as never,
|
||||
{
|
||||
now: new Date("2026-08-14T01:00:00.000Z"),
|
||||
validationId: "preview-gate-consistency",
|
||||
dependencyRevision: "r1",
|
||||
catalog: PREVIEW_CATALOG,
|
||||
documents: [],
|
||||
assets: [],
|
||||
},
|
||||
);
|
||||
assert.ok(
|
||||
within(panel).getByRole("heading", { level: 1, name: "카탈로그로만 뒷받침되는 근거 미리보기" }),
|
||||
report.issues.some((issue) => issue.code === "EVIDENCE_UNSUPPORTED"),
|
||||
JSON.stringify(report.issues),
|
||||
);
|
||||
});
|
||||
|
||||
// The pixels come from `createAssetCatalogResolver` (a last-wins `Map`) while
|
||||
// the block descriptor came from `resolveAssetDescriptor` (a first-wins
|
||||
// `find`) -- a fourth expression in the same path, disagreeing with the third
|
||||
// whenever two READY Assets share a key.
|
||||
test("Instant Preview renders the same asset the gate and the descriptor picked when two assets share a key", () => {
|
||||
const older = assetFixture({
|
||||
id: "dddddddd-dddd-4ddd-8ddd-ddddddddddd1",
|
||||
assetKey: "dup-pixels",
|
||||
publicPath: "/media/dup-pixels-older.svg",
|
||||
updatedAt: "2026-08-14T00:00:00.000Z",
|
||||
});
|
||||
const newer = assetFixture({
|
||||
id: "dddddddd-dddd-4ddd-8ddd-ddddddddddd2",
|
||||
assetKey: "dup-pixels",
|
||||
publicPath: "/media/dup-pixels-newer.svg",
|
||||
updatedAt: "2026-08-14T00:00:01.000Z",
|
||||
});
|
||||
|
||||
renderInstantPreview(
|
||||
':::evidence key="dup-pixels" alt="중복 키 그림" caption="근거" zoom="false"\n:::',
|
||||
[newer, older],
|
||||
);
|
||||
|
||||
assert.equal(screen.queryByRole("alert"), null);
|
||||
assert.equal(
|
||||
screen.getByAltText("중복 키 그림").getAttribute("src"),
|
||||
"/media/dup-pixels-newer.svg",
|
||||
);
|
||||
});
|
||||
|
||||
// The idempotency ledger must not freeze an *uncharacterized* internal
|
||||
// failure: the port reports it as retryable, so a same-key retry has to
|
||||
// actually re-run the work instead of replaying the cached 500.
|
||||
test("a same-key retry re-runs after an uncharacterized internal failure instead of replaying a cached 500", async () => {
|
||||
let remainingFailures = 1;
|
||||
const gateway = createMockStudioGateway({
|
||||
dependencyRevision: {
|
||||
current: () => {
|
||||
if (remainingFailures > 0) {
|
||||
remainingFailures -= 1;
|
||||
throw new Error("의존성 리비전을 읽지 못했습니다.");
|
||||
}
|
||||
return "r1";
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const created = await gateway.createDocument(
|
||||
{
|
||||
kind: "CASE",
|
||||
title: "일시적 내부 실패 재시도",
|
||||
slug: "transient-internal-failure-retry",
|
||||
summary: "성격이 규명되지 않은 내부 실패는 원장에 얼려두면 안 됩니다.",
|
||||
topicId: FIXTURE_IDS.topicJpa,
|
||||
projectId: FIXTURE_IDS.projectBackend,
|
||||
relations: [],
|
||||
problem: "문제",
|
||||
conclusion: "결론",
|
||||
environment: "env",
|
||||
reproduction: "repro",
|
||||
lastVerifiedOn: "2026-08-14",
|
||||
bodyMarkdown: "## 본문\n\n내용입니다.",
|
||||
},
|
||||
{ idempotencyKey: "transient-create" },
|
||||
);
|
||||
|
||||
await assert.rejects(
|
||||
gateway.validateDocument(
|
||||
created.id,
|
||||
{ expectedVersion: created.version },
|
||||
{ idempotencyKey: "transient-validate" },
|
||||
),
|
||||
(error: unknown) => {
|
||||
assert.ok(error instanceof StudioGatewayError, `expected a StudioGatewayError, got ${String(error)}`);
|
||||
assert.equal(error.code, "STUDIO_UNAVAILABLE");
|
||||
assert.equal(error.retryable, true);
|
||||
return true;
|
||||
},
|
||||
);
|
||||
|
||||
const report = await gateway.validateDocument(
|
||||
created.id,
|
||||
{ expectedVersion: created.version },
|
||||
{ idempotencyKey: "transient-validate" },
|
||||
);
|
||||
assert.equal(report.documentId, created.id);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user