Skip to content

resolve the slice function from the criterion line, not the file name - #3

Closed
samiujan wants to merge 1 commit into
mainfrom
upstream/slice-default-function
Closed

samiujan wants to merge 1 commit into
mainfrom
upstream/slice-default-function

Conversation

@samiujan

@samiujan samiujan commented Oct 14, 2026 •

Copy link
Copy Markdown
Owner

Fixes the default function resolution in rgctl slice. Based on main (8f09655).

Without --function, the function to slice within was guessed from the file stem (and the literal "process" for Python). Two failure modes followed, and the second misreported the cause:

// src/probe.ts holds `calc`
slice src/probe.ts --line 3 --variable y
  before: Error: Resource not found: function 'probe' not found
  after:  { "lines": [2, 3] }

// useAgentPositions.ts holds useAgentPositions (1-4) and fetchPositions (6-10)
slice src/useAgentPositions.ts --line 8 --variable total
  before: no PDG node for total at line 8       # blames the variable and line
  after:  { "lines": [7, 8] }

The second is the costly one: it names the variable and the line — both supplied by the caller and both correct — while the real cause is the function they never supplied and did not know was being guessed. It reads as a broken PDG, so the response is to try other lines and variables rather than to pass --function, which the error never mentions.

Fix: resolve the function that contains the criterion line, using the existing index_function_locations. --function still wins. Nested functions mean several spans can contain a line, so take the innermost. If no span contains the line, the historical stem fallback is kept, so behaviour changes only where the old default was plainly wrong.

Also name the function searched when a criterion lookup fails:

Error: Resource not found: no PDG node for total at line 8 — searched only
'useAgentPositions' (from --function). This file defines: 'fetchPositions', 'useAgentPositions'.

Unrelated errors pass through unchanged.

Verified: 7 new tests (1-based line mapping, out-of-range, resolution across two functions, innermost-span preference, no-match, error annotation, and pass-through). Full lib suite adds no new failures — the two failures present (languages::registry::tests::test_registry_creation, cli::agent_pack::tests::embedded_pack_lists_cursor_skills) reproduce on a clean main with no changes. fmt and clippy add no new findings.

Tracked upstream as sshaaf#114. This fork PR deliberately carries no closing keyword: GitHub
resolves those within a single repository, and this fork has no matching issue.

`rgctl slice` needs a function to slice within. Without `--function` it guessed
one from the file stem (`path.file_stem()`, and the literal `"process"` for
Python). A file name is not a defensible default: files routinely hold several
functions and the stem may match none of them. Two failure modes followed.

Stem matches nothing — the function is invented from the file name:

    // src/probe.ts holds `calc`
    slice src/probe.ts --line 3 --variable y
      before: Error: Resource not found: function 'probe' not found
      after:  { "lines": [2, 3] }

Stem matches one function but the line is in another — the error blames the
wrong thing:

    // useAgentPositions.ts holds useAgentPositions (1-4) and fetchPositions (6-10)
    slice src/useAgentPositions.ts --line 8 --variable total
      before: no PDG node for total at line 8        # blames the variable and line
      after:  { "lines": [7, 8] }

The second is the worse one. It names the variable and the line — the two things
the caller supplied and which are both correct — while the real cause is the
function they never supplied and did not know was being guessed. It reads as a
broken PDG, so the natural response is to try other lines and variables rather
than to pass `--function`, which the error never mentions.

Resolve the function that contains the criterion line, using the existing
`index_function_locations` index. `--function` still wins when given. Nested
functions mean several spans can contain a line, so take the innermost (smallest
span). If no span contains the line, keep the historical stem fallback, so
behaviour changes only where the old default was plainly wrong.

Also name the function that was searched when a criterion lookup fails. The
slicer reports `no PDG node for <var> at line <N>` with no mention of the
function, so the failure is unattributable; it now says which function was
searched, whether it came from `--function` or from the line, and which functions
the file actually defines. Unrelated errors pass through unchanged.

Tests: line-start mapping including 1-based and out-of-range, resolution across
two functions, innermost-span preference for nested functions, no-match, and the
error annotation (including the explicit-`--function` wording and the
pass-through case). Verified end to end as well — the four commands above return
the lines shown.

Signed-off-by: Sami Jan <sami.jan@gmail.com>
@samiujan

Copy link
Copy Markdown
Owner Author

Closing without merging — redundant now that the change is proposed upstream as sshaaf#116 (issue sshaaf#114).

This PR only ever existed for visibility in the fork. Merging it would put a commit into main that upstream will carry under a different SHA once sshaaf#116 lands, which would stop the fork's main from fast-forwarding to upstream/main.

The branch upstream/slice-default-function stays — it is still the head of sshaaf#116. Once that merges, main gets the change by fast-forwarding to upstream/main.

@samiujan samiujan closed this Oct 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant