evals: move the cases into the skill folder as evals/evals.json

The case file used its own schema at the repository root, so nothing
could run it but a person. agent-skills-eval and skill-creator both read
<skill>/evals/evals.json with skill_name, evals[], and expected_output,
so the same file now serves the validator here and a judge-model harness
outside CI. The why, source, and rule fields stay; harnesses ignore them.

The validator looks for evals/evals.json under each skill directory and
checks that skill_name matches the directory. The CI path filter follows
the file, and both READMEs describe the new layout and how to run the
cases with a harness.
This commit is contained in:
Gaic4o
2026-09-15 15:22:21 +09:00
parent 6720d8d2ca
commit 03df8117a6
6 changed files with 107 additions and 51 deletions
+25 -7
View File
@@ -657,8 +657,11 @@ function validateCaseRule(evalsFile, label, testCase, documentsBySkill) {
}
}
function validateEvals(documentsBySkill) {
const evalsFile = path.join(ROOT, "evals", "cases.json");
// Each skill keeps its cases in <skill>/evals/evals.json, the layout that
// skill-creator and agent-skills-eval read, so the same file serves the
// validator here and a judge-model harness outside CI.
function validateEvals(skillDirectory, documentsBySkill) {
const evalsFile = path.join(skillDirectory, "evals", "evals.json");
if (!isFile(evalsFile)) {
return;
@@ -673,16 +676,28 @@ function validateEvals(documentsBySkill) {
return;
}
const cases = parsed?.cases;
const skillName = path.basename(skillDirectory);
// Harnesses key their reports on skill_name, so a mismatch files the
// results under a skill that does not exist.
if (parsed?.skill_name !== skillName) {
recordError(
evalsFile,
`skill_name "${parsed?.skill_name}" does not match the ` +
`skill directory "${skillName}"`,
);
}
const cases = parsed?.evals;
if (!Array.isArray(cases) || cases.length === 0) {
recordError(evalsFile, "must contain a non-empty cases array");
recordError(evalsFile, "must contain a non-empty evals array");
return;
}
log(` evals/cases.json: ${cases.length} case(s)`);
log(` ${skillName}/evals/evals.json: ${cases.length} case(s)`);
const required = ["id", "prompt", "expect", "why", "source", "rule"];
const required = ["id", "prompt", "expected_output", "why", "source", "rule"];
const seen = new Set();
for (const [index, testCase] of cases.entries()) {
@@ -750,7 +765,10 @@ export function validateRepository(root = DEFAULT_ROOT, options = {}) {
validateSkill(skillDirectory, documentsBySkill);
}
validateEvals(documentsBySkill);
for (const skillDirectory of skillDirectories) {
validateEvals(skillDirectory, documentsBySkill);
}
return errors;
}
+16 -7
View File
@@ -14,16 +14,18 @@ const ROOT = path.resolve(import.meta.dirname, "../..");
const SKILL = "feature-sliced-design/SKILL.md";
const ASSETS = "feature-sliced-design/references/asset-handling.md";
const AUTH = "feature-sliced-design/references/auth-and-api.md";
const CASES = "evals/cases.json";
const CASES = "feature-sliced-design/evals/evals.json";
const LINE_LIMIT = 500;
function problemsAfter(mutate) {
const dir = mkdtempSync(path.join(tmpdir(), "validate-skills-"));
try {
for (const entry of ["feature-sliced-design", "evals"]) {
cpSync(path.join(ROOT, entry), path.join(dir, entry), { recursive: true });
}
cpSync(
path.join(ROOT, "feature-sliced-design"),
path.join(dir, "feature-sliced-design"),
{ recursive: true },
);
mutate?.(dir);
@@ -61,8 +63,8 @@ function bodyLineCount(dir) {
function setRuleOfFirstCase(dir, rule) {
const filePath = path.join(dir, CASES);
const parsed = JSON.parse(readFileSync(filePath, "utf8"));
parsed.cases[0].rule = rule;
parsed.cases[0].source = SKILL;
parsed.evals[0].rule = rule;
parsed.evals[0].source = SKILL;
writeFileSync(filePath, JSON.stringify(parsed, null, 2));
}
@@ -142,6 +144,13 @@ test("a case whose rule matches a heading of the source passes", () => {
assert.deepEqual(problems, []);
});
test("a skill_name that differs from the skill directory is reported", () => {
const problems = problemsAfter((dir) =>
replaceIn(dir, CASES, '"skill_name": "feature-sliced-design"', '"skill_name": "fsd"'),
);
assertOneProblemMatching(problems, /skill_name "fsd" does not match the skill directory/);
});
test("a reference file pointing at a missing sibling is reported", () => {
const problems = problemsAfter((dir) =>
appendTo(dir, ASSETS, "\nMore in `references/gone.md`.\n"),
@@ -216,7 +225,7 @@ test("a null eval document is reported instead of crashing", () => {
const problems = problemsAfter((dir) =>
writeFileSync(path.join(dir, CASES), "null\n"),
);
assertOneProblemMatching(problems, /must contain a non-empty cases array/);
assertOneProblemMatching(problems, /must contain a non-empty evals array/);
});
// Ids past one digit used to match nothing at all, so a dangling reference
+2 -2
View File
@@ -5,7 +5,7 @@ on:
paths:
- "**/SKILL.md"
- "**/references/**"
- "evals/**"
- "**/evals/**"
- ".github/scripts/**"
- ".github/workflows/validate.yml"
push:
@@ -13,7 +13,7 @@ on:
paths:
- "**/SKILL.md"
- "**/references/**"
- "evals/**"
- "**/evals/**"
- ".github/scripts/**"
- ".github/workflows/validate.yml"
+4 -5
View File
@@ -68,10 +68,9 @@ feature-sliced-design/
framework-integration.md Next.js, React Router, Nuxt, Vite, Astro setup
auth-and-api.md Auth, type definitions, API request handling
state-management.md Redux, TanStack Query (React Query)
evals/
README.md How to run and maintain the cases
cases.json Placement regression cases
evals/
README.md How to run and maintain the cases
evals.json Placement regression cases
```
`SKILL.md` is the entry point. It tells the agent to read a reference file only when the task calls for it, so the initial context stays small.
@@ -93,7 +92,7 @@ It enforces this repository's skill-package rules, which are based in part on th
- Every file under `references/` is routed from the `Conditional references` section of `SKILL.md`, so a reference that section forgets fails the build instead of shipping unreachable. Naming it elsewhere in the body, or inside a fenced example, does not count. A skill with no such section falls back to requiring a mention anywhere in `SKILL.md`.
- Every numbered cross-reference resolves. `Section N`, `Section N-M`, and `Rule N-M` always mean a numbered heading in `SKILL.md`, whichever file mentions them; `Step N`, `Strategy X`, `Snapshot N`, `Part N`, and `Question N` mean a heading or bold label somewhere in the package.
- A named rule such as "the request placement rule" that is cited from more than one file is a heading or bold label somewhere in the package, so renaming the anchor fails the build instead of stranding its readers.
- `evals/cases.json` is valid JSON with at least one case; every case has `id`, `prompt`, `expect`, `why`, `source`, and `rule`, ids are unique, every `source` path exists, and every `rule` fragment resolves to a passage of the skill (see `evals/README.md`).
- Each skill's `evals/evals.json` is valid JSON whose `skill_name` matches the skill directory and whose `evals` array is non-empty; every case has `id`, `prompt`, `expected_output`, `why`, `source`, and `rule`, ids are unique, every `source` path exists, and every `rule` fragment resolves to a passage of the skill (see `feature-sliced-design/evals/README.md`).
The validator has its own tests, which break one thing at a time in a copy of the repository and assert that it is reported:
@@ -1,8 +1,14 @@
# Skill evaluation cases
`cases.json` records the placement answers this skill is supposed to produce.
`evals.json` records the placement answers this skill is supposed to produce.
The validator keeps the file honest; a person or a harness grades the answers.
The file uses the `evals/evals.json` layout that Anthropic's `skill-creator`
and [agent-skills-eval](https://github.com/darkrishabh/agent-skills-eval)
read, so the same cases run in a judge-model harness without conversion.
The `why`, `source`, and `rule` fields are this repository's additions;
harnesses ignore them and the validator checks them.
## Why this exists
The other checks in this repository verify that the documents are well formed:
@@ -16,31 +22,55 @@ noticed. A case list is the cheapest way to catch the next one.
## Running the cases
There is no semantic grader in CI, because comparing an answer to `expect`
needs a model or a person. Run them by hand, or wire `cases.json` into
whatever harness you already use.
There is no semantic grader in CI, because comparing an answer to
`expected_output` needs a model or a person.
### With a harness
From the repository root, with an OpenAI-compatible API key in the
environment:
```bash
npx agent-skills-eval . --target <model> --judge <model> --baseline
```
It runs every prompt twice, with and without the skill in context, grades
both against `expected_output`, and writes a report under
`agent-skills-workspace/`. Read the `--baseline` column first. A case that
passes without the skill is guarding a mistake the model does not make, so
consider dropping it. A case that passes only with the skill shows where the
skill changes the answer.
The harness puts `SKILL.md` and every file under `references/` into context
at once, so a pass shows that the documents decide the case correctly.
Whether `SKILL.md` sends an agent to the right reference is a separate
question the harness cannot answer, because an agent reads the skill one
file at a time. Check that by hand.
### By hand
1. Start an agent session with only this skill installed.
2. Send one `prompt` verbatim. Do not add context; the point is to see what
the skill alone produces.
3. Compare the answer to `expect`. Judge the placement, not the wording.
3. Compare the answer to `expected_output`. Judge the placement, not the
wording.
4. On a mismatch, read the file named in `source` and check whether the rule
is absent, ambiguous, contradicted elsewhere, or whether the case itself
no longer describes the behavior the skill intends. Fix whichever one is
wrong. Never edit `expect` just to match what the model said.
wrong. Never edit `expected_output` just to match what the model said.
Start a fresh session per case. A previous answer in the same conversation
will steer the next one.
## Adding a case
Add an object to `cases` with all six fields:
Add an object to `evals` with all six fields:
| Field | Meaning |
| --- | --- |
| `id` | kebab-case, unique |
| `prompt` | what the user types, verbatim |
| `expect` | the placement, plus what must not happen if that matters |
| `expected_output` | the placement, plus what must not happen if that matters |
| `why` | what regression this case guards against |
| `source` | repo-relative path to the primary file that decides it |
| `rule` | the passage that decides it, as `;`-separated fragments; name a passage from another file too when the decision leans on one |
@@ -1,11 +1,11 @@
{
"$comment": "Placement cases for the feature-sliced-design skill. See README.md in this directory for how to run them. Every `source` path is checked by .github/scripts/validate-skills.mjs.",
"skill": "feature-sliced-design",
"cases": [
"$comment": "Placement cases for the feature-sliced-design skill, in the evals/evals.json layout that agent-skills-eval and skill-creator read. See README.md in this directory for how to run them. Every `source` path and `rule` fragment is checked by .github/scripts/validate-skills.mjs.",
"skill_name": "feature-sliced-design",
"evals": [
{
"id": "auth-token",
"prompt": "Where do I put the auth token and session helpers?",
"expect": "shared/auth/ (or shared/api/). Do not create a user entity to hold the token; whether an existing current-user entity may store it is a separate question.",
"expected_output": "shared/auth/ (or shared/api/). Do not create a user entity to hold the token; whether an existing current-user entity may store it is a separate question.",
"why": "Tokens and session DTOs are infrastructure, not a domain model, and wrapping a login response in an entity is the common wrong turn.",
"source": "feature-sliced-design/SKILL.md",
"rule": "Section 2, Step 2; Section 6 anti-pattern on the user entity"
@@ -13,7 +13,7 @@
{
"id": "small-project-entities",
"prompt": "I am starting a project with two pages. Should I create the entities and features layers now?",
"expect": "No. app/ + pages/ + shared/ is valid FSD. Add entities or features only when a shared responsibility has a stable boundary its consumers must agree on; reuse alone does not require a layer.",
"expected_output": "No. app/ + pages/ + shared/ is valid FSD. Add entities or features only when a shared responsibility has a stable boundary its consumers must agree on; reuse alone does not require a layer.",
"why": "A common failure mode: small projects split into every layer up front.",
"source": "feature-sliced-design/SKILL.md",
"rule": "Section 5-2 'Start without entities'; Section 5-3"
@@ -21,7 +21,7 @@
{
"id": "plain-reusable-request",
"prompt": "getUserById just wraps GET /users/:id and every page calls it. Does it belong in entities/user/api/?",
"expect": "No. shared/api/. Plain resource access stays infrastructure however many consumers call it. A request moves into an entity only when an established entity boundary owns that domain responsibility.",
"expected_output": "No. shared/api/. Plain resource access stays infrastructure however many consumers call it. A request moves into an entity only when an established entity boundary owns that domain responsibility.",
"why": "Guards the two-question placement rule. The old bullet list sent this exact function to entities.",
"source": "feature-sliced-design/references/auth-and-api.md",
"rule": "Request placement rule, Question 2"
@@ -29,7 +29,7 @@
{
"id": "single-page-request",
"prompt": "Dashboard stats are fetched only on the dashboard page. Where does the request go?",
"expect": "pages/dashboard/api/. It stays with its only consumer.",
"expected_output": "pages/dashboard/api/. It stays with its only consumer.",
"why": "Question 1 must be asked before Question 2, or pages-first breaks.",
"source": "feature-sliced-design/references/auth-and-api.md",
"rule": "Request placement rule, Question 1"
@@ -37,7 +37,7 @@
{
"id": "same-layer-import",
"prompt": "features/profile needs something from features/auth. Is that allowed?",
"expect": "Generally no. Try strategies A to C first. If the documented last resort is genuinely necessary, import only through the other slice's public API, never its internals.",
"expected_output": "Generally no. Try strategies A to C first. If the documented last resort is genuinely necessary, import only through the other slice's public API, never its internals.",
"why": "The MUST rule reads as an absolute ban; the documented escape hatch must stay reachable.",
"source": "feature-sliced-design/references/cross-import-patterns.md",
"rule": "Strategy D; SKILL.md Rules 4-1 and 4-3"
@@ -45,7 +45,7 @@
{
"id": "shared-ui-import",
"prompt": "How do I import the Button from the shared UI kit?",
"expect": "Through the Shared UI public API: @/shared/ui when the segment index exports Button, or @/shared/ui/Button when Button has been given its own index. Never an internal file such as @/shared/ui/Button/Button.tsx.",
"expected_output": "Through the Shared UI public API: @/shared/ui when the segment index exports Button, or @/shared/ui/Button when Button has been given its own index. Never an internal file such as @/shared/ui/Button/Button.tsx.",
"why": "Rule 4-2 makes the segment index the default and a per-component index the fallback for tree-shaking, so both imports are correct and only reaching past the boundary is not.",
"source": "feature-sliced-design/SKILL.md",
"rule": "Rule 4-2"
@@ -53,7 +53,7 @@
{
"id": "business-themed-in-shared",
"prompt": "Can the company logo component and an autocomplete input live in shared/ui?",
"expect": "Yes, as long as they encode no business rules and no slice-specific behavior. Business-themed presentation and generic UI interaction logic may live in shared/ui; business logic may not.",
"expected_output": "Yes, as long as they encode no business rules and no slice-specific behavior. Business-themed presentation and generic UI interaction logic may live in shared/ui; business logic may not.",
"why": "Rule 4-5 is easy to over-apply. Shared excludes business logic, but business-themed code and UI logic are explicitly allowed, and an agent that flattens the rule removes both.",
"source": "feature-sliced-design/SKILL.md",
"rule": "Section 2, Step 2"
@@ -61,7 +61,7 @@
{
"id": "new-project-three-pages",
"prompt": "Set up an FSD structure for a shop with home, product, and search pages. Home and search both call the same fetchProducts request, and every page reads the same ProductDTO.",
"expect": "app/, pages/ with three slices, shared/. No entities or features. ProductDTO and the shared fetchProducts go in shared/api; a request only one page calls stays in that page's api/ segment.",
"expected_output": "app/, pages/ with three slices, shared/. No entities or features. ProductDTO and the shared fetchProducts go in shared/api; a request only one page calls stays in that page's api/ segment.",
"why": "The finished-structure reflex. Fails if any layer beyond three appears unprompted.",
"source": "feature-sliced-design/references/growth-walkthrough.md",
"rule": "Snapshot 0 and Snapshot 1; auth-and-api.md request placement rule"
@@ -69,7 +69,7 @@
{
"id": "diverged-rule",
"prompt": "Two pages each compute whether a product is on sale, and one copy is now out of date. What should change?",
"expect": "Move the rule to entities/product/model. Leave the ProductDTO in shared/api and the badge UI in the pages.",
"expected_output": "Move the rule to entities/product/model. Leave the ProductDTO in shared/api and the badge UI in the pages.",
"why": "Reuse alone did not open entities; a rule that must agree with itself does. Also checks that the DTO does not follow the rule into the entity.",
"source": "feature-sliced-design/references/growth-walkthrough.md",
"rule": "Snapshot 2"
@@ -77,7 +77,7 @@
{
"id": "app-header-placement",
"prompt": "New project. Where does the app-wide header with navigation and the user menu go?",
"expect": "The header composition goes in app/ as an application-level layout, not a new widgets/ slice. Its parts keep their own ownership: an already-established feature may supply a reused action, an established entity may supply domain UI, context-free UI comes from shared. A section only one page renders stays in that page.",
"expected_output": "The header composition goes in app/ as an application-level layout, not a new widgets/ slice. Its parts keep their own ownership: an already-established feature may supply a reused action, an established entity may supply domain UI, context-free UI comes from shared. A section only one page renders stays in that page.",
"why": "The header is the textbook widgets example. Placement is decided by scope, and the skill discourages opening widgets for it.",
"source": "feature-sliced-design/references/layer-structure.md",
"rule": "Where should layouts be placed?; SKILL.md Section 1 widgets callout"
@@ -85,7 +85,7 @@
{
"id": "top-level-assets-segment",
"prompt": "Should I create src/assets/ to hold all the images and icons?",
"expect": "No. Keep an asset with the slice that owns it. A presentation asset several slices must share goes to the shared UI module that owns it, global styles and imported fonts go with app-level code, and files served as-is go in the framework's configured public directory.",
"expected_output": "No. Keep an asset with the slice that owns it. A presentation asset several slices must share goes to the shared UI module that owns it, global styles and imported fonts go with app-level code, and files served as-is go in the framework's configured public directory.",
"why": "A type-based assets folder is the common default and the official guidance calls it not recommended.",
"source": "feature-sliced-design/references/asset-handling.md",
"rule": "Caution; Decision tree"
@@ -93,7 +93,7 @@
{
"id": "nextjs-app-layer-name",
"prompt": "Next.js App Router project. Where does the FSD app layer go, and what is it called?",
"expect": "src/_app/, with src/_pages/ for the pages layer. The Next.js app/ routing folder stays at the project root and only re-exports from FSD pages.",
"expected_output": "src/_app/, with src/_pages/ for the pages layer. The Next.js app/ routing folder stays at the project root and only re-exports from FSD pages.",
"why": "An older pattern kept the FSD layers unprefixed under src/ and moved the routing folder instead. Both existed in the wild.",
"source": "feature-sliced-design/references/framework-integration.md",
"rule": "Next.js; Projects on the previously recommended pattern"
@@ -101,7 +101,7 @@
{
"id": "widgets-not-deprecated",
"prompt": "Our FSD 2.0 project already uses widgets. Does 2.1 require us to remove the layer?",
"expect": "No. Widgets are discouraged for new adoption, not deprecated, so an existing layer stays valid and keeps working. processes/ is the layer 2.1 deprecates. Phasing widgets out is optional.",
"expected_output": "No. Widgets are discouraged for new adoption, not deprecated, so an existing layer stays valid and keeps working. processes/ is the layer 2.1 deprecates. Phasing widgets out is optional.",
"why": "Discouraged is not deprecated. The reader most likely arrived from 2.0 already using widgets, and the guide calls this migration optional.",
"source": "feature-sliced-design/references/migration-guide.md",
"rule": "Phasing out a small widgets layer (optional); SKILL.md Section 1 widgets callout"
@@ -109,7 +109,7 @@
{
"id": "phase-out-widget-placement",
"prompt": "We decided to remove our small widgets layer. We have an app shell, a section only one page renders, a stable user action reused on several pages that must behave the same in each, and a generic UI block. Where does each go?",
"expect": "App shell to app; the one-page section inlines into that page; the reused action and its UI to features; the context-free UI block to shared/ui. A widget that only composes features moves up to the page or the route layout.",
"expected_output": "App shell to app; the one-page section inlines into that page; the reused action and its UI to features; the context-free UI block to shared/ui. A widget that only composes features moves up to the page or the route layout.",
"why": "Splits the routing table off the deprecated-versus-discouraged question so a failure says which half is wrong.",
"source": "feature-sliced-design/references/migration-guide.md",
"rule": "Phasing out a small widgets layer (optional)"
@@ -117,7 +117,7 @@
{
"id": "entity-selector-root-state",
"prompt": "In entities/todo/model/todo.ts I am writing selectTodos. RootState is exported from app/providers/store.ts. How do I type the selector?",
"expect": "Type it against only the state it reads, e.g. (state: { todos: TodoState }) => state.todos.items. Must not import RootState from app/: entities cannot depend on a higher layer. Type against RootState at the app layer if that guarantee is needed.",
"expected_output": "Type it against only the state it reads, e.g. (state: { todos: TodoState }) => state.todos.items. Must not import RootState from app/: entities cannot depend on a higher layer. Type against RootState at the app layer if that guarantee is needed.",
"why": "Regression case: the skill's own example once made this upward import. Redux tutorials type selectors with RootState, so an agent reaches for it unless the dependency rule is spelled out.",
"source": "feature-sliced-design/references/state-management.md",
"rule": "Business-entity slice in entities; SKILL.md Rule 4-1"
@@ -125,7 +125,7 @@
{
"id": "similar-product-cards",
"prompt": "Home and search both render a ProductCard. The home card shows a recommendation reason, the search card shows a query-match snippet. Should I extract ProductCard to entities/product/ui?",
"expect": "No, not because both pages have one. They are not the same component, they change for their own reasons, and separate page-local copies are valid. Extract only if the two must agree and need one home.",
"expected_output": "No, not because both pages have one. They are not the same component, they change for their own reasons, and separate page-local copies are valid. Extract only if the two must agree and need one home.",
"why": "The whole skill decides extraction on a shared responsibility, not on a count or a matching filename, and every other case tests the positive direction.",
"source": "feature-sliced-design/references/growth-walkthrough.md",
"rule": "Snapshot 1, 'The second card is a copy, not an extraction'"
@@ -133,7 +133,7 @@
{
"id": "single-use-user-action",
"prompt": "Only the checkout page has a coupon form right now. Should I create features/apply-coupon?",
"expect": "No, not yet. Keep the form, its request, and its state in the checkout page while it has one consumer. Extract when another consumer appears and the action has to behave the same in both.",
"expected_output": "No, not yet. Keep the form, its request, and its state in the checkout page while it has one consumer. Extract when another consumer appears and the action has to behave the same in both.",
"why": "Guards the verb-equals-feature reflex, the features counterpart of small-project-entities, which no case covered.",
"source": "feature-sliced-design/SKILL.md",
"rule": "Section 2, Step 3; Section 5-1"
@@ -141,7 +141,7 @@
{
"id": "user-entity-does-not-own-token",
"prompt": "We now have entities/user because profile identity is reused across the app. Should I move the access and refresh tokens out of shared/auth into entities/user?",
"expect": "shared/auth remains the default. Entity-owned credentials are also valid, but only when an already-established user or session entity genuinely owns that authentication state; the entity existing, or profile data being reused, is not enough on its own.",
"expected_output": "shared/auth remains the default. Entity-owned credentials are also valid, but only when an already-established user or session entity genuinely owns that authentication state; the entity existing, or profile data being reused, is not enough on its own.",
"why": "The line between reusable user-domain state and authentication infrastructure moved twice while these references were being aligned, so pin where it landed.",
"source": "feature-sliced-design/references/auth-and-api.md",
"rule": "When to use shared/auth vs a user entity"
@@ -149,7 +149,7 @@
{
"id": "single-consumer-crud",
"prompt": "Only the settings page calls updateNotificationSettings. It is a plain PATCH with no business rules. Should it go in shared/api because it is CRUD?",
"expect": "No. Keep it in pages/settings/api/ while that page is its only consumer. CRUD not being entity material does not make it Shared material; it moves to shared/api once it is genuinely shared.",
"expected_output": "No. Keep it in pages/settings/api/ while that page is its only consumer. CRUD not being entity material does not make it Shared material; it moves to shared/api once it is genuinely shared.",
"why": "Guards pages-first request ownership against the shortcut that CRUD always goes to shared/api, which three passages stated before this alignment.",
"source": "feature-sliced-design/references/auth-and-api.md",
"rule": "Request placement rule, Question 1"