Skip to content

🧪 add unit test for iqraGraph#4

Merged
Moeabdelaziz007 merged 1 commit into
mainfrom
testing-improvement-iqra-graph-15171371017903317709
May 11, 2026
Merged

🧪 add unit test for iqraGraph#4
Moeabdelaziz007 merged 1 commit into
mainfrom
testing-improvement-iqra-graph-15171371017903317709

Conversation

@Moeabdelaziz007
Copy link
Copy Markdown
Collaborator

@Moeabdelaziz007 Moeabdelaziz007 commented May 6, 2026

The testing gap addressed was the lack of unit tests for the public iqraGraph API in lib/iqra/orchestrator.ts.
I implemented a new test file tests/unit/orchestrator.test.ts that mocks external LLM providers and internal dependencies.
The test verifies that the graph correctly handles user input and produces the expected response state.
This increases the reliability of the orchestration layer.


PR created automatically by Jules for task 15171371017903317709 started by @Moeabdelaziz007

Summary by CodeRabbit

  • Tests
    • Added comprehensive unit test coverage for core orchestration functionality to enhance system reliability and code quality assurance.

Review Change Stack

This commit adds a new unit test for the `iqraGraph` exported from `lib/iqra/orchestrator.ts`.
The test covers:
- Verification that the graph is defined.
- Processing of a message through the 'agent' node.
- Assertions on the message state and response content.

Mocks are used for LangChain models to ensure isolation and prevent network calls.

Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
@google-labs-jules
Copy link
Copy Markdown
Contributor

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

Copy link
Copy Markdown
Collaborator Author

@Moeabdelaziz007 Moeabdelaziz007 left a comment

Choose a reason for hiding this comment

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

❌ REJECTED: Destructive deletions detected.

@Moeabdelaziz007 Moeabdelaziz007 marked this pull request as ready for review May 11, 2026 06:47
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 11, 2026

📝 Walkthrough

Walkthrough

A new Vitest unit test suite validates iqraGraph by mocking LangChain and internal IQRA dependencies. The tests confirm the graph is defined and that it correctly processes user messages through mocked orchestration and memory modules.

Changes

iqraGraph Unit Tests

Layer / File(s) Summary
Test Setup and Module Mocks
tests/unit/orchestrator.test.ts
Imports Vitest and configures mocks for LangChain providers (@langchain/groq, @langchain/google-genai, @langchain/anthropic, @langchain/openai) and internal IQRA modules (brain, memory) to isolate tests from network calls and external dependencies.
Existence Check
tests/unit/orchestrator.test.ts
Test case asserts iqraGraph is defined.
Message Handling Test
tests/unit/orchestrator.test.ts
Test case invokes iqraGraph with an initial message state and asserts the resulting output contains two messages with the last message content matching the mocked Groq response.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A test suite hops into view,
Mocking calls, keeping real ones at bay—
iqraGraph now proven true,
Messages flowing the right way. ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'add unit test for iqraGraph' clearly summarizes the main change: adding a new unit test for the iqraGraph component, which is exactly what the changeset accomplishes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch testing-improvement-iqra-graph-15171371017903317709

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/unit/orchestrator.test.ts`:
- Line 54: The test assertion expecting an exact message count
(expect(result.messages.length).toBe(2)) is brittle; change it to assert a
minimum or check presence of required messages instead—e.g., replace with
expect(result.messages.length).toBeGreaterThanOrEqual(2) or assert that
result.messages contains the initial user message and an agent response by
checking message types/contents (inspect result.messages array entries) so the
test tolerates extra system/tool messages while still verifying the core
behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 0c289f88-b38d-4d51-b29f-4ff02594e5c0

📥 Commits

Reviewing files that changed from the base of the PR and between 555e806 and 26c5c71.

📒 Files selected for processing (1)
  • tests/unit/orchestrator.test.ts


// Assertions
expect(result.messages).toBeDefined();
expect(result.messages.length).toBe(2); // Initial user message + Agent response
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Make the message-count assertion less brittle.

On Line 54, asserting an exact count of 2 can fail on valid graph changes (e.g., extra system/tool messages) even when behavior is correct.

Suggested patch
-    expect(result.messages.length).toBe(2); // Initial user message + Agent response
+    expect(result.messages.length).toBeGreaterThanOrEqual(2); // Keep test resilient to extra orchestration messages
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
expect(result.messages.length).toBe(2); // Initial user message + Agent response
expect(result.messages.length).toBeGreaterThanOrEqual(2); // Keep test resilient to extra orchestration messages
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unit/orchestrator.test.ts` at line 54, The test assertion expecting an
exact message count (expect(result.messages.length).toBe(2)) is brittle; change
it to assert a minimum or check presence of required messages instead—e.g.,
replace with expect(result.messages.length).toBeGreaterThanOrEqual(2) or assert
that result.messages contains the initial user message and an agent response by
checking message types/contents (inspect result.messages array entries) so the
test tolerates extra system/tool messages while still verifying the core
behavior.

@Moeabdelaziz007 Moeabdelaziz007 merged commit 1f7725a into main May 11, 2026
1 check passed
@Moeabdelaziz007 Moeabdelaziz007 deleted the testing-improvement-iqra-graph-15171371017903317709 branch May 11, 2026 08:43
Moeabdelaziz007 added a commit that referenced this pull request May 14, 2026
…#95)

Wraps the existing SkillLoader (08-cognitive/skills/loader.ts) with an
Ed25519 verification + TTL cache + policy layer. L2 reads skills from
L3 (aix-agent-skills) as data; no code dependency.

Signing protocol v1:
  - skills.json:           Ed25519 over SHA-256 of JCS-canonical bytes
  - skills/<name>.md:      Ed25519 over SHA-256 of raw UTF-8 bytes
  - sig files:             detached .sig with base64url-encoded 64-byte sig

Policy: off | permissive (default) | strict.
Env vars: IQRA_MARKETPLACE_PUBKEY, IQRA_MARKETPLACE_POLICY,
          IQRA_MARKETPLACE_CACHE_TTL_MS.

Tests are real e2e (No Mocks per IQRA_SUPREME): real Ed25519 keypairs,
real SHA-256 digests, real on-disk fixtures under os.tmpdir().
15/15 tests passing. Pre-existing tsc baseline (1 error in 13-utils
personas.ts) unaffected.

Contract documented in docs/L2_L3_INTEGRATION.md.

Closes part of #71 (Hygiene Pass PR #4).

Co-authored-by: Mohamed Abdelaziz <mabdela1@students.kennesaw.edu>
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.

1 participant