Skip to content

MacOS prototype#72

Merged
fooblahblah merged 7 commits intomainfrom
minor-fixes
Dec 9, 2023
Merged

MacOS prototype#72
fooblahblah merged 7 commits intomainfrom
minor-fixes

Conversation

@mrjbq7
Copy link
Contributor

@mrjbq7 mrjbq7 commented Dec 7, 2023

Some minor fixes and some macOS prototype code.

Screenshot 2023-12-06 at 10 12 47 PM

Don't merge yet, but might be a worthwhile initial direction.

  1. Cleans up some ruff warnings and minor python style things
  2. Searches for ~/.secure_ai and ~/SecureAI by default so no need to set that if using default location
  3. Updated the dependencies, because why not
  4. Fixed an exception with old_files when refreshing a page while a document is being indexed
  5. Experimental macOS menubar app using rumps module, has the version, an "Open" for opening a web browser to the streamlit backend, and a "Quit" to close it.

For some reason, the multiprocessing module wasn't working with pyoxidizer, so I just did a quick-and-dirty os.fork/os.kill/os.wait. This is largely because streamlit.web.bootstrap.run uses asyncio.run and needs to set signals in the main thread, but we want the main thread to run the UI thread... so either need to re-implement bootstrap.run in a background thread, or get the process fork/join logic to be clean and correct.

For details about rumps: https://github.com/jaredks/rumps. It also has notification integration which might be really useful for things like "document(s) finished indexing"...

Traceback (most recent call last):
  File "/Users/jbenedik/Projects/secure-ai/.venv/lib/python3.11/site-packages/streamlit/runtime/scriptrunner/script_runner.py", line 534, in _run_script
    exec(code, module.__dict__)
  File "/Users/jbenedik/Projects/secure-ai/secure_ai/Docs.py", line 379, in <module>
    app()
  File "/Users/jbenedik/Projects/secure-ai/secure_ai/Docs.py", line 258, in app
    retriever = configure_retriever(linked_files, db_path, embeddings_path, embeddings_model_name)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/jbenedik/Projects/secure-ai/secure_ai/Docs.py", line 140, in configure_retriever
    print("\n ######### old_files ######### \n", old_files, )
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
UnboundLocalError: cannot access local variable 'old_files' where it is not associated with a value
need to audit the os.fork() / os.kill() / os.wait() logic.

for some reason multiprocessing wasn't working with pyoxidizer properly
@mrjbq7
Copy link
Contributor Author

mrjbq7 commented Dec 7, 2023

Also noticed this additional exception when refreshing while i guess document indexing, and choosing the database that was being updated (in the background?)...

Traceback (most recent call last):
  File "/Users/jbenedik/Projects/secure-ai/.venv/lib/python3.11/site-packages/streamlit/runtime/scriptrunner/script_runner.py", line 534, in _run_script
    exec(code, module.__dict__)
  File "/Users/jbenedik/Projects/secure-ai/secure_ai/Docs.py", line 379, in <module>
    app()
  File "/Users/jbenedik/Projects/secure-ai/secure_ai/Docs.py", line 258, in app
    retriever = configure_retriever(linked_files, db_path, embeddings_path, embeddings_model_name)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/jbenedik/Projects/secure-ai/secure_ai/Docs.py", line 148, in configure_retriever
    vector_db = Chroma.from_documents(splits, embeddings, persist_directory=str(db_path))
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/jbenedik/Projects/secure-ai/.venv/lib/python3.11/site-packages/langchain/vectorstores/chroma.py", line 612, in from_documents
    return cls.from_texts(
           ^^^^^^^^^^^^^^^
  File "/Users/jbenedik/Projects/secure-ai/.venv/lib/python3.11/site-packages/langchain/vectorstores/chroma.py", line 576, in from_texts
    chroma_collection.add_texts(texts=texts, metadatas=metadatas, ids=ids)
  File "/Users/jbenedik/Projects/secure-ai/.venv/lib/python3.11/site-packages/langchain/vectorstores/chroma.py", line 208, in add_texts
    self._collection.upsert(
  File "/Users/jbenedik/Projects/secure-ai/.venv/lib/python3.11/site-packages/chromadb/api/models/Collection.py", line 302, in upsert
    self._client._upsert(
  File "/Users/jbenedik/Projects/secure-ai/.venv/lib/python3.11/site-packages/chromadb/api/segment.py", line 286, in _upsert
    self._validate_embedding_record(coll, r)
  File "/Users/jbenedik/Projects/secure-ai/.venv/lib/python3.11/site-packages/chromadb/api/segment.py", line 522, in _validate_embedding_record
    self._validate_dimension(collection, len(record["embedding"]), update=True)
  File "/Users/jbenedik/Projects/secure-ai/.venv/lib/python3.11/site-packages/chromadb/api/segment.py", line 534, in _validate_dimension
    self._sysdb.update_collection(id=id, dimension=dim)
  File "/Users/jbenedik/Projects/secure-ai/.venv/lib/python3.11/site-packages/chromadb/db/mixins/sysdb.py", line 353, in update_collection
    cur.execute(sql, params)
sqlite3.OperationalError: attempt to write a readonly database
Linked file: POH_TBM940____STD_EN_E0R1-Complet.pdf

@mrjbq7
Copy link
Contributor Author

mrjbq7 commented Dec 7, 2023

For running asyncio in the background, could perhaps use this approach:

https://stackoverflow.com/a/26270790

@mrjbq7
Copy link
Contributor Author

mrjbq7 commented Dec 7, 2023

I get this error sometimes, so perhaps pyoxidizer is doing something before the fork.

ggml_metal_init: allocating
objc[73986]: +[NSPlaceholderMutableString initialize] may have been in progress in another thread when fork() was called.
objc[73986]: +[NSPlaceholderMutableString initialize] may have been in progress in another thread when fork() was called. We cannot safely call it or ignore it in the fork() child process. Crashing instead. Set a breakpoint on objc_initializeAfterForkError to debug.

Need to troubleshoot that still, using a background thread would be a better solution.

@fooblahblah
Copy link
Contributor

Tested on Linux and it works as before.

@mrjbq7
Copy link
Contributor Author

mrjbq7 commented Dec 7, 2023

It's almost ready, there's one bug that causes the app to restart in a way that registers two menubar items when you try a document database query:

Screenshot 2023-12-07 at 11 48 05 AM

Have to figure out what's causing that first, then we can merge.

@gaemus
Copy link
Collaborator

gaemus commented Dec 7, 2023

@mrjbq7 this looks great!

I built and tested on my Mac. works great. minor issue of the double-launch that we discussed.

when I copied the app over to my other computer, Mac OS said it was damaged and would not run it. I tried this process in reverse too with the same result. so it seems the app will only launch directly on the machine it was built on.
xattr -rd com.apple.quarantine SecureAI.app/
fixes this and allows the app to launch. this is probably code-signing?

@mrjbq7
Copy link
Contributor Author

mrjbq7 commented Dec 7, 2023

okay, i think the double app issue was fixed -- worth doing some testing then can keep adding features , or merge and do more different stuff

# check whether the saved trial time plus trial period is greater than the current time. If so this
# indicates the trial period is over
return True if current_timestamp < trial_timestamp else False
return current_timestamp < trial_timestamp
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haha both of these are great changes and obvious from looking from the outside.

@mrjbq7
Copy link
Contributor Author

mrjbq7 commented Dec 7, 2023

As an aside, I'm hoping to update pyoxidizer to allow builds with python 3.11 and 3.12. I made a PR for them:

indygreg/PyOxidizer#729

Locally I've been testing with python 3.11.

@fooblahblah fooblahblah merged commit 88803fa into main Dec 9, 2023
@fooblahblah fooblahblah deleted the minor-fixes branch December 9, 2023 17:59
@fooblahblah fooblahblah restored the minor-fixes branch December 9, 2023 21:31
fooblahblah added a commit that referenced this pull request Dec 9, 2023
@fooblahblah fooblahblah deleted the minor-fixes branch January 22, 2024 21:47
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.

3 participants