Deleted files

[ishowvel]

Resolves #270

[ishowvel]

https://github.com/ishowvel/text-conversation-rewards/actions/runs/13540880010
tests should be passing, they pass locally and here too

[ishowvel]

tests are becoming more and more unreliable because of the timeouts, they fail in both of my pull requests when they should have passed

[ubiquity-os-beta]

The current implementation only skips counting deleted files but does not subtract their line counts from the total as specified in the issue. The code needs to be modified to: 1) Track the total lines removed from deleted files separately 2) Subtract those lines from the final line count calculation 3) Consider different weighting for deleted cypress test files vs application code. Please update the implementation to fully address these requirements.

[ishowvel]

The current implementation only skips counting deleted files but does not subtract their line counts from the total as specified in the issue. The code needs to be modified to: 1) Track the total lines removed from deleted files separately 2) Subtract those lines from the final line count calculation 3) Consider different weighting for deleted cypress test files vs application code. Please update the implementation to fully address these requirements.

@gentlementlegen total line count was made from adding the deletions of deleted files earlier, this implementation just skips adding the deleted files diff to avoid adding and then subtracting

edit: maybe this far-fetched but the bot could check if the requested changes message has been replied to; if it has, the bot could use the replies "counter argument" and review again. if the review fails again the bot should also say that "if you havent found this result satisfactory, please contact codeowners"

[ishowvel]

will qa in a bit

[0x4007]

will qa in a bit

If you can show QA that will allow us to approve this easily.

[0x4007]

The current implementation only skips counting deleted files but does not subtract their line counts from the total as specified in the issue.

It seems like if you passed in the entire file as context (not just the diff) it would be able to understand the full picture and possibly let this pass.

[gentlementlegen]

Also, I would like some unit tests with this pull-request.

[ishowvel]

The current implementation only skips counting deleted files but does not subtract their line counts from the total as specified in the issue.

It seems like if you passed in the entire file as context (not just the diff) it would be able to understand the full picture and possibly let this pass.

Passing full files would eat up the context length really quickly, if a single log is edited on a file and the entire file gets submitted as context. That would be a waste in my opinion
@gentlementlegen what do you think?

[ishowvel]

Also, I would like some unit tests with this pull-request.

Will add tomorrow, it's past midnight right now

[0x4007]

The current implementation only skips counting deleted files but does not subtract their line counts from the total as specified in the issue.

It seems like if you passed in the entire file as context (not just the diff) it would be able to understand the full picture and possibly let this pass.

Passing full files would eat up the context length really quickly, if a single log is edited on a file and the entire file gets submitted as context. That would be a waste in my opinion
@gentlementlegen what do you think?

I agree but I'm not sure how else to solve for this. Maybe it can request to see more of the file after internal reasoning step. Chain of thought @shiv810 rfc

[shiv810]

I agree but I'm not sure how else to solve for this. Maybe it can request to see more of the file after internal reasoning step. Chain of thought @shiv810 rfc

We could create a rolling code viewer, like the one used in swe-agent for code parsing. This would let us adjust the prompt step by step (CoT) as we go through the code. Another simpler option might be to look at one file(with diff) at a time, especially if the file is large. We could load as many relevant pages(file with diffs) as needed, remove unused pages, and replace them with new ones as we progress.

[0x4007]

I really hope that we can use an existing open source setup instead of reinventing the wheel.

[Keyrxng]

I really hope that we can use an existing open source setup instead of reinventing the wheel.

ubiquity-os-marketplace/daemon-pull-review#15 - Let's start here?

I've suggested LangChain before and was told no, although they have a lot of those types of tools that we could use very easily

[ubiquity-os-beta]

@ishowvel, you have used 3 of 3 available deadline extensions. Please provide an update on your progress.

[ishowvel]

@ishowvel, you have used 3 of 3 available deadline extensions. Please provide an update on your progress.

Giving an exam today, will add tests and provide qa as soon as I come back

[0x4007]

You'll need to self unassign before you get disqualified.

[ishowvel]

@ishowvel, you have used **3** of **3** available deadline extensions. Please provide an update on your progress.

on a side note, isnt the bot supposed to notify on each use of a deadline extension, or is this intentional?

[ishowvel]

knip shouldnt fail as "GithubDiff" is used in both "process.issue.test.ts" and "review-incentivizer.test.ts", so its not an unused export

[gentlementlegen]

@ishowvel, you have used **3** of **3** available deadline extensions. Please provide an update on your progress.

on a side note, isnt the bot supposed to notify on each use of a deadline extension, or is this intentional?

The top ups are based on the time since you started the task so it is intentional.

knip shouldnt fail as "GithubDiff" is used in both "process.issue.test.ts" and "review-incentivizer.test.ts", so its not an unused export

I don't think tests files are referenced as en entry point for knip which is probably why it considers that this file is not used.

[ishowvel]

knip shouldnt fail as "GithubDiff" is used in both "process.issue.test.ts" and "review-incentivizer.test.ts", so its not a u

@gentlementlegen heres the qa, i decided against removing the row with the empty changes as it wouldnt make sense at least in my opinion

[0x4007]

I just realized but the precheck said thumbs up and requested changes. It needs to clear its previous state after running again.

[0x4007]

QA seems fine

[gentlementlegen]

Changes seem fine.