docs(imap/commands): revert incorrect SEARCH criteria claim#84
Merged
Conversation
… has full flag support Self-correction. On PR #81 I added a "Not yet supported" section listing ANSWERED, DELETED, DRAFT, FLAGGED, SEEN, UNSEEN as missing, based on an incomplete read of parser.ts. I grepped for `case "` and saw 17 switch statement entries, concluding the flag criteria were unsupported. I missed the FLAG_CRITERIA lookup table earlier in the same function that dispatches flag tokens BEFORE the switch: const FLAG_CRITERIA: Record<string, { flag: string; negated: boolean }> = { ANSWERED, DELETED, DRAFT, FLAGGED, SEEN, RECENT, UNANSWERED, UNDELETED, UNDRAFT, UNFLAGGED, UNSEEN, OLD }; All 12 flag criteria + `NEW` + `ALL` + the header / date / size / text / UID / NOT / OR criteria ARE supported. The parser tests at packages/imap/tests/protocol/parser.test.ts cover ANSWERED and DELETED / UNDELETED explicitly. The original doc in vault had the full list correct. PR #81's "fix" removed correct information and replaced it with an incorrect gap claim. This commit restores the correct list and adds explicit reference to the FLAG_CRITERIA table + the parser test coverage so a future reader can audit the claim against source without making the same mistake. Also closing issue #82 (the "missing SEARCH criteria" gap I filed based on the same mistake) as invalid. The actual remaining gaps are narrower: KEYWORD, UNKEYWORD, HEADER, SENTBEFORE / SENTON / SENTSINCE. Those are accurately flagged as "Not yet supported" in this revision. ## How this slipped through Per my own memory rule "Before recommending from memory, verify the file still exists" -- I verified against source but only partially. Next time: read the entire parser function, not just the switch statement. Lookup tables can live above the switch and short-circuit before any case hits. ## Verification - Verified FLAG_CRITERIA contents against packages/imap/src/protocol/parser.ts - Verified parser tests at packages/imap/tests/protocol/parser.test.ts:326-339 - pnpm test: 614 passing (unchanged) - pnpm lint: 0 warnings, 0 errors - pnpm format:check: clean Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Self-correction to a mistake in PR #81 (already merged). The SEARCH command section now falsely claims that flag-based criteria (`ANSWERED`, `DELETED`, `DRAFT`, `FLAGGED`, `SEEN`, `UNSEEN`) are unsupported. They are fully supported by the parser via a `FLAG_CRITERIA` lookup table I missed on my first audit.
How the mistake happened
I grepped for `case "` in `packages/imap/src/protocol/parser.ts` and counted 17 switch statement entries inside `parseCriterion`. I concluded the flag-based criteria were unsupported because they were not in the switch. I missed this at the top of the same function:
```typescript
const FLAG_CRITERIA: Record<string, { flag: string; negated: boolean }> = {
ANSWERED: { flag: "\\Answered", negated: false },
DELETED: { flag: "\\Deleted", negated: false },
DRAFT: { flag: "\\Draft", negated: false },
FLAGGED: { flag: "\\Flagged", negated: false },
OLD: { flag: "\\Recent", negated: true },
RECENT: { flag: "\\Recent", negated: false },
SEEN: { flag: "\\Seen", negated: false },
UNANSWERED: { flag: "\\Answered", negated: true },
UNDELETED: { flag: "\\Deleted", negated: true },
UNDRAFT: { flag: "\\Draft", negated: true },
UNFLAGGED: { flag: "\\Flagged", negated: true },
UNSEEN: { flag: "\\Seen", negated: true },
};
function parseCriterion(): SearchCriterion {
const token = next().toUpperCase();
const flagCriterion = FLAG_CRITERIA[token];
if (flagCriterion) {
return { type: "flag", ...flagCriterion }; // dispatches BEFORE the switch
}
switch (token) { ... }
}
```
The lookup table short-circuits flag tokens before the switch statement runs. All 13 flag criteria (12 flags + RECENT/OLD) are handled. Parser tests at `packages/imap/tests/protocol/parser.test.ts:326-339` explicitly cover `ANSWERED` and `DELETED` / `UNDELETED`.
The fix
Related
Lesson
For the memory: when auditing a parser, read the entire function, not just the switch statement. Lookup tables can dispatch tokens before any `case` is evaluated. My existing memory rule "Before recommending from memory, verify the file still exists" needs a corollary: "and read the whole function, not just the part that matches your grep."
Verification