-
-
Notifications
You must be signed in to change notification settings - Fork 609
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
errors with org files containing certain types of structures #83
Comments
It doesn't seem to be an issue with Are you seeing this issue even when you only set the Next Steps:
|
But what about numbered lists and (Additional: on excluding files and so on: can there be multiple filters? (for files in different locations) and is there a way to exclude files from a filter?) |
Ok, I think I've narrowed it down to particular types of files, for which I can get at least two different types of errors. One, a file with *-headings, but only *-headers (nothing "underneath" any of the headings):
Two, a file with no *-headings at all (but which is still a valid .org file):
I can provide specific examples of such files if necessary. |
Benjamin Slade ***@***.***> writes:
self.level = len(level)
TypeError: object of type 'int' has no len()
```
For reference, I am also seeing this error on my Org files.
Note that Orgnode is very simplistic. The most accurate and fast Org
parser in the wild that I know of is https://github.com/tecosaur/Org.jl
P.S. Your project is very promising :)
…--
Ihor Radchenko,
Org mode contributor,
Learn more about Org mode at https://orgmode.org/.
Support Org development at https://liberapay.com/org-mode,
or support my work at https://liberapay.com/yantar92
|
Ah, thanks for the additional details. I can reproduce both issues now.
Where an Context:
As an immediate mitigation, I'll make |
Debanjum ***@***.***> writes:
> Ok, I think I've narrowed it down to particular types of files, for which I can get at least two different types of errors.
Ah, thanks for the additional details. I can reproduce both issues now.
To summarize `khoj` is failing to handle two type of files:
1. Files with only entries with no body
2. Files with no entries
*Where an `entry` in `org` terminology is anything that starts with a `heading` and `heading` is anything that starts with `*`s*
Clarification: heading must start at bol and must have a space after
"*": "^\\*+ ".
3. It only indexes entries with body text (i.e has something underneath the heading)
This was done for quality of results reasons. Do you feel a need for `khoj` to index entries with only headings or is it fine to ignore such entries? If index heading only entries is needed, we can index them but I'll add a filter to ignore entries with no body filter before we do.
When Org is used for bookmark management, empty bodies are not uncommon.
Also, do you consider property drawer as a part of body?
As an immediate mitigation, I'll make `khoj` safely ignore the two cases instead of failing. Later, if needed, we can add more thought out solutions. Does that sound reasonable?
In the context of org-roam, files without headings may not be uncommon -
the "entry" is then defined by #+TITLE keyword or something similar.
Of course, not failing on Org files without headings is a good starting point.
…--
Ihor Radchenko,
Org mode contributor,
Learn more about Org mode at https://orgmode.org/.
Support Org development at https://liberapay.com/org-mode,
or support my work at https://liberapay.com/yantar92
|
Khoj doesn't currently support multiple |
Agreed, That's how it is handled in code. I was just trying to keep my definition less verbose :)
Property drawers are not considered part of body* for the purposes of indexing in
I see. Can you clarify the bookmark management scenario a bit more? Seems like there is a use-case for handling headings with empty bodies. So I can add an issue to track that change. |
Debanjum ***@***.***> writes:
I see. Can you clarify the bookmark management scenario a bit more? Seems like there is a use-case for handling headings with empty bodies. So I can add an issue to track that change.
See examples in https://github.com/yantar92/org-capture-ref
Bookmarks to this repo looks like
***** SOMEDAY [#A] debanjum [Github] debanjum/khoj: Natural Language Search Engine for your Org-Mode and Markdown notes, Beancount transactions and Photos :BOOKMARK:FLAGGED:misc:SOMEDAY:
:PROPERTIES:
:TITLE: debanjum/khoj: Natural Language Search Engine for your Org-Mode and Markdown notes, Beancount transactions and Photos
:BTYPE: misc
:ID: Github-debanjum-debanjum-khoj-natural-e4a
:AUTHOR: debanjum
:CREATED: [2022-09-07 Wed 21:51]
:HOWPUBLISHED: Github
:NOTE: Online; accessed 07 September 2022
:RSS: https://github.com/debanjum/khoj/commits.atom
:URL: https://github.com/debanjum/khoj
:END:
:LOGBOOK:
- Refiled on [2022-09-07 Wed 22:47]
:END:
…--
Ihor Radchenko,
Org mode contributor,
Learn more about Org mode at https://orgmode.org/.
Support Org development at https://liberapay.com/org-mode,
or support my work at https://liberapay.com/yantar92
|
Yeah, OrgNode is very basic. I've modified it for |
I see. This will happen to get indexed as entries with logbook drawer notes get indexed. But I see the use-case for indexing entries with no body text in |
Debanjum ***@***.***> writes:
Yeah, OrgNode is very basic. I've modified it for `khoj` to handle more scenarios but it's pretty ad-hoc. `Org.jl` looks interesting. I'm also tracking [Org-Parser](https://github.com/200ok-ch/org-parser) as they're being more methodical about parsing org syntax
org-parser has major issues with performance scaling
(200ok-ch/org-parser#56).
Org.jl, on the other hand, has been developed by one of the core Org
developers :) It is even faster than tree sitter Org syntax
(https://github.com/milisims/tree-sitter-org).
Yet another parser is https://github.com/tgbugs/laundry
Of course, the basic headline parsing does not require all these fancy
parsers.
…--
Ihor Radchenko,
Org mode contributor,
Learn more about Org mode at https://orgmode.org/.
Support Org development at https://liberapay.com/org-mode,
or support my work at https://liberapay.com/yantar92
|
Ah, hadn't seen the org-parser perf concerns. The rest of the parsers info is very informative too. But yeah for |
Debanjum ***@***.***> writes:
... The parsing required to create index is expected to be done in the background, so speed should be less of a concern.
This may be problematic if there is a single large Org file and user
makes changes to it followed by searching those changes. Such use-case is
one of the two common paradigms to organize Org files (one large file
vs. many small files aka org-roam).
I am now trying to index my 20Mb notes.org file and the estimate says
that the process will take over 1 hour to complete. Even if done in the
background, such a long re-indexing will take forever to complete.
…--
Ihor Radchenko,
Org mode contributor,
Learn more about Org mode at https://orgmode.org/.
Support Org development at https://liberapay.com/org-mode,
or support my work at https://liberapay.com/yantar92
|
Yes, indexing takes quite a while for larger data sets. Most of this is due to the model generating embeddings. And not the actual file parsing itself. PR #75 is meant to make this long indexing time only required the first time (or whenever a large amount of new data is to be indexed). But for subsequent runs it'll only re-index new or modified entries. This should speed up updating the index significantly. Enough hopefully so that the index can be updated automatically in the background from within the app itself 🤞🏾 |
Note: Currently the index has to be manually updated by the user (by calling the This shouldn't impact most practical use-cases IMO, as you're usually searching for older entries that you don't recall, not the latest edits to notes you may have just made. |
- Previously we were failing if no valid entries while computing embeddings. This was obscuring the actual issue of no valid entries found in the specified content files - Throwing an exception early with clear message when no entries found should make clarify the issue to be fixed - See issue #83 for details
- Parsed `level` argument passed to OrgNode during init is expected to be a string, not an integer - This was resulting in app failure only when parsing org files with no headings, like in issue #83, as level is set to string of `*`s the moment a heading is found in the current file
- Set LINE, SOURCE link properties in property drawer correctly for content which falls under no heading - See Issue #83 for more details
- Set LINE, SOURCE link properties in property drawer correctly for content which falls under no heading - See Issue #83 for more details
### Main Changes - bf01a4f Use filename or "#+TITLE" as heading for 0th level content in org files - d6bd7bf Fix initializing `OrgNode` `level` to string to parse org files with no headings - d835467 Throw exception if no valid entries found in specified content files ### Miscellaneous Improvements - 7df39e5 Reuse search models across `pytest` sessions. Merge unused pytest fixtures - 2dc0588 Do not normalize absolute filenames for entry links in `OrgNode` - e00bb53 Init word filter dictionary with default value as set to simplify code Resolves #83
@emacsomancer, @yantar92 I've merged fixes for the 2 main issues found on this thread to master.
It'd be great if you can try the merged changes by upgrading to a pre-release build of khoj with: pip install --upgrade --pre khoj-assistant Let me know if this hasn't fixed the above issues for you all |
The error is gone on my side. |
That's good to know! Thanks for verifying |
On the two files I had tested previously, now khoj runs without errors (though it still doesn't seem to actually index headers with no bodies, but expected given #87). But when I try running with the input-filter on a larger set of files, I am now encountering a new error (not sure what file is triggering it, as that doesn't appear as part of the error output):
|
Thanks for validating! Good to know that the initial issue is resolved. I'll push an update to index header only entries soon.
And thanks for discovering another bug! :) Could you please open a separate Github issue to track this new error. It'll make it easier to track separate bugs (and fixes) for future reference |
With a khoj.yml file containing:
running
khoj --regenerate
produces an error:The text was updated successfully, but these errors were encountered: