Skip to content

feat(fmlint): add param-fidelity rules to catch silent-discard params (X001-X003)#72

Open
marcoaperez wants to merge 2 commits into
petrowsky:mainfrom
Taiko-Solutions:fix/fmlint-param-fidelity-x001-x003
Open

feat(fmlint): add param-fidelity rules to catch silent-discard params (X001-X003)#72
marcoaperez wants to merge 2 commits into
petrowsky:mainfrom
Taiko-Solutions:fix/fmlint-param-fidelity-x001-x003

Conversation

@marcoaperez

Copy link
Copy Markdown
Contributor

FileMaker's fmxmlsnippet parser silently discards child elements whose tag doesn't match what a step expects — the step pastes fine, but the parameter quietly falls back to its default. These bugs are invisible at the XML-schema level and only manifest at runtime (a "Go to Record" that never advances, a Card window that opens as Document, an unresolved cross-file script reference).

What this adds

Three catalog-driven fmlint rules:

  • X001 unknown-param-element — flags a child element not recognized by the step's catalog params[], with a difflib "did you mean" suggestion. Only enforced against catalog entries with status: "complete"; allowlists {Text, Animation} (elements FileMaker tolerates that the catalog deliberately doesn't model as params).
  • X002 missing-discriminator — flags a param present without its required discriminator sibling (e.g. <Layout> without <LayoutDestination> on Go to Layout / New Window).
  • X003 known-silent-discard-patterns — two hand-verified combinations: Style="Card" without the Styles bitmask (opens as Document instead); <FileReference> nested inside <Script> or missing <UniversalPathList> (cross-file script reference doesn't resolve).

Includes a synthetic test suite (11 cases) plus a mass smoke test against snippet_examples/ (0 false positives). The smoke test surfaced two catalog gaps, fixed here: the Layout param was missing entirely from New Window, and UniversalPathList was missing from Generate Response from Model.

Bonus fix

While porting this, found that the current step-catalog-en.json on main has invalid JSON — the Layout param entry under Go to Layout is missing a comma between "notes" and "discriminator", so any strict JSON parser (including this catalog loader) fails to load the file at all. Fixed in the same commit since the new tests need a loadable catalog to run.

Test plan

  • python3 -m unittest agent.fmlint.tests.test_param_fidelity -v — 11/11 pass
  • Smoke test against the full snippet_examples/ corpus — 0 false positives

@agentic-fm agentic-fm left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really nice work, @marcoaperez — this catches a whole class of bugs (silently-discarded params) that are invisible at the XML-schema level and only bite at runtime. The difflib "did you mean" touch on X001 is a good call.

Verified end-to-end on my side: fetched your branch at f7a64f5, overlaid param_fidelity.py + tests + your catalog onto our internal tree, and ran the suite — 11/11 pass, including the corpus smoke test (zero X-family false positives across all of snippet_examples). The rules are correctly gated to status: complete catalog entries with forward-compat skips, and the New Window / Perform Script catalog additions line up with the rules that consume them.

One small thing before merge: since this was opened, main picked up the missing-comma fix (the same one your diff includes at ~L2958), so the catalog has moved under you. Could you rebase onto current main? The comma line will resolve to your side / as a no-op; the rest applies clean. Approving now so it's ready to go once rebased. 🙌

@agentic-fm agentic-fm added the reviewed Reviewed and approved; awaiting merge label Jul 15, 2026
marcoaperez and others added 2 commits July 16, 2026 17:52
… (X001-X003)

FileMaker's fmxmlsnippet parser silently discards child elements whose tag
doesn't match what a step expects: the step pastes fine, but the parameter
quietly falls back to its default. These bugs are invisible at the
XML-schema level and only manifest at runtime (a "Go to Record" that never
advances, a Card window that opens as Document, an unresolved cross-file
script reference).

Adds three catalog-driven rules:

- X001 unknown-param-element: flags a child element not recognized by the
  step's catalog params, with a difflib "did you mean" suggestion. Only
  enforced against catalog entries with status "complete"; allowlists
  {Text, Animation} (elements FileMaker tolerates that the catalog
  deliberately doesn't model as params).
- X002 missing-discriminator: flags a param present without its required
  discriminator sibling (e.g. <Layout> without <LayoutDestination> on
  Go to Layout / New Window).
- X003 known-silent-discard-patterns: two hand-verified combinations —
  Style="Card" without the Styles bitmask (opens as Document instead);
  <FileReference> nested inside <Script> or missing <UniversalPathList>
  (cross-file script reference doesn't resolve).

Includes a synthetic test suite (11 cases) plus a mass smoke test against
snippet_examples/ (0 false positives). The smoke test surfaced two catalog
gaps, fixed here: the Layout param was missing entirely from New Window,
and UniversalPathList was missing from Generate Response from Model.

While porting this, found that the current step-catalog-en.json on main
has invalid JSON — the Layout param entry under Go to Layout is missing a
comma between "notes" and "discriminator", so any strict JSON parser
(including this catalog loader) fails to load the file at all. Fixed the
same commit since the new tests need a loadable catalog to run.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
…xtures

Two test fixtures used client-specific / non-English placeholder names
("Borneo-Controller", "Controlador") left over from local development.
Replaced with generic "OtherFile" throughout — no behavior change, tests
still pass identically.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
@marcoaperez
marcoaperez force-pushed the fix/fmlint-param-fidelity-x001-x003 branch from f7a64f5 to 87a601f Compare July 16, 2026 15:53
marcoaperez added a commit to Taiko-Solutions/agentic-fm that referenced this pull request Jul 16, 2026
…NTS.md

Per maintainer review: agent/scripts/ is reserved for agent/LLM-operation
scripts (companion_server.py, deploy.py, clipboard.py); repo-level CI and
developer tooling doesn't belong there. Moves:

  agent/scripts/ci_checks.py       → scripts/ci_checks.py
  agent/scripts/hooks/pre-push     → scripts/hooks/pre-push
  agent/scripts/install-hooks.sh   → scripts/install-hooks.sh

Updates every internal path reference accordingly (REPO_ROOT depth in
ci_checks.py, the hook's own self-reference, install-hooks.sh's
HOOKS_SRC). Adds scripts/AGENTS.md with actionable setup instructions —
what each file does, the one-command install, what the hook gates on,
and the emergency bypass — written for an agent wiring this up in a
collaborator's clone, per the review request.

Also hardens ci_checks.py while verifying it against a real upstream/main
checkout: the "converter tests" and "fmlint tests" steps referenced
agent/scripts/test_fm_xml_to_snippet.py and agent/fmlint/tests/, neither
of which exists on main yet (the fmlint suite ships in petrowsky#72, still open;
the converter test file was never proposed upstream). Both steps now
check for their target path first and report SKIP instead of a hard FAIL
when it's absent, so the checks stay usable standalone rather than
red out of the box on a checkout that doesn't have those suites yet.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

reviewed Reviewed and approved; awaiting merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants