Skip to content

Feature/voice authentication#2

Merged
Khubaib8281 merged 10 commits into
mainfrom
feature/voice-authentication
Jul 23, 2026
Merged

Feature/voice authentication#2
Khubaib8281 merged 10 commits into
mainfrom
feature/voice-authentication

Conversation

@tayyaba-code

Copy link
Copy Markdown
Collaborator

Fixed the voice enrollment workflow by adding validation, retries, and proper saving of valid recordings.
Improved audio preprocessing (loading, mono conversion, resampling, padding) to make it compatible with the SpeechBrain ECAPA model.
Fixed the embedding extraction pipeline and improved error handling for debugging.
Fixed the model training pipeline so embeddings are generated correctly and used to train the authentication model.
Fixed the voice verification and live authentication flow by removing bugs in waveform handling and authentication logic.
Updated the authentication adapter to use the correct verification method.
Updated and fixed the unit tests to match the implementation changes.
Performed end-to-end testing of the complete pipeline (enrollment → training → authentication).

@Khubaib8281
Khubaib8281 self-requested a review July 19, 2026 04:59

@Khubaib8281 Khubaib8281 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Acknowledge Improvements :
yes the audio preprocessing is significantly improved,

  • retry logic
  • audio validation rather than blindly accepting ,
  • unit testing is much improved
  • better exception handling
  • embedding extraction is reasonably improved and consistent than before

issues need to be addressed:

  • this PR is too long, but it'd be much better, if PRs were like, PR1: audio preprocessing, PR2: enrollment improvements. please consider splitting future work into smaller, focused PRs.
  • dummy_model.pkl is uploaded, however .pkl is in .gitignore
  • coverage.xml file isn't uploaded in artifacts, check the upload coverage job
  • still in some files exception isn't used but return False
  • consider replacing the remaining print statements with logging

Once these issues are addressed, I'm happy to review the updated changes.

@Khubaib8281 Khubaib8281 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

good about the changes:

  • improvement in logging and error handling
  • unnecessary files are removed

concern:

  • coverage report shows there is no testing for the
    enroll_and_authenticate.py module and embeddings.py | 21% , audio_utils.py | 38%
    , cli.py | 63.33% , verifier.py | 48% , trainer.py | 59% test coverage. overall test coverage is 62% which is moderate but improve the test coverage.

for this PR approval, improve the test coverage

@Khubaib8281 Khubaib8281 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

yes this PR now addresses all the issues mentioned previously and test coverage is substantially improved to 98.58% that's solid.

You can now move to the next feature

@Khubaib8281
Khubaib8281 merged commit d2086b3 into main Jul 23, 2026
4 checks passed
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.

2 participants