From 39ea62d5f1149e108352c7a8f510de83df87c693 Mon Sep 17 00:00:00 2001 From: denispetre Date: Tue, 4 Aug 2026 16:17:44 +0300 Subject: [PATCH 1/2] test(model-onboarding): fix false passes in the file-reading assertion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to #1009, addressing review findings 2, 3 and 7. Each was confirmed by executing the merged code, not by reading it. The core problem: the test could pass without the model reading the file. _matches("dog", "There is no dog in this image; it is a cat.") -> True _matches("dog", "I don't have access... probably a dog.") -> True Root cause was the fixtures, not the matcher. "What animal is in this image?" over dog.jpg has "dog" as its most likely answer with no image at all, and the file name was visible in the prompt. dummy.pdf's text ("Dummy PDF file") reads as a placeholder, so the model editorialized about whether it was real instead of reporting it — flaky 3-of-6 runs. Compensating for that needed a refusal-phrase blocklist, which then rejected correct answers carrying commentary (finding 2). Replaced both with generated local fixtures whose answers cannot be guessed: a PDF containing "Verification code: PDF-CODE-74915", and a purple square asked for its colour. With no prior to fall back on, the blocklist becomes unnecessary — refusals now fail simply because they do not contain the code. _matches is a plain case-insensitive whole-token match; 17-case suite covers correct formattings, refusals and near misses. Also fixes finding 7: create_messages interpolated state.model_dump(), rendering the attachment as {'id': UUID('8da6...'), 'full_name': ...} while the Analyze Files tool documents {"ID": "8da6..."}. A model copying UUID('...') gets INVALID_ATTACHMENT_ID; one emitting lowercase `id` has the item skipped and analyzes nothing — and then passed the permissive matcher. Finding 3 was masking finding 7, so fixing either alone would have misled. Now model_dump(by_alias=True, mode="json"). Fixtures are local so the expected answer is a property of bytes in this repo rather than a third-party host that could change a file and quietly weaken the test. Pinned -text in .gitattributes (EOL translation would corrupt them); document.pdf is uncompressed so the code stays greppable. Verified against alpha: image and pdf pass 3/3 (the PDF was 3/6). Co-Authored-By: Claude Opus 5 --- .gitattributes | 5 + testcases/model-onboarding/fixtures/README.md | 48 +++++++ .../model-onboarding/fixtures/document.pdf | Bin 0 -> 608 bytes testcases/model-onboarding/fixtures/shape.png | Bin 0 -> 1903 bytes .../src/agents/file_processing/agent.py | 43 +++++-- testcases/model-onboarding/src/main.py | 117 ++++++++++-------- 6 files changed, 154 insertions(+), 59 deletions(-) create mode 100644 testcases/model-onboarding/fixtures/README.md create mode 100644 testcases/model-onboarding/fixtures/document.pdf create mode 100644 testcases/model-onboarding/fixtures/shape.png diff --git a/.gitattributes b/.gitattributes index c8f9eb864..e8e8b822b 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1,2 +1,7 @@ *.db filter=lfs diff=lfs merge=lfs -text **/cached_embeddings/** filter=lfs diff=lfs merge=lfs -text + +# Test fixtures are binary: the assertion compares bytes the model read back, +# so any EOL translation on checkout would corrupt them. +*.pdf -text +*.png -text diff --git a/testcases/model-onboarding/fixtures/README.md b/testcases/model-onboarding/fixtures/README.md new file mode 100644 index 000000000..f7a9a573c --- /dev/null +++ b/testcases/model-onboarding/fixtures/README.md @@ -0,0 +1,48 @@ +# Test fixtures + +Both files are generated, committed, and read from disk by +`src/agents/file_processing/agent.py`. They are deliberately tiny (608 B and +1.9 KB). + +| File | Content | Question asked | Expected | +|---|---|---|---| +| `document.pdf` | One page: `Verification code: PDF-CODE-74915` | "What is the verification code written in this document?" | `PDF-CODE-74915` | +| `shape.png` | 512×512, purple square (`#800080`) on white | "What colour is the large shape in the centre?" | `purple` | + +## Why these, and not files from the web + +The answers are **unguessable**, which is the only reason the assertion proves +anything. A model that never opened the file cannot produce a random code, and +has no prior for an arbitrary colour. + +The previous fixtures were borrowed URLs and both were guessable: + +- `dog.jpg` was asked "what animal is in this image?" — "dog" is the single + most likely answer to that question with no image at all, and the file name + appears in the prompt. A model that skipped the file scored correct. +- `dummy.pdf` contained the literal text "Dummy PDF file", which reads as a + placeholder. The model kept commenting on whether the content was real + instead of reporting it, failing in both directions across runs. + +Compensating for guessable answers previously required a refusal-phrase +blocklist and a position-in-the-answer heuristic in `_matches`. Both were +brittle, and neither was needed once the fixtures changed: a plain token match +now suffices. + +Local rather than remote also means the expected answer is a property of bytes +in this repo — a third-party host cannot silently change the file and break or, +worse, quietly weaken the test. + +## Regenerating + +`document.pdf` is written uncompressed on purpose, so the code stays greppable +in the raw bytes and can be checked without a PDF library: + +```bash +grep -c 'PDF-CODE-74915' fixtures/document.pdf # 1 +``` + +If you change a fixture, update `expected` in `FILE_REGISTRY` +(`src/main.py`) to match. The generator scripts are not committed — these are +static test inputs, and regenerating them is a deliberate act, not part of the +build. diff --git a/testcases/model-onboarding/fixtures/document.pdf b/testcases/model-onboarding/fixtures/document.pdf new file mode 100644 index 0000000000000000000000000000000000000000..e18c52a2fb4fe51145299479d28bf4cfa772477c GIT binary patch literal 608 zcmZWnO;5r=5WVlOmaJ!`oVh9JmVxmR@jTho!p;IiuuGuaI{q@eYMXGFi=)QR~ z@8#{Z=95`l-9$n`0Cv3-g8|6+>jOcKjW>I{1vxicEFdJp0)|6@dBQ}#|81yv@*ztK zSaq2GN>l71_n;b$T)Avy8HYrzbmS8znHkw5tODY9RS3O)Nb?q0*n``UA$B^m)KUH@ z(5y&0{E=+#X<7kMjh<}t{iObg`PRQdSF^jqJ2dZN6w_1%VFYmsJyHzjns|~Q$az9;P;Sm-O-T38S6t?h5U@^+Y3x7^2DdX3nSWqe^^I1c;1<8kFH#h dW%iBGrN^+A-?mtVkI=UHx3vIVCg!0O9&fZU6uP literal 0 HcmV?d00001 diff --git a/testcases/model-onboarding/src/agents/file_processing/agent.py b/testcases/model-onboarding/src/agents/file_processing/agent.py index f555f0535..cb04db6d9 100644 --- a/testcases/model-onboarding/src/agents/file_processing/agent.py +++ b/testcases/model-onboarding/src/agents/file_processing/agent.py @@ -74,10 +74,19 @@ def create_messages(state: AgentInput) -> Sequence[SystemMessage | HumanMessage] fileIn = getattr(state, 'fileIn', '') prompt = getattr(state, 'prompt', '') + # Serialize with by_alias + mode="json" so the attachment interpolates as + # the shape the Analyze Files tool documents — {"ID": "8da6…"} — rather than + # a Python repr. A plain model_dump() renders it as + # {'id': UUID('8da6…'), 'full_name': …}: a model copying UUID('…') gets + # INVALID_ATTACHMENT_ID, and one emitting lowercase `id` has the item + # skipped and analyzes nothing. mode="json" is required — without it the + # UUID stays a UUID object and serialization raises downstream. + state_values = state.model_dump(by_alias=True, mode="json") + # Apply system prompt template current_date = datetime.now(timezone.utc).strftime('%Y-%m-%d') system_prompt_content = """You are a file-processing assistant. You are given a single file (PDF or image) and a task. Use the Analyze Files tool to read the file's contents, then answer the task concisely based only on what the file contains. If the file cannot be read, say so plainly.""" - system_prompt_content = interpolate_legacy_message(system_prompt_content, state.model_dump()) + system_prompt_content = interpolate_legacy_message(system_prompt_content, state_values) enhanced_system_prompt = ( AGENT_SYSTEM_PROMPT_TEMPLATE .replace('{{systemPrompt}}', system_prompt_content) @@ -89,7 +98,7 @@ def create_messages(state: AgentInput) -> Sequence[SystemMessage | HumanMessage] SystemMessage(content=enhanced_system_prompt), HumanMessage(content=interpolate_legacy_message("""{{prompt}} -{{fileIn}}""", state.model_dump())), +{{fileIn}}""", state_values)), ] @@ -122,15 +131,31 @@ def build_graph(llm): async def _upload_attachment(file_info) -> Attachment: - """Fetch the test file and register it as a platform attachment.""" - import httpx - from uipath._utils._ssl_context import get_httpx_client_kwargs + """Read the test file and register it as a platform attachment. + + Accepts a local path or an ``http(s)`` URL. The committed fixtures are + local so the assertion cannot drift when a third-party host changes a + file, and so the expected answer is a property of bytes in this repo. + """ + from pathlib import Path + from uipath.platform import UiPath - async with httpx.AsyncClient(**get_httpx_client_kwargs()) as client: - response = await client.get(file_info.url) - response.raise_for_status() - content = response.content + if file_info.url.startswith(("http://", "https://")): + import httpx + from uipath._utils._ssl_context import get_httpx_client_kwargs + + async with httpx.AsyncClient(**get_httpx_client_kwargs()) as client: + response = await client.get(file_info.url) + response.raise_for_status() + content = response.content + else: + # Resolve relative to the project root (this file is src/agents/ + # file_processing/agent.py), so the run works from any cwd. + path = Path(file_info.url) + if not path.is_absolute(): + path = Path(__file__).resolve().parents[3] / path + content = path.read_bytes() sdk = UiPath() attachment_id = await sdk.attachments.upload_async( diff --git a/testcases/model-onboarding/src/main.py b/testcases/model-onboarding/src/main.py index 86cd4da60..13fd27572 100644 --- a/testcases/model-onboarding/src/main.py +++ b/testcases/model-onboarding/src/main.py @@ -12,11 +12,15 @@ } } -Every file in ``FILE_REGISTRY`` is exercised. Each asks a question with one -deterministic answer that only its contents reveal — "what animal is this?" -over a photo of a dog, "what is the first word inside?" over a PDF reading -"Dummy PDF file". The answer word appears nowhere in the file name, so a model -that never opened the file cannot produce it. +Every file in ``FILE_REGISTRY`` is exercised. Each asks a question whose answer +is **unguessable** — a random code inside a PDF, the colour of a shape in an +image. That property is what makes the assertion meaningful: there is no prior +a model can fall back on, so producing the answer proves it read the file. + +This matters more than it sounds. Earlier fixtures asked "what animal is in +this image?" over ``dog.jpg``; "dog" is the most likely answer to that question +with no image at all, and the file name was visible in the prompt, so a model +that never opened the file still scored correct. Each ``api_flavors`` entry is a ``vendor_type:api_flavor`` pair forwarded to ``get_chat_model`` (e.g. ``openai:responses``, ``awsbedrock:converse``, @@ -24,6 +28,7 @@ """ import logging +import re from langgraph.graph import END, START, StateGraph from pydantic import BaseModel, Field @@ -51,70 +56,82 @@ class FileCase(BaseModel): model_config = {"arbitrary_types_allowed": True} -# Files the agent processes, selected by name via `model_spec.files`. +# Files the agent processes. Both fixtures are generated and committed under +# fixtures/ (see fixtures/README.md), and both answers are unguessable — which +# is the whole point. +# +# The previous fixtures were borrowed from the web and both were guessable: +# +# - dog.jpg asked "what animal is this?", and "dog" is the single most likely +# answer to that question with no image at all. A model that never opened the +# file scored correct. The file name was visible in the prompt too. +# - dummy.pdf's text is the literal string "Dummy PDF file", which reads as a +# placeholder, so the model kept editorializing about whether the content was +# real instead of reporting it — flaky in both directions. +# +# A random code and an arbitrary color have no prior to fall back on: the model +# either read the file or it did not. FILE_REGISTRY: dict[str, FileCase] = { "image": FileCase( - # A white Samoyed sitting on grass. + # A purple square on white. The subject carries no colour prior. file=FileInfo( - url="https://raw.githubusercontent.com/pytorch/hub/master/images/dog.jpg", - name="animal.jpg", - mime_type="image/jpeg", + url="fixtures/shape.png", + name="shape.png", + mime_type="image/png", + ), + question=( + "What colour is the large shape in the centre of this image? " + "Answer with one word only." ), - question="What animal is in this image? Answer with one word only.", - expected="dog", + expected="purple", ), "pdf": FileCase( - # A one-page PDF whose entire content is the line "Dummy PDF file". + # A one-page PDF whose only text is "Verification code: PDF-CODE-74915". file=FileInfo( - url="https://www.w3.org/WAI/ER/tests/xhtml/testfiles/resources/pdf/dummy.pdf", + url="fixtures/document.pdf", name="document.pdf", mime_type="application/pdf", ), - # This file's text is the literal string "Dummy PDF file", which reads - # as a placeholder — across six runs the model answered it correctly - # three times and three times refused, reporting the file unreadable - # while quoting the very text the tool had returned. Asking it to - # repeat the tool output verbatim removes the judgement call; the - # instruction not to evaluate the text is what makes this stable. question=( - "Call the Analyze Files tool, then repeat its result back " - "verbatim as your entire answer. Do not evaluate, judge or " - "comment on whether the text is meaningful." + "What is the verification code written in this document? " + "Answer with the code only." ), - expected="dummy", + expected="PDF-CODE-74915", ), } -# Phrases a model uses when it quotes the expected word while denying it read -# the file. A plain substring check passed those, asserting the opposite of -# what the answer said. -_REFUSAL_MARKERS = ( - "cannot", - "can't", - "unable", - "not available", - "no actual", - "placeholder", - "unreadable", - "appears to be", - "rather than", -) - - def _matches(expected: str, answer: str) -> bool: - """Was the expected word actually given as the answer? - - Requires the word as a real token and no refusal language. Length is not a - criterion: a verbatim transcription legitimately carries the parser's - ```` prefix, which a word-count limit rejected - even though the answer was correct. + """Does the answer contain the expected value as a whole token? + + Deliberately simple, and only safe because the expected values are + unguessable (a random code, an arbitrary colour). Earlier fixtures were + guessable, which forced a refusal-phrase blocklist and a + position-in-answer heuristic to tell "read the file" from "guessed the + obvious"; both were brittle — they rejected correct answers that carried + commentary, and still passed "there is no dog; it is a cat". + + Choosing a fixture whose answer cannot be guessed removes the need to + interpret the prose around it: presence of the token *is* the evidence. + Matching is case-insensitive and on whole tokens, so a substring like + "Samoyed" cannot satisfy "dog", and markdown or a parser prefix around the + answer is harmless. """ - lowered = answer.lower() - if any(marker in lowered for marker in _REFUSAL_MARKERS): + # Hyphens are token separators here, so "PDF-CODE-74915" is compared as its + # parts in order — robust to the model reformatting the separator. + def tokens(text: str) -> list[str]: + return re.findall(r"\w+", text.lower()) + + expected_tokens = tokens(expected) + answer_tokens = tokens(answer) + if not expected_tokens: return False - words = [w.strip(".,'\"“”‘’!?:;()<>/").lower() for w in lowered.split()] - return expected.lower() in words + # Look for the expected token sequence anywhere in the answer. + span = len(expected_tokens) + return any( + answer_tokens[i : i + span] == expected_tokens + for i in range(len(answer_tokens) - span + 1) + ) class ModelSpec(BaseModel): From 02df133000ef24e253f00cb2c420efc2d11bc55b Mon Sep 17 00:00:00 2001 From: denispetre Date: Tue, 4 Aug 2026 16:28:20 +0300 Subject: [PATCH 2/2] test(model-onboarding): report a bad flavor instead of aborting the run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding 1. Confirmed: settings and each get_chat_model call sat outside any try, so one unusable flavor killed the node — `uipath run` exits non-zero and `set -e` in run.sh stops the script before validate_output.sh. No summary, no trace assertion, and every flavor that already passed was discarded. That is the most likely condition during real onboarding, and it is exactly what this test should be reporting. Reproduced with api_flavors ["bogusvendor:whatever", "openai:responses"] (bad one first, so an abort would hide the good one). Before: the run died. After: bogusvendor:whatever: build: ✗ AgentStartupError: The model '...' is not available... openai:responses: build: ✓ UiPathAzureChatOpenAI image: ✓ purple pdf: ✓ PDF-CODE-74915 success=False, exit 0, summary intact. Also from the review: - Logs the class actually built (finding 6). Discovery can override the requested api_flavor, and an unrecognized Bedrock flavor falls through to Converse, so echoing the requested string alone would sign off on a surface never exercised. - Empty api_flavors now fails with "no api_flavors supplied" instead of reporting a vacuous success on an empty summary. - Summary lines are logged as they are produced, so a run that dies for an unforeseen reason still leaves partial results in the job log. Happy path unchanged; full assert.py incl. trace assertions passes. Co-Authored-By: Claude Opus 5 --- testcases/model-onboarding/src/main.py | 81 ++++++++++++++++++++------ 1 file changed, 64 insertions(+), 17 deletions(-) diff --git a/testcases/model-onboarding/src/main.py b/testcases/model-onboarding/src/main.py index 13fd27572..b3fad0a28 100644 --- a/testcases/model-onboarding/src/main.py +++ b/testcases/model-onboarding/src/main.py @@ -153,24 +153,70 @@ class GraphOutput(BaseModel): result_summary: str +def _one_line(error: BaseException) -> str: + """Collapse an exception to one line for the per-cell summary.""" + return " ".join(str(error).split()) + + async def probe_file_processing(state: GraphInput) -> GraphOutput: - """Run the agent for every api_flavor x file, collecting its answers.""" + """Run the agent for every api_flavor x file, collecting its answers. + + Nothing here is allowed to abort the run. Every cell — settings, each + model build, each file — is recorded as ✓ or ✗ and the loop continues, + because a model that fails one flavor is exactly the condition this test + is meant to *report*. An unguarded raise kills the node, `uipath run` + exits non-zero, and `set -e` in run.sh stops the script before + validate_output.sh, discarding the summary and every flavor that already + passed. + """ spec = state.model_spec - settings = PlatformSettings(agenthub_config=spec.agenthub_config) lines: list[str] = [] failed = False + def record(line: str) -> None: + """Append a summary line and log it immediately. + + Logged as it happens so a run that dies for an unforeseen reason still + leaves the partial results in the job log. + """ + lines.append(line) + logger.info(line) + + try: + settings = PlatformSettings(agenthub_config=spec.agenthub_config) + except Exception as e: + # A bad agenthub_config fails every flavor, so report it once and stop + # rather than repeat the same error per flavor. + detail = f"settings: ✗ {type(e).__name__}: {_one_line(e)}"[:300] + record(detail) + return GraphOutput(success=False, result_summary="\n".join(lines)) + for flavor in spec.api_flavors: + record(f"{flavor}:") vendor_type, _, api_flavor = flavor.partition(":") - model = get_chat_model( - model=spec.model_name, - client_settings=settings, - vendor_type=vendor_type or None, - api_flavor=api_flavor or None, - temperature=0.0, - max_tokens=2000, - ) - lines.append(f"{flavor}:") + + # get_chat_model raises on an unknown vendor (ValueError) or a model + # the tenant cannot serve (AgentStartupError, via ModelNotFoundError). + # Both are ordinary findings for this test, not crashes. + try: + model = get_chat_model( + model=spec.model_name, + client_settings=settings, + vendor_type=vendor_type or None, + api_flavor=api_flavor or None, + temperature=0.0, + max_tokens=2000, + ) + except Exception as e: + failed = True + record(f" build: ✗ {type(e).__name__}: {_one_line(e)}"[:300]) + continue + + # Log the class actually constructed: discovery can override the + # requested api_flavor, and an unrecognized Bedrock flavor falls + # through to Converse — so echoing the requested string alone would + # sign off on a surface that was never exercised. + record(f" build: ✓ {type(model).__name__}") for file_name, case in FILE_REGISTRY.items(): try: @@ -179,20 +225,21 @@ async def probe_file_processing(state: GraphInput) -> GraphOutput: ) except Exception as e: failed = True - # Collapse newlines: some SDK errors are multi-line and would - # break the one-cell-per-line summary. - detail = " ".join(str(e).split()) - lines.append(f" {file_name}: ✗ {type(e).__name__}: {detail}"[:300]) + record(f" {file_name}: ✗ {type(e).__name__}: {_one_line(e)}"[:300]) continue if _matches(case.expected, answer): - lines.append(f" {file_name}: ✓ {answer}"[:200]) + record(f" {file_name}: ✓ {answer}"[:200]) else: failed = True - lines.append( + record( f" {file_name}: ✗ expected '{case.expected}', got: {answer}"[:300] ) + if not spec.api_flavors: + failed = True + record("✗ no api_flavors supplied") + summary = "\n".join(lines) logger.info(f"Success: {not failed}\nSummary:\n{summary}") return GraphOutput(success=not failed, result_summary=summary)