fix: un-collapse evidence gate 1 from gate 2 to close a raw-Error escape
Fix round 2 (review of 9864958):
Round 1's I2 fix made gate 1 (supportsEvidenceKeyIn) read the same
merged catalog gate 2 already checks, so gate 1 stopped asking "does a
real Asset (or the legacy key) back this key" and started asking the
identical question gate 2 asks ("is there any EVIDENCE catalog row for
it"). A real EVIDENCE catalog row that exists for something other than
media (fixtures.ts's row for a QUESTION resolution target) then passed
gate 1 with nothing backing it as evidence. validateDocument reported
VALID; createPreview's resolver -- which only ever knew the legacy key
and real Assets -- threw a raw, unwrapped Error, violating the port's
StudioGatewayError-only contract. I2's disagreement reproduced in the
opposite direction.
Rebuilt gate 1 in all three callers (validate-working-copy.ts,
adapters/mock/project-public-render-model.ts, instant-preview.tsx) as
"legacy key OR an Asset-derived catalog entry only" -- never the
merged document catalog -- while gate 2 keeps reading the merged
catalog as before. Also stopped emitting the real Asset UUID as the
synthesized CatalogEntry's id (prefixed instead), closing a path where
pasting an Asset's real id -- never something the Picker itself
produces -- would have resolved as an evidence key.
Restored content-format.test.ts's domain tests to exercise the same
gate-1 composition production callers now use instead of a bare
hand-injected predicate, and added coverage for: a real non-asset
EVIDENCE row still being rejected, an Asset-backed key being accepted
with no document-catalog row at all, the fixture-UUID case failing
both mock paths with no raw Error crossing the gateway port, and the
EVIDENCE_ALT_REQUIRED decorative-Asset pairing that had no test.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
98649585e6
commit
7ff9728a5c
@@ -22,18 +22,36 @@ type ProjectArguments = Parameters<typeof projectWorkingCopyWithEvidence>;
|
||||
// and Instant Preview use, so this mock's own `createPreview` agrees with
|
||||
// `validateDocument` about which evidence keys exist), so it resolves the
|
||||
// descriptor here to produce a genuine, fully-resolved `PublicRenderModel`.
|
||||
//
|
||||
// Fix round 2 (I2 regression). Gate 1 (`supportsEvidenceKey` below) must ask
|
||||
// "does a real Asset (or the legacy key) back this", not "is there any
|
||||
// EVIDENCE catalog row for it" -- `catalog` carries EVIDENCE rows unrelated
|
||||
// to media (e.g. a QUESTION resolution-target row). Handing gate 1 the
|
||||
// merged catalog let a key backed by nothing pass the projection and reach
|
||||
// the resolver below, whose own `isSupportedEvidenceKey`-only fallback then
|
||||
// threw a raw, unwrapped `Error` -- the disagreement in the opposite
|
||||
// direction from I2 (preview crashing instead of `validateDocument`
|
||||
// rejecting), and a contract violation on top (`StudioGatewayError` is what
|
||||
// may leave this port, never a bare `Error`). With gate 1 narrowed to
|
||||
// asset-backing/legacy only, any key the resolver would fail to resolve was
|
||||
// already rejected before `projectWorkingCopyWithEvidence` returns, so the
|
||||
// resolver's fallback throw below is unreachable for every key that made it
|
||||
// this far.
|
||||
export function projectWorkingCopy(
|
||||
input: ProjectArguments[0],
|
||||
catalog: ProjectArguments[1],
|
||||
context: ProjectArguments[2],
|
||||
assets: readonly Asset[] = [],
|
||||
) {
|
||||
const evidenceCatalog = [...catalog, ...evidenceCatalogEntriesFromAssets(assets)];
|
||||
const assetEntries = evidenceCatalogEntriesFromAssets(assets);
|
||||
const evidenceCatalog = [...catalog, ...assetEntries];
|
||||
const supportsEvidenceKey = (key: string) =>
|
||||
isSupportedEvidenceKey(key) || supportsEvidenceKeyIn(assetEntries)(key);
|
||||
const model = projectWorkingCopyWithEvidence(
|
||||
input,
|
||||
evidenceCatalog,
|
||||
context,
|
||||
supportsEvidenceKeyIn(evidenceCatalog),
|
||||
supportsEvidenceKey,
|
||||
);
|
||||
|
||||
return resolveCaseEvidenceAssets(model, (key) => {
|
||||
@@ -52,6 +70,10 @@ export function projectWorkingCopy(
|
||||
};
|
||||
}
|
||||
if (!isSupportedEvidenceKey(key)) {
|
||||
// Defensive only -- gate 1 above already rejects any key that would
|
||||
// reach here without a resolvable Asset or the legacy key. Kept as a
|
||||
// guard rather than removed so a future gate regression fails loudly
|
||||
// here too, not silently.
|
||||
throw new Error(`Unknown local evidence asset: ${key}`);
|
||||
}
|
||||
return resolveEvidenceAssetDescriptor(key);
|
||||
|
||||
@@ -6,6 +6,7 @@ import {
|
||||
} from "../../domain/content-format/asset-evidence-catalog.ts";
|
||||
import { parseCaseContent } from "../../domain/content-format/parse-case-content.ts";
|
||||
import { evidenceCatalogEntryFor } from "../../domain/content-format/project-public-render-model.ts";
|
||||
import { isSupportedEvidenceKey } from "../static/evidence-assets.ts";
|
||||
|
||||
type CatalogEntry = components["schemas"]["CatalogEntry"];
|
||||
type WorkingCopyInput = components["schemas"]["WorkingCopyInput"];
|
||||
@@ -23,6 +24,11 @@ export type ValidationDependencies = {
|
||||
* failed validation. Merged into `catalog` via
|
||||
* `evidenceCatalogEntriesFromAssets`, the same function Instant Preview
|
||||
* uses, so both paths agree on which keys a document may reference.
|
||||
*
|
||||
* Fix round 2: only used for gate 2 (the merged-catalog lookup) and the
|
||||
* `EVIDENCE_ALT_REQUIRED` decorative check. Gate 1 (`supportsEvidenceKey`
|
||||
* below) intentionally does NOT read the merged catalog -- see that
|
||||
* variable's comment.
|
||||
*/
|
||||
assets: ReadonlyArray<Asset>;
|
||||
};
|
||||
@@ -176,8 +182,19 @@ export function validateWorkingCopy(document: WorkingCopy, dependencies: Validat
|
||||
if (blank(document.problem)) error("CASE_PROBLEM_REQUIRED", "/problem", "문제를 입력하세요."); if (blank(document.conclusion)) error("CASE_CONCLUSION_REQUIRED", "/conclusion", "결론을 입력하세요.");
|
||||
if (blank(document.bodyMarkdown)) error("CASE_BODY_REQUIRED", "/bodyMarkdown", "본문을 입력하세요.");
|
||||
else try {
|
||||
const evidenceCatalog = [...dependencies.catalog, ...evidenceCatalogEntriesFromAssets(dependencies.assets)];
|
||||
const supportsEvidenceKey = supportsEvidenceKeyIn(evidenceCatalog);
|
||||
const assetEntries = evidenceCatalogEntriesFromAssets(dependencies.assets);
|
||||
const evidenceCatalog = [...dependencies.catalog, ...assetEntries];
|
||||
// Fix round 2 (I2 regression). Gate 1 must ask "does a real Asset (or
|
||||
// the legacy key) back this" -- NOT "is there any EVIDENCE catalog row
|
||||
// for it". `dependencies.catalog` carries EVIDENCE rows unrelated to
|
||||
// media (e.g. a QUESTION resolution-target row), so handing gate 1 the
|
||||
// merged catalog (as fix round 1 did) let a key backed by nothing
|
||||
// resolve here while `createPreview`'s resolver -- which only ever
|
||||
// knew about the legacy key and real Assets -- threw a raw, unwrapped
|
||||
// Error. Gate 2 (`evidenceCatalogEntryFor` below) still reads the
|
||||
// merged catalog; only gate 1 narrows.
|
||||
const supportsEvidenceKey = (key: string) =>
|
||||
isSupportedEvidenceKey(key) || supportsEvidenceKeyIn(assetEntries)(key);
|
||||
const readyAssetsByKey = new Map(dependencies.assets.filter((asset) => asset.managementStatus === "READY").map((asset) => [asset.assetKey, asset]));
|
||||
for (const block of parseCaseContent(document.bodyMarkdown)) if (block.type === "EVIDENCE_FIGURE") {
|
||||
if (!supportsEvidenceKey(block.key)) error("EVIDENCE_UNSUPPORTED", "/bodyMarkdown", `지원하지 않는 Evidence: ${block.key}`);
|
||||
|
||||
@@ -17,14 +17,22 @@ type CatalogEntry = components["schemas"]["CatalogEntry"];
|
||||
* preview perfectly and then fail validation. Both paths now call this one
|
||||
* function instead.
|
||||
*
|
||||
* Every `EVIDENCE` `CatalogEntry` the document catalog issues today already
|
||||
* carries the referenced key in `label` (see the one pre-existing fixture:
|
||||
* `{ type: "EVIDENCE", label: "fetch-strategy-boundary" }`), so a `READY`
|
||||
* Asset is mapped the same way: `label` <- `assetKey`. A key backed by no
|
||||
* loaded `READY` Asset and no catalog row synthesizes no entry here, so
|
||||
* `evidenceCatalogEntryFor` (the domain's one matching rule, shared by both
|
||||
* gates) still rejects it -- this cannot forge a pass for a dangling
|
||||
* reference.
|
||||
* Fix round 2 (I2 regression): this module supplies the *building blocks* for
|
||||
* gate 1 (`supportsEvidenceKey` — "may this document reference this key at
|
||||
* all") and gate 2 (`evidenceCatalogEntryFor` — "does some catalog actually
|
||||
* carry it"); it deliberately does not hand callers a single combined
|
||||
* predicate. Round 1 called `supportsEvidenceKeyIn(mergedCatalog)` for BOTH
|
||||
* gates, which made gate 1 ask the identical question gate 2 already asks —
|
||||
* so any `EVIDENCE` catalog row backing something other than an Asset (e.g.
|
||||
* the pre-existing "Fetch Join Case" row that exists for QUESTION resolution
|
||||
* targets, not media) passed gate 1 even though no Asset and no legacy key
|
||||
* back it. Callers must instead compose gate 1 from *asset-backing only*:
|
||||
* `(key) => isSupportedEvidenceKey(key) || supportsEvidenceKeyIn(evidenceCatalogEntriesFromAssets(assets))(key)`,
|
||||
* then hand gate 2 the merged catalog (`[...documentCatalog,
|
||||
* ...evidenceCatalogEntriesFromAssets(assets)]`) separately. See
|
||||
* `validate-working-copy.ts`, `adapters/mock/project-public-render-model.ts`,
|
||||
* and `instant-preview.tsx` for the three compositions (each has its own
|
||||
* notion of "legacy key" per its layer).
|
||||
*/
|
||||
export function evidenceCatalogEntriesFromAssets(
|
||||
assets: readonly Asset[],
|
||||
@@ -32,7 +40,14 @@ export function evidenceCatalogEntriesFromAssets(
|
||||
return assets
|
||||
.filter((asset) => asset.managementStatus === "READY")
|
||||
.map((asset) => ({
|
||||
id: asset.id,
|
||||
// Not `asset.id` (fix round 2): `evidenceCatalogEntryFor` also matches
|
||||
// on `id`, so reusing the real Asset UUID here would let anyone who
|
||||
// has seen that UUID elsewhere (an API response, an admin view) type
|
||||
// it directly as a directive key and have it resolve — a path the
|
||||
// Picker never produces (it always emits `assetKey`) and that widens
|
||||
// the reference surface for no benefit. Prefixed so it can never
|
||||
// collide with a real `asset.id` a caller might paste in.
|
||||
id: `asset-evidence:${asset.id}`,
|
||||
type: "EVIDENCE",
|
||||
label: asset.assetKey,
|
||||
publicPath: asset.publicPath ?? undefined,
|
||||
|
||||
@@ -9,6 +9,7 @@ import {
|
||||
projectWorkingCopy,
|
||||
resolveCaseEvidenceAssets,
|
||||
} from "../../../domain/content-format/project-public-render-model.ts";
|
||||
import type { SupportsEvidenceKey } from "../../../domain/public-render-content.ts";
|
||||
import { createAssetCatalogResolver } from "../../shared/public-render/asset-resolvers.ts";
|
||||
import { PublicRecordRenderer } from "../../shared/public-render/public-record-renderer.tsx";
|
||||
import { useStudio } from "../use-studio.ts";
|
||||
@@ -19,16 +20,29 @@ type PublicRenderModel = components["schemas"]["PublicRenderModel"];
|
||||
|
||||
/**
|
||||
* The one legacy key predates the Asset gateway entirely: the document
|
||||
* catalog carries a fixture `EVIDENCE` row for it (so the gates above pass
|
||||
* it), but no `Asset` record backs it, so `assets` never resolves it. Kept
|
||||
* narrow and separate from `supportsEvidenceKeyIn` -- that function decides
|
||||
* whether a document is *allowed* to reference a key; this one only fills in
|
||||
* the one pre-existing key's pixels when nothing else can.
|
||||
* catalog carries a fixture `EVIDENCE` row for it, but no `Asset` record
|
||||
* backs it, so `assets` never resolves it. This is also gate 1's "legacy"
|
||||
* half now (fix round 2) -- see `supportsEvidenceKeyForAssets` below.
|
||||
*/
|
||||
function isLegacyStaticEvidenceKey(key: string): boolean {
|
||||
return key === "fetch-strategy-boundary";
|
||||
}
|
||||
|
||||
/**
|
||||
* Gate 1: "may this document reference this key at all" -- the legacy key,
|
||||
* or a `READY` Asset actually backs it. Deliberately narrower than gate 2
|
||||
* (the merged-catalog lookup `projectWorkingCopy` runs internally): fix
|
||||
* round 1 collapsed this into `supportsEvidenceKeyIn(effectiveCatalog)`,
|
||||
* which made gate 1 ask the same question as gate 2 and let any `EVIDENCE`
|
||||
* catalog row backing something other than an Asset (e.g. a QUESTION
|
||||
* resolution-target row) pass gate 1 with no Asset and no legacy key behind
|
||||
* it. See `asset-evidence-catalog.ts`'s module doc for the shared shape.
|
||||
*/
|
||||
function supportsEvidenceKeyForAssets(assets: readonly Asset[]): SupportsEvidenceKey {
|
||||
const assetOnly = supportsEvidenceKeyIn(evidenceCatalogEntriesFromAssets(assets));
|
||||
return (key: string) => isLegacyStaticEvidenceKey(key) || assetOnly(key);
|
||||
}
|
||||
|
||||
/**
|
||||
* `resolveCaseEvidenceAssets` only needs *a* `ResolvedAsset` to turn the authoring
|
||||
* model into a genuine `PublicRenderModel` -- nothing downstream reads `block.asset`
|
||||
@@ -97,7 +111,7 @@ export function InstantPreview({
|
||||
draft,
|
||||
effectiveCatalog,
|
||||
{ mode: "PREVIEW", publishedAt: null },
|
||||
supportsEvidenceKeyIn(effectiveCatalog),
|
||||
supportsEvidenceKeyForAssets(assets),
|
||||
),
|
||||
resolveAssetDescriptor(assets),
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user