Skip to content

feat(lid-pn): make add_lid_pn_mapping pub for embedder-learned sources - #1013

Merged
jlucaso1 merged 1 commit into
oxidezap:mainfrom
blaueeiner:feat/pub-add-lid-pn-mapping
Jul 9, 2026
Merged

jlucaso1 merged 1 commit into
oxidezap:mainfrom
blaueeiner:feat/pub-add-lid-pn-mapping

Conversation

@blaueeiner

Copy link
Copy Markdown
Contributor

Embedders can learn LID-PN pairs from sources the library does not process itself — e.g. app-state ContactAction mutations carry lidJid/pnJid for the user's address-book contacts, which never hit the message/usync learning paths.

Exposing the durable learn path (add_lid_pn_mapping) lets an embedder feed those pairs through the cache, store, the #1011 source-aware write policy, and session migration, instead of writing the backend mapping table behind the cache's back (which would bypass the write policy entirely).

One-line visibility change plus a comment documenting the rationale; no behavior change.

@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

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

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 96e84abd-1d56-437d-b54c-72ca84c320fd

📥 Commits

Reviewing files that changed from the base of the PR and between 332f36b and 8b10753.

📒 Files selected for processing (1)
  • src/client/lid_pn.rs

📝 Walkthrough

Summary by CodeRabbit

  • New Features
    • Exposed LID↔PN mapping updates via a public client method so external integrations can add mappings directly through the supported API.
  • Documentation
    • Expanded documentation to guide embedders on routing learned LID↔phone pairs through the public entry point to keep cache, persistence, write policy, and session behavior consistent.

Walkthrough

Client::add_lid_pn_mapping is now public, and its documentation was expanded to direct external callers through this API for learned LID↔phone pairs.

Changes

Visibility Change

Layer / File(s) Summary
Public API exposure for LID-PN mapping
src/client/lid_pn.rs
add_lid_pn_mapping is now pub instead of pub(crate), with updated docs describing it as the public entry point for external consumers.
Estimated code review effort: 1 (Trivial) ~2 minutes

Possibly related PRs

  • oxidezap/whatsapp-rust#1011: Touches the same Client::add_lid_pn_mapping path and changes its write/conflict behavior, so it is directly related at the code level.

Suggested labels: api-design

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly states the visibility change and its embedder-focused purpose.
Description check ✅ Passed The description matches the change and explains why exposing the mapping path is needed.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

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 `@src/client/lid_pn.rs`:
- Around line 151-156: The visibility-rationale note above add_lid_pn_mapping is
using plain // comments between the #[cfg_attr] attribute and the public
function, so it won’t show up in rustdoc and breaks doc-comment placement. Move
that rationale into the existing /// doc comment for add_lid_pn_mapping so the
public API guidance is documented for external consumers, and keep the attribute
immediately followed by the function signature.
🪄 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: ASSERTIVE

Plan: Pro Plus

Run ID: 30dfc979-354d-4405-b27b-189638197fdf

📥 Commits

Reviewing files that changed from the base of the PR and between f95eb2d and 332f36b.

📒 Files selected for processing (1)
  • src/client/lid_pn.rs

Comment thread src/client/lid_pn.rs Outdated
@greptile-apps

greptile-apps Bot commented Jul 9, 2026

Copy link
Copy Markdown

Greptile Summary

This PR promotes add_lid_pn_mapping from pub(crate) to pub on the Client type, accompanied by a /// doc comment that documents the rationale and parameter semantics for external callers.

  • The change lets embedders feed LID-PN pairs that the library never observes internally (e.g. app-state ContactAction mutations) through the same cache/store/write-policy/migration pipeline as internally learned pairs, avoiding out-of-band writes that would bypass the #1011 source-aware write policy.
  • LearningSource is already publicly accessible to embedders via the pub mod lid_pn_cache re-export, so the function's signature is fully usable from outside the crate.

Confidence Score: 5/5

Safe to merge — the change is a pure visibility promotion with no altered logic, no new code paths, and no behavioral modifications.

The entire diff is a single keyword change (pub(crate) → pub) plus appended doc-comment paragraphs. The function body, write policy, persistence, and migration paths are all untouched. LearningSource is already reachable by embedders through the existing pub mod lid_pn_cache re-export, so the newly public signature is fully usable from outside the crate.

No files require special attention.

Important Files Changed

Filename Overview
src/client/lid_pn.rs One-line visibility change (pub(crate) → pub) on add_lid_pn_mapping with a new /// doc comment; no logic changes, no new code paths introduced.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant E as Embedder
    participant C as Client (pub)
    participant Cache as LidPnCache
    participant Store as PersistenceManager
    participant Migrate as SessionMigration

    E->>C: add_lid_pn_mapping(lid, pn, source)
    C->>Cache: record_lid_pn_in_memory(lid, pn, source)
    alt Skipped (already durable)
        Cache-->>C: RecordOutcome::Skipped
        C-->>E: Ok(())
    else NeedsUsync (observational conflict)
        Cache-->>C: RecordOutcome::NeedsUsync
        C->>C: spawn_lid_usync_reconcile([pn])
        C-->>E: Ok(())
    else Written
        Cache-->>C: "RecordOutcome::Written { entry, is_new }"
        C->>Store: persist_and_migrate_lid_pn(entry, is_new)
        Store->>Cache: mark_persisted(pn, lid)
        opt is_new
            Store->>Migrate: migrate_device_registry_on_lid_discovery
            Store->>Migrate: migrate_signal_sessions_on_lid_discovery
        end
        C-->>E: Ok(())
    end
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant E as Embedder
    participant C as Client (pub)
    participant Cache as LidPnCache
    participant Store as PersistenceManager
    participant Migrate as SessionMigration

    E->>C: add_lid_pn_mapping(lid, pn, source)
    C->>Cache: record_lid_pn_in_memory(lid, pn, source)
    alt Skipped (already durable)
        Cache-->>C: RecordOutcome::Skipped
        C-->>E: Ok(())
    else NeedsUsync (observational conflict)
        Cache-->>C: RecordOutcome::NeedsUsync
        C->>C: spawn_lid_usync_reconcile([pn])
        C-->>E: Ok(())
    else Written
        Cache-->>C: "RecordOutcome::Written { entry, is_new }"
        C->>Store: persist_and_migrate_lid_pn(entry, is_new)
        Store->>Cache: mark_persisted(pn, lid)
        opt is_new
            Store->>Migrate: migrate_device_registry_on_lid_discovery
            Store->>Migrate: migrate_signal_sessions_on_lid_discovery
        end
        C-->>E: Ok(())
    end
Loading

Reviews (2): Last reviewed commit: "feat(lid-pn): make add_lid_pn_mapping pu..." | Re-trigger Greptile

Comment thread src/client/lid_pn.rs Outdated
Comment thread src/client/lid_pn.rs Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

All reported issues were addressed across 1 file

Confidence score: 5/5

  • Safe to merge after the addressed issues were fixed.

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread src/client/lid_pn.rs Outdated
Embedders can learn LID-PN pairs from sources the library does not
process itself (e.g. app-state ContactAction mutations carry
lidJid/pnJid). Exposing the durable learn path lets them feed those
through cache, store, write policy, and session migration instead of
writing the backend mapping table behind the cache's back.
@blaueeiner
blaueeiner force-pushed the feat/pub-add-lid-pn-mapping branch from 332f36b to 8b10753 Compare July 9, 2026 09:31
@jlucaso1
jlucaso1 merged commit d8fd43f into oxidezap:main Jul 9, 2026
26 of 27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants