I think we should either disallow this "r" stripping, or preserve it in getIdentifierMap(). That is, if you make a call like:

->withIdentifiers(array('r1'))

...and the query finds a matching result, the key r1 (i.e., the exact key you passed in) should always be present in the identifier map. Otherwise you have to trim the "r" off yourself to figure out which result matches, and you might as well do that in the first place.

You can use this to identify repository PHIDs:

$repository_type = PhabricatorRepositoryRepositoryPHIDType::TYPECONST;
if (phid_get_type($identifier) === $repository_type) {
  // ...
}

(It's not perfect, but should always behave in reasonable/expected ways.)

We should initialize the identifier map in loadPage(), before executing the query:

if ($this->identifierMap === null) {
  $this->identifierMap = array();
}

Two reasons:

• willFilterPage() may get called multiple times in one query. For example, if the caller requests 100 results, and we load 100 rows but then privacy-filter 50 of them, we'll go load another 100 results to complete the request.
• If the query returns 0 results, willFilterPage() won't get called at all, so getIdentifierMap() will throw an exception ("you must call execute()") instead of returning an empty array.

I think it might be cleaner to not reuse the ids and phids property to construct this clause. One issue with the current approach is that this:

$some_query
  ->withIdentifiers(array('1', 'X'))
  ->withPHIDs(array('the-PHID-of-repository-1', 'the-phid-of-repository-Q'))

...will incorrectly select repositories r1, rX, and rQ, when it should select only repository r1.

You could just define separate properties like:

public function withIdentifiers(array $identifiers) {
  // ... split them up ...
  $this->numericIdentifiers = $numbery_ones;
  $this->callsignIdentifiers = $callsigney_ones;
}

...and then do this:

if ($this->numericIdentifiers || $this->callsignIdentifiers) {
  $identifier_clause = array();
  if ($this->numericIdentifiers) {
    $identifier_cause[] = qsprintf(...);
  }
  if ($this->callsignIdentifiers) {
    $identifier_clause[] = qsprinf(...);
  }
  $where[] = '('.implode(') OR (', $identifier_clause).')';
}

Then withIdentifiers() will interact as expected with calls separate calls to withIDs(), withPHIDs(), and withCallsigns().

This should be %Ld (list of integers) vs %Ls (list of strings).