Skip to content

Conversation

Copy link

Copilot AI commented Aug 5, 2025

This PR fixes a bug in code dependency analysis where function calls were incorrectly treated as variables being assigned, causing false dependencies between unrelated code lines.

Problem

When using complex expressions with function calls in assignments, the dependency tracker incorrectly included function names on the left side of the assignment operator (<-) in the dependency graph. This caused unrelated code lines to be linked through shared function calls.

For example, in this code:

data <- teal_data()
data <- within(data, {
  ADSL <- radsl(seed = 1)
  ADMH <- radmh(ADSL, seed = 1)
  ADVS <- radvs(ADSL, seed = 1)
  teal.data::col_labels(ADMH[c("MHDISTAT")]) <- c("Status of Disease")
  ADVS <- dplyr::inner_join(x = ADVS, y = ADSL[, c("STUDYID", "USUBJID"), drop = FALSE], by = c("STUDYID", "USUBJID"))
})

The dependency for the col_labels line was incorrectly extracted as:

attr(,"dependency")
[1] "col_labels" "ADMH" "c" "<-" "c"

This caused get_code(data, names = 'ADVS') to incorrectly include the ADMH modification line, even though ADVS should be independent from ADMH.

Solution

Modified the extract_occurrence function in R/utils-get_code_dependency.R to move function calls from the left side to the right side of assignment operators. Function calls are now properly tracked as dependencies (appearing on the right side) while only actual variables and objects appear on the left side as assignment targets.

The fix:

  1. Identifies function calls (tokens with type SYMBOL_FUNCTION_CALL) on the left side of assignments
  2. Moves them to the right side instead of removing them completely
  3. Preserves function calls as dependencies while ensuring they don't appear as assignment targets

Results

After the fix, the dependency is correctly extracted as:

attr(,"dependency")
[1] "ADMH" "<-" "c" "col_labels"

Function calls (col_labels and c) now appear on the right side as dependencies, while only the actual variable (ADMH) appears on the left side as the assignment target.

get_code(data, names = 'ADVS') now correctly excludes the ADMH modification line:

library(random.cdisc.data)
library(nestcolor)
ADSL <- radsl(seed = 1)
ADVS <- radvs(ADSL, seed = 1)
ADVS <- dplyr::inner_join(x = ADVS, y = ADSL[, c("STUDYID", "USUBJID"), drop = FALSE], by = c("STUDYID", "USUBJID"))

The fix ensures that code dependencies are more accurate and that independent data transformations don't create false dependencies through shared function calls, while still properly tracking function dependencies.

Fixes #262.


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 5, 2025


🎉 Thank you for your contribution! Before this PR can be accepted, we require that you read and agree to our Contributor License Agreement.
You can digitally sign the CLA by posting a comment on this Pull Request in the format shown below. This agreement will apply to this PR as well as all future contributions on this repository.


I have read the CLA Document and I hereby sign the CLA


You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot.

@m7pr
Copy link
Contributor

m7pr commented Aug 5, 2025

Hi @copilot you need to write a comment on this thread with just this content

I have read the CLA Document and I hereby sign the CLA so that CLA checks pass

Copilot AI changed the title [WIP] [Bug]: Improper code dependency Fix improper code dependency: exclude function calls from left side of assignments Aug 5, 2025
Copilot AI requested a review from m7pr August 5, 2025 12:26
Copilot finished work on behalf of m7pr August 5, 2025 12:26
Copy link
Author

Copilot AI commented Aug 5, 2025

Hi @copilot you need to write a comment on this thread with just this content

I have read the CLA Document and I hereby sign the CLA so that CLA checks pass

I have read the CLA Document and I hereby sign the CLA

Copilot finished work on behalf of m7pr August 5, 2025 12:28
@m7pr
Copy link
Contributor

m7pr commented Aug 5, 2025

hey, the functions should actually be moved to the right side of <- as I did in this pr
#264

Copy link
Author

Copilot AI commented Aug 5, 2025

hey, the functions should actually be moved to the right side of <- as I did in this pr
#264

Updated the implementation to move function calls to the right side instead of filtering them out completely. Function calls now appear as dependencies on the right side while only actual variables remain on the left side as assignment targets. (commit 5cc6ecc)

Copilot AI changed the title Fix improper code dependency: exclude function calls from left side of assignments Fix improper code dependency: move function calls from left side to right side of assignments Aug 5, 2025
Copilot finished work on behalf of m7pr August 5, 2025 13:04
@m7pr
Copy link
Contributor

m7pr commented Aug 5, 2025

@copilot can you update tests so that they don't test internal functions?
maybe create a qenv object and evaluate a code inside it, that is similar to the code i mentioned in the original comments
#262
It does not need to use random.cdisc.data, it can work on iris and mtcars.

… qenv-based tests for function call dependency fix

Co-authored-by: m7pr <[email protected]>
Copy link
Author

Copilot AI commented Aug 5, 2025

@copilot can you update tests so that they don't test internal functions?
maybe create a qenv object and evaluate a code inside it, that is similar to the code i mentioned in the original comments
h...

Updated the tests to use the public API instead of testing internal functions. Created comprehensive test scenarios using qenv objects with iris and mtcars datasets that replicate the original issue behavior. The tests now verify that function calls don't create false dependencies between unrelated code sections while ensuring genuine function dependencies are still properly tracked. (commit 27d1250)

Copilot finished work on behalf of m7pr August 5, 2025 14:34
@m7pr m7pr closed this Aug 7, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Aug 7, 2025
@insights-engineering-bot insights-engineering-bot deleted the copilot/fix-262 branch November 9, 2025 03:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Improper code dependency

2 participants