search

run-llama / llama_index

LlamaIndex is a data framework for your LLM applications
https://docs.llamaindex.ai
MIT License
35.23k stars 4.95k forks source link

[Bug]: CodeHierarchyQuery Engine doing redundant operations & not finding heirarchical nodes #12981

Closed 345ishaan closed 1 month ago

345ishaan commented 4 months ago

Bug Description

The following lines of code are redundant: https://github.com/run-llama/llama_index/blob/954398e1957027a364d0d332fee61733ad322f8b/llama-index-packs/llama-index-packs-code-hierarchy/llama_index/packs/code_hierarchy/query_engine.py#L116-L121

1) We are doing DFS to find all kvs from the root_map 2) Then we are again doing DFS to find if there is any exact match.

Instead the kvs should be made from self.node_dict and DFS should be happening on the self.root_map

cc @logan-markewich

Version

Latest

Steps to Reproduce

NA

Relevant Logs/Tracbacks

No response

dosubot[bot] commented 4 months ago

Hello @345ishaan! 👋 I'm Dosu, a bot here to assist you with bug fixes, answer your questions, and help you contribute effectively while we await a human maintainer. I'm diving into your issue about the CodeHierarchyQuery Engine doing redundant operations and not finding hierarchical nodes. I'll get back to you with a comprehensive response shortly!

logan-markewich commented 4 months ago

Feel free to make a pr @345ishaan 👍

345ishaan commented 4 months ago

Feel free to make a pr @345ishaan 👍

I have created a PR here: https://github.com/run-llama/llama_index/pull/12983 but I think the current behavior is correct and we should remove the unncessary DFS logic.

Maybe the owner of the this llama-pack can shed some light on what was the reason for doing this.

dosubot[bot] commented 4 months ago

It looks like you've already taken the right steps by creating a PR and seeking further clarification on the necessity of the current logic. Since @logan-markewich has encouraged you to make a PR and you're looking for more insight into the design decisions, it would be best for the owner of the llama-pack or @logan-markewich to provide further details on why the redundant DFS logic was initially implemented. This will help ensure that any changes made align with the overall design and functionality goals of the project.

#### Details **About Dosu** This response is meant to be useful and save you time. It is **not meant to be a precise solution**, but rather a starting point for your own research. Help me be more useful! Please leave a 👍 if this is helpful and 👎 if it is irrelevant.

To continue the conversation, mention @dosu.