Skip to content

scraping: expect.maxDrop without history is silently inert, and nothing refuses the combination #154

Closed

Description

@sebyx07

What

packages/scraping/src/expect.ts — maxDrop compares a run's yield against a baseline that only exists if history: is declared. Declare expect: { maxDrop } without history: and the guard is inert: there is no baseline, so the alarm can never fire. Nothing refuses the combination at definition time, and nothing says so at runtime.

The two wrong fixes, and why

Record history unconditionally (the sweep's original proposal) — rejected with evidence. expect.test.ts:76-80 pins the current behaviour deliberately by name: "no expect declared is no alarm and no history written". And recording unchecked runs has a real cost: a stretch of silent zero-row runs would become the median a later maxDrop is measured against, so arming the guard later would arm it against a poisoned baseline.

Leave it as documentation — rejected by axiom 3. A convention that is not a build error does not exist, and this one is invisible: the scrape runs, the guard reports nothing, and the author believes they are protected.

The right fix

Refuse the combination where it is declared. expect.maxDrop present and history absent is a definition-time refusal with a stable code, a cause naming both fields, and an executable fix: — the shape assertIdempotencyScope and assertRateLimitScope already use elsewhere in the framework: a guarantee that cannot be shown to hold is refused where it is declared, rather than assumed at 3am.

Alternatively, make it unrepresentable in the type — history required whenever maxDrop is present — which is cheaper and catches it at compile time, with the runtime refusal as the JSON/JS backstop. That pairing is what @ultimat3/auth just did for mfa.required.

Why it was deferred

It needs a new X_SCRAPE_* error code, which needs a wiki/Error-Codes.md row and a regenerated framework.manifest.json. Both are coordinator-owned, and it surfaced during review of a security PR (#152) where scope growth was the wrong trade.

Raised by CodeRabbit on #152; the underlying gap was found by the multi-agent bug sweep, 2026-08-18.

Activity

  1. developerz-ai commented on Aug 19, 2026

    @developerz-ai
    Contributor

    Thanks for reporting this bug — the silent inertness of expect.maxDrop without history: is exactly the kind of invisible gap that erodes trust in a guard.

    Scoped plan

    Problem: Using expect.maxDrop without history: is silently ignored — no validation prevents the invalid combo.

    Acceptance criteria:

    1. Defining expect.maxDrop without history: fails at config load time with X_SCRAPE_* error
    2. Error message names both fields (expect.maxDrop, history)
    3. Error includes fix: command to add history: or remove maxDrop
    4. New X_SCRAPE_* error code added to wiki/Error-Codes.md
    5. framework.manifest.json regenerated with new error
    6. Follows assertIdempotencyScope/assertRateLimitScope pattern for definition-time validation

    Areas: packages/scraping/src/expect.ts, packages/scraping/src/config.ts, wiki/Error-Codes.md, framework.manifest.json

    Forbidden fixes: Recording history unconditionally, only adding documentation, silently allowing with console warning, removing maxDrop entirely.


    Want to submit a fix? I can hand this off to the maintainer's coding agent.

    🤖 Posted by developerz.ai — the maintainer agent, not a human.

  2. sebyx07 commented on Aug 19, 2026

    @sebyx07
    ContributorAuthor

    Error-code analysis from the agent that fixed the adjacent review items, so this does not need re-deriving:

    SCRAPE_OWNED_ERROR_CODES (packages/scraping/src/errors.ts:16-39) has nothing that fits. The two near-misses both mean something else:

    • X_SCRAPE_YIELD_COLLAPSED means the opposite — a run genuinely collapsed, which is the alarm this issue is about arming, not the misconfiguration that leaves it unarmed.
    • X_SCRAPE_OUTPUT_INVALID is about rows failing the declared schema.

    One escape hatch exists: core ships X_CONFIG_INVALID, and @ultimat3/auth borrowed exactly that for mfa.required in the same release — a declared option whose guarantee cannot be shown to hold, refused where it is declared. Using it would avoid a new registry row and a manifest regeneration.

    The argument against it, which I find persuasive: packages/scraping/CLAUDE.md makes every scraping failure carry an owned code with a registered retry classification, and errors.test.ts enforces that. Borrowing X_CONFIG_INVALID here would be the first exception to a rule the package tests.

    So: a new X_SCRAPE_* code is the consistent answer, X_CONFIG_INVALID is the cheap one, and the choice is a real one rather than an oversight.

    One trap for whoever implements it. guardYield (packages/scraping/src/expect.ts:96-113) treats "no history" as a silent no-op for the maxDrop branch, and the comment at :97-101 documents delay-not-a-hole reasoning for a different case (expect absent entirely). A definition-time refusal must not break the legitimate case where history is absent because only minRows is declared — minRows needs no baseline.

  3. sebyx07 commented on Aug 20, 2026

    @sebyx07
    ContributorAuthor

    Fixed in #219 — refused at declaration time, new code X_SCRAPE_YIELD_HISTORY_MISSING.

    Confirmed inert exactly as filed: expect.ts:105-106 reads [] when history is undefined, so history.length < MIN_BASELINE_RUNS is true forever and X_SCRAPE_YIELD_COLLAPSED can never be raised. A silent no-op in a scrape expectation is a scrape that cannot fail.

    Raised from scrape() where the combination is written, before any attempt exists — which is also why it is terminal.

    expect.minRows is deliberately still legal on its own. It is an absolute floor checked at expect.ts:65-67, before the history gate, so it needs no baseline. Refusing it too would have been over-refusal, and that is pinned by its own test and mutation.

    The registry caught the author on the way: adding a code redded errors.test.ts until a retry classification was supplied — axiom 3 working, a code cannot ship unclassified.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions