Add --valuable-results to search feedback

[dtaivpp]

Summary

• Replace --valuable-result-positions with --valuable-results on firecrawl search-feedback and firecrawl feedback, forwarding the API's valuableResults field (Collect search feedback on results across all sources firecrawl#4109).
• Accepts a compact source:position list ("web:1,news:2") or a JSON array of {source, position, reason} entries.
• Update both skills: jq snippets now print source:position alongside URLs, and feedback guidance requires exhaustive marking (unlisted results are treated as not useful) with --valuable-sources reserved for URLs that were not among the returned results.

Why the source is required

Search results come back grouped — data.web, data.images, data.news — and each group is numbered from 1 independently. A bare position could only ever mean "web", so news and image results were unreachable. That matters most for those groups: their url fields are optional, so --valuable-sources can't reliably address them either. web:1 and news:1 are different results, and the parser rejects a bare 1,3 rather than guessing.

Also

Reverts the generic parsePositiveIntArrayArg helper this branch had extracted from parsePageNumbersArg — nothing shares it now that positions aren't a plain int array.

Tests

• npm run build
• npx vitest run — 23 files, 367 tests passing (5 new parser cases: compact form, JSON form with reasons, missing source, unknown source, non-positive position)

🤖 Generated with Claude Code

[cubic-dev-ai]

2 issues found across 7 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/commands/feedback.ts">

<violation number="1" location="src/commands/feedback.ts:269">
P2: The `feedback` command serves `search`, `scrape`, `parse`, and `map` endpoints, but `valuableResults` is only meaningful for search results, whose positions index into `web`/`images`/`news` groups. This change forwards `valuableResults` to every endpoint with no guard, so a user passing `--valuable-results web:1` on a scrape/parse/map job sends a body referencing groups that do not exist for that job, producing an API error. The CLI help already labels the flag "Search only"; enforce that by rejecting `valuableResults` when the endpoint is not `search`.</violation>

<violation number="2" location="src/commands/feedback.ts:269">
P1: The PR goal is to forward the API's `valuableResultPositions` field by raising a new `--valuable-result-positions` flag that accepts bare 1-indexed positions ("1,3" or [1,3]), parsed like `--page-numbers`. This change instead reuses the existing `--valuable-results` flag and `parseValuableResultsArg`, which only accepts `source:position` objects (e.g. `web:1`) and rejects bare positions, then forwards them as the field `valuableResults`. As a result the stated capability is not delivered: bare-position input errors out and the intended `valuableResultPositions` field is never sent. Note this does match the in-repo README/help text (which document `source:position`), so the code and the PR description conflict; confirm which contract the API expects and align the flag, parser, and forwarded field name.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic</sub>

[cubic-dev-ai]

P1: The PR goal is to forward the API's valuableResultPositions field by raising a new --valuable-result-positions flag that accepts bare 1-indexed positions ("1,3" or [1,3]), parsed like --page-numbers. This change instead reuses the existing --valuable-results flag and parseValuableResultsArg, which only accepts source:position objects (e.g. web:1) and rejects bare positions, then forwards them as the field valuableResults. As a result the stated capability is not delivered: bare-position input errors out and the intended valuableResultPositions field is never sent. Note this does match the in-repo README/help text (which document source:position), so the code and the PR description conflict; confirm which contract the API expects and align the flag, parser, and forwarded field name.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/commands/feedback.ts, line 269:

<comment>The PR goal is to forward the API's `valuableResultPositions` field by raising a new `--valuable-result-positions` flag that accepts bare 1-indexed positions ("1,3" or [1,3]), parsed like `--page-numbers`. This change instead reuses the existing `--valuable-results` flag and `parseValuableResultsArg`, which only accepts `source:position` objects (e.g. `web:1`) and rejects bare positions, then forwards them as the field `valuableResults`. As a result the stated capability is not delivered: bare-position input errors out and the intended `valuableResultPositions` field is never sent. Note this does match the in-repo README/help text (which document `source:position`), so the code and the PR description conflict; confirm which contract the API expects and align the flag, parser, and forwarded field name.</comment>

<file context>
@@ -261,6 +266,7 @@ export async function executeEndpointFeedback(
       ['tags', normalizeList(options.tags)],
       ['note', options.note],
       ['valuableSources', options.valuableSources],
+      ['valuableResults', options.valuableResults],
       ['missingContent', options.missingContent],
       ['querySuggestions', options.querySuggestions],
</file context>

[cubic-dev-ai]

P2: The feedback command serves search, scrape, parse, and map endpoints, but valuableResults is only meaningful for search results, whose positions index into web/images/news groups. This change forwards valuableResults to every endpoint with no guard, so a user passing --valuable-results web:1 on a scrape/parse/map job sends a body referencing groups that do not exist for that job, producing an API error. The CLI help already labels the flag "Search only"; enforce that by rejecting valuableResults when the endpoint is not search.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/commands/feedback.ts, line 269:

<comment>The `feedback` command serves `search`, `scrape`, `parse`, and `map` endpoints, but `valuableResults` is only meaningful for search results, whose positions index into `web`/`images`/`news` groups. This change forwards `valuableResults` to every endpoint with no guard, so a user passing `--valuable-results web:1` on a scrape/parse/map job sends a body referencing groups that do not exist for that job, producing an API error. The CLI help already labels the flag "Search only"; enforce that by rejecting `valuableResults` when the endpoint is not `search`.</comment>

<file context>
@@ -261,6 +266,7 @@ export async function executeEndpointFeedback(
       ['tags', normalizeList(options.tags)],
       ['note', options.note],
       ['valuableSources', options.valuableSources],
+      ['valuableResults', options.valuableResults],
       ['missingContent', options.missingContent],
       ['querySuggestions', options.querySuggestions],
</file context>