feat(core): add temporal qualifiers and time-aware search (SPEC-82) - #1426
Open
phernandez wants to merge 11 commits into
Open
feat(core): add temporal qualifiers and time-aware search (SPEC-82)#1426phernandez wants to merge 11 commits into
phernandez wants to merge 11 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 417a355fe4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 56ad933afb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Member
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 56ad933afb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
phernandez
force-pushed
the
spec-82-temporal
branch
from
September 1, 2026 18:37
2ce4bf5 to
4e1246c
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e1246cdd5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
phernandez
force-pushed
the
spec-82-temporal
branch
from
September 1, 2026 18:56
4e1246c to
64cd016
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 64cd0161a6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c0180bdfcc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d3f7079493
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 85c83c5d99
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
phernandez
force-pushed
the
spec-82-temporal
branch
from
September 2, 2026 04:45
ae493d3 to
06cce5f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 06cce5f98d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bfafc9c067
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Observations may carry one authored temporal qualifier naming when a claim
applies in the world, and search gains explicit valid-time filtering over
it. Valid time and recorded time stay separate axes; recorded history waits
for SPEC-59 stable identity rather than shipping a half-version that makes
false historical claims.
Authored forms accept what dateparser reads, canonicalized into one portable
range model:
@effective[2026-06-10,2026-07-27) range literal, agents and precision
@effective:2026-07-27 point, role named
@2026-07-27 point, files on valid
A naive timestamp is read as UTC, matching the house convention rather than
enforcing an offset rule that exists nowhere else in the system. Date-only
input never acquires a time of day. Slash-formatted dates resolve under a new
date_order config setting. The only diagnostic is an unknown role: everything
else either reads as time or stays ordinary content, silently, because direct
file editing is a supported path and does not deserve warnings.
Storage is a portable projection table with scalar bounds, so SQLite and
Postgres share one logical contract and pass the same containment, overlap,
inclusivity, unbounded, and empty-range tests. Rows rebuild on every index
pass and die with the entity. Undated notes are unchanged when no temporal
filter is present, and excluded when one is.
Closes the Phase 1 and Phase 2 MVP of SPEC-82.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014pmKq6bqCi6Zp6BTHuZjrp
Signed-off-by: phernandez <paul@basicmachines.co>
Calendar dates are a discrete domain, so preserving authored bounds made the overlap predicate report false positives: (2026-01-01,2026-01-03) holds only Jan 2 and (2026-01-02,2026-01-04) holds only Jan 3, yet each raw endpoint lies inside the other. Date ranges now canonicalize to half-open [lower,upper) at construction, the way PostgreSQL normalizes daterange, so the existing predicate becomes correct without special cases. Instants are a continuous domain and are left alone. The authored token is preserved separately in source_text, so files still round-trip byte-exact. Also: the malformed-qualifier diagnostic interpolated a Path, which renders with backslashes on Windows and failed the assertion there. It now uses as_posix(), matching how entity.file_path, permalinks, and search rows all name files. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014pmKq6bqCi6Zp6BTHuZjrp Signed-off-by: phernandez <paul@basicmachines.co>
Multi-word dates now work by delimiting them explicitly:
- [decision] @occurred:"June 10, 2026" The cutover ran.
Inside quotes the author has marked where the date ends, so the
specific-day guard does not apply and dateparser's full vocabulary is
reachable — including relative and month-only forms whose unquoted
spellings are refused. Unquoted behavior is unchanged: one token, same
guards. The quoted opener must be tried before the bare-point pattern,
whose \S+ would otherwise capture the opening quote and read '"June'
as June. Only the double quote opens a value, matching the one existing
quote-aware scanner; an unterminated quote is reported rather than
raised, so one typo cannot fail a note's whole index.
A refused token now names the fix when the line looks like a truncated
date — a known kind and a following digit — and stays silent otherwise,
so prose is never nagged.
Naming: the effective/valid/occurred/due/mentioned family is now 'kind',
because that is what it says — what kind of time this is. Date-vs-instant
becomes 'axis', which is what the code already called it in prose. Both
names now describe what they hold. The migration is edited in place
rather than stacked, since SPEC-82 has not shipped.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014pmKq6bqCi6Zp6BTHuZjrp
Signed-off-by: phernandez <paul@basicmachines.co>
@effective:9999-12 constructed date(10000, 1, 1) and raised, and nothing in the chain caught it: not the qualifier reader, not the observation parser, not entity_parser. A note whose second observation carried that qualifier failed the whole document parse — its other observations and all its relations went with it. Auditing the rest of the successor arithmetic found a worse instance the report did not name: _instant_value calls astimezone(UTC), which raises OverflowError when the offset shift crosses the calendar edge. OverflowError is not a ValueError, so it escaped even the existing except clause, and the same bounds reach the search router — where ValueError maps to 400 and this was a 500. Three spellings were reachable, including an underflow at 0001-01-01. Terminal periods now render as the unbounded range they represent (@effective:9999 -> [9999-01-01,), @effective:9999-12 -> [9999-12-01,)); a year beyond the calendar stays content, and an instant that leaves the calendar in UTC is refused at the bound rather than thrown. The other six arithmetic sites were audited and are safe, each for a stated reason. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014pmKq6bqCi6Zp6BTHuZjrp Signed-off-by: phernandez <paul@basicmachines.co>
Two SPEC-82 paths turned a malformed temporal value into a plausible-looking answer rather than an error. **All-projects search reported "no matches" for an invalid filter.** With `search_all_projects=True`, each per-project leg converted the API's 400 on a bad `valid_at` / `valid_overlaps` / `time_kind` into a `# Search Failed` string, which the fan-out cannot tell from a project being unavailable: it logged it and skipped on. Every project skipped left an empty response that still reported `temporal_applied=True` -- a typo wearing the shape of a successful search. `search_notes` now parses the three filter strings once, before the fan-out begins, and raises naming the bad value. Per-project availability failures are still logged and skipped exactly as before; only client-side validation failures abort. The parser itself moves to `temporal.parse_temporal_filter`, so the tool's pre-check and `search_service.build_temporal_filter` can never disagree about what is well formed. **An impossible ISO timestamp was indexed as a different instant.** `@occurred:2026-13-01T10:00:00` fell through to dateparser, which reads it as 10:00 on the 13th of January and projected `[2026-01-13T10:00:00.000000Z,)`. Every reindex reproduced the same wrong instant. The date-only branch above it already took a strict path for exactly this reason; the canonical timestamp shape now takes the same one and is refused rather than reinterpreted, leaving the qualifier as ordinary observation content. Flexible spellings the canonical form does not cover (`2026-06-10 10:00 AM`, a timestamp with no seconds) still reach dateparser unchanged. Signed-off-by: phernandez <paul@basicmachines.co>
The strict guards added for the canonical `YYYY-MM-DD` and `T`-separated
timestamp shapes matched a token only when it was *exactly* one of them, so
every other ISO-shaped spelling still reached dateparser:
@occurred:2026-13 -> [2026-09-13,) (on 2026-09-01)
@occurred:"2026-13-01 10:00:00" -> [2026-01-13T10:00:00.000000Z,)
@occurred:2026-13-01T10:00 -> [2026-01-13T10:00:00.000000Z,)
dateparser reads the impossible month as a *day* and then fills the month it
never got from the current date. `2026-13` is the worst of these: the same note
projects a different date on every reindex day, so a query that matched it last
week can stop matching it today with nothing having been edited. Both were also
peeled off the observation content, so the wrong date replaced the author's text
rather than sitting beside it.
Extend the same strict-branch pattern one step further out: validate the ISO
calendar components a point *opens* with, and refuse the point when they name no
real date. Only the leading date is judged, so the flexible spellings keep
working -- `2026-06-10 10:00 AM`, `2026-06-10T14:00`, `2026-06`, `2026-1-5`,
`10/07/2026` and every natural-language phrase go on reaching dateparser
untouched, and an impossible *time* already read as no date at all.
Refusal is `None`, as in the neighbouring branches: the token stays ordinary
observation content, unindexed but still full-text searchable.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014pmKq6bqCi6Zp6BTHuZjrp
Signed-off-by: phernandez <paul@basicmachines.co>
The ISO guard matched calendar components at a fixed two-digit width, so a run of the wrong width matched nothing at all and fell through to dateparser -- the one outcome the guard exists to prevent. `@occurred:2026-01-0100` came back as `[2026-01-01,2026-02-01)` and `@occurred:2026-0100` as the whole of 2026, so a slipped keystroke silently widened one day into a range nobody wrote, and every reindex projected it again. Match each component as an unbounded run and judge it, rather than bounding the run and letting a wider one escape. Width is what separates an author's shorthand from an author's typo: a month or a day is written with one or two digits, so `2026-1-5` stays a legitimate unpadded spelling while the `0100` in `2026-01-0100` is no day at all. Width is checked before conversion because `date` takes a C long and raises OverflowError -- not the ValueError the guard catches -- once a run of digits grows past it. Every flexible spelling still reads exactly as before: `2026-06-10 10:00 AM`, `2026-06-10T14:00`, `2026-06`, `9999-12`, `2026-1-5`, `2026/03/04`, `June 10, 2026`. The lookahead still ends the run at the first character that cannot continue a calendar date, so a date carrying a time is matched on its date part alone. Signed-off-by: phernandez <paul@basicmachines.co>
The ISO guard validated only the calendar prefix, so a token that was
well formed in front and malformed behind fell through to dateparser,
which drops a suffix it cannot use and answers with the date alone:
@occurred:2026-01-01T -> [2026-01-01,)
@occurred:2026-01-01Z -> [2026-01-01,)
@occurred:2026-01-01+14:00 -> [2026-01-01,)
The author reached for an instant and the index recorded a whole
open-ended day, peeled off the observation and re-derived identically by
every reindex.
This is the fourth cut at the same guard, and the three before it were
each too narrow in the same way: a bounded or prefix-only test lets a
wider malformed shape slip past, because "no match" means "not this
guard's business". So the fix is not another branch for the reported
suffixes. Naming reserved markers would have caught exactly those three
and missed `2026-01-01UTC`, `2026-01-01,` and `2026-01-01-`, and would
have missed the worse defect entirely: a stray character makes dateparser
abandon the ISO reading and re-guess the components under the configured
order, so `2026-06-10x` came back as *October 6* -- the date itself moved.
Judge the whole token instead. An ISO-shaped point has exactly two
halves, and both are now checked:
* The calendar head must name a real date. Its trailing `(?![\d-])`
lookahead is dropped, because that lookahead was the same hole on the
suffix side: `2026-01-01-` matched nothing at all and so skipped the
guard. A head that always matches leaves a remainder always judged.
* Whatever trails the head must be a time of day on that very date --
the only thing that can legally follow a complete calendar date.
The second test is stated on what the reader *returned*, not on what the
suffix looks like, and that is what closes the class rather than three
examples: any trailing text dateparser silently drops or reinterprets
fails it, anticipated or not. It also catches a shape nobody reported --
`2026-06 10:00` gave dateparser a clock but no day, and it filled the day
from *today*, so the note projected `2026-06-07` in March and
`2026-06-01` in September.
Because the rule asks the reader rather than parsing the suffix, no
readable spelling is lost, including ones no grammar would have admitted:
`2026-06-10 10:00 AM`, `2026-06-10T14:00`, `2026-06-10 14:00:00+02:00`,
`2026-06-10 noon`, `2026-06-10 2pm`, `2026-06`, `9999-12`, `2026-1-5`,
`2026/03/04`, `June 10, 2026` all read exactly as before. The strict
range-literal path already validated its bounds whole -- `[2026-01-01T,)`
was always a hard error -- so this brings the lenient path to the same
standard: the reader must account for everything the author typed.
Refusal is `None`, as in the neighbouring branches: the token stays
ordinary observation content, unindexed but still full-text searchable.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014pmKq6bqCi6Zp6BTHuZjrp
Signed-off-by: phernandez <paul@basicmachines.co>
The previous commit closed the dangling-suffix hole by adding a fourth
rejection clause to `parse_authored_point`. That is the shape the file had
been drifting into, and `.agents/skills/pythonic-code/SKILL.md` names it
in its opening paragraph: describe what the program supports, rather than
start from a broad representation and grow a list of invalid combinations.
Four rounds of review had appended four such clauses -- exact
`_DATE_BOUND`, exact `_INSTANT_BOUND`, an ISO-component check, then the
trailing-text check -- and the reason each round found a new gap is
structural, not an oversight. Every clause tested a *shape*, and failing a
shape test meant "not my business", so the token fell through to
dateparser, which guesses. The next round then found another shape that
failed the test.
Model the positive space instead. An author writes a point in one of two
languages, and they come with opposite promises: ISO calendar syntax fixes
its own meaning and must be read literally; anything else (`June 10,
2026`, `2026/03/04`, `yesterday`) has no literal reading for a guess to
contradict, so the flexible reader is trusted with it. A token is
classified into that closed union once:
type _AuthoredPoint = _IsoDay | _IsoMonth | _MalformedIso | _FlexiblePoint
consumed with an exhaustive `match` and `assert_never`, following
`EntityVectorPreparePlan` and the other closed unions in this codebase.
What closes the class is that `_classify_authored_point` is *total*:
opening with ISO syntax settles the question, and the three ISO variants
are all a token can then be. There is no fourth answer and no
fall-through, so no ISO-shaped token can reach an unvalidated guess --
`2026-13-01`, `2026-01-0100`, `2026-0100`, `2026-01-01T`, `2026-01-01Z`,
`2026-01-01+14:00`, `2026-01-01-` and `2026-06-10x` are all just
`_MalformedIso`, with no clause of their own. A fifth shape has nowhere
to arrive.
Two variants carry their invariant in their type rather than in a check:
* `_IsoDay` holds the day the author wrote, so the flexible reader can
only ever supply the *clock* -- its answer is checked against that day.
* `_IsoMonth` has nowhere to put trailing text, which makes `2026-06
10:00` unrepresentable rather than merely rejected. That one mattered:
a clock reading needs a day to fall on, and dateparser filled the
missing day from *today*, so it read as June 7 in March and June 1 in
September.
Observable behavior is unchanged apart from the refusals the previous
commit introduced; the full unit suite (6552 tests) and the ten temporal
suites pass, and
`test_the_iso_guard_leaves_every_readable_spelling_to_the_lenient_reader`
is untouched and green.
The split also surfaced a real gap the old shared code path hid: the
flexible reader's instant branch had only ever been exercised by
ISO-shaped tokens, so nothing pinned `@occurred:"June 10, 2026 2pm"` or
`@occurred:"10/07/2026 14:00"`. Both are supported forms, and both are
now tested.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014pmKq6bqCi6Zp6BTHuZjrp
Signed-off-by: phernandez <paul@basicmachines.co>
`_INSTANT_BOUND` caps a fractional second at six digits and refuses a longer one rather than truncating it, because dropping digits would store a different instant than the author wrote. That refusal only ever governed the strict path. An over-precise point never matches `_INSTANT_BOUND`, so `_read_iso_day` fell through to the flexible reader, which truncated the fraction and answered with a time of day on the very day the ISO head names. Every check that path makes then passed: `@occurred:2026-01-01T10:00:00.1234567` and its quoted form both indexed as `2026-01-01T10:00:00.123456Z`, re-derived identically on every reindex, with nothing said to the author. The day check is what guards the flexible reading, and a truncated fraction sails straight through it -- the digits it drops were never in the answer to be checked. Judged on the author's text in `_classify_authored_point` instead, so both readers refuse the same token for the same reason, and for the same reason the calendar width rule already exists: a digit run wider than the syntax allows is a typo, not a shorthand. Six digits and fewer are untouched, so `14:00:00.5` and `.123456` still read; the range-literal path already refused these correctly. This also corrects an overstated claim in the classifier's own comment. It said none of the three ISO variants can reach the flexible reader, which is false: an `_IsoDay`'s trailing text is deliberately read by it, and that is what reads `2026-06-10 10:00 AM`. What the classifier actually settles for good is the calendar -- a head naming no date dies there and is never re-guessed. The trailing is fenced by two rules, one on what the reader returned and one on what the author wrote, and the comment now says so. A sweep of 216 ISO-time spellings (T/t/space separators, minute and second precision, Z/z and offset zones with and without a colon, fractions of 0/1/6/7/9 and 30 digits) reported 54 tokens whose stored instant disagreed with a literal reading of the text. All 54 were this one defect; after the fix the sweep reports none. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014pmKq6bqCi6Zp6BTHuZjrp Signed-off-by: phernandez <paul@basicmachines.co>
phernandez
force-pushed
the
spec-82-temporal
branch
from
September 2, 2026 06:31
bfafc9c to
5782845
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5782845549
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| def _read_flexible_point(point: str, date_order: DateOrder) -> TemporalRange | None: | ||
| """Read a point written in no machine syntax, taking the flexible reader at its word.""" | ||
| date_data = _date_data_parser(date_order).get_date_data(point) |
There was a problem hiding this comment.
When a note uses a documented relative or under-specified form such as @occurred:yesterday, @occurred:"2 days ago", or a yearless date, get_date_data() resolves it against the wall clock of each parse. Reindexing the unchanged note on a later day therefore replaces its temporal row with a different valid range (for example, yesterday resolved to [2026-09-01,) on September 2 and [2026-09-09,) on September 10), so historical searches change without any canonical Markdown edit and repeated reindexing never converges. Pass a stable reference derived from the accepted note version into dateparser, or reject relative/under-specified qualifiers.
AGENTS.md reference: AGENTS.md:L161-L165
Useful? React with 👍 / 👎.
Two ways a valid-time query could silently return nothing, both from derived
state answering a question about the note it came from.
**An authored assertion could become permanently unqueryable.** A temporal
qualifier is peeled off an observation before the content is stored, so two
lines that differ only in their qualifier persist identical content and derive
identical synthetic permalinks. The search index is unique on (permalink,
project_id), so `index_entity_markdown` skipped the second as a duplicate --
while its temporal row went on addressing an observation with no search
projection. Querying the second window returned nothing, and every reindex
reproduced the omission from the same markdown:
- [decision] @effective[2026-06-10,2026-07-27) The cache layer will use Redis.
- [decision] @effective[2027-06-10,2027-07-27) The cache layer will use Redis.
Reproduced before the fix: two observation rows, one search row, two
memory_time_index rows, and the 2027 query returning zero results.
The duplicate check is not what is wrong -- it guards a real unique index. Its
input is. Identity is derived after the peel, so it is derived from a string the
note does not consider distinguishing. Nothing on the observation row separates
these two, so no row-local rule can: category, content, context and tags are all
equal, and the qualifier lives in its own projection by an explicit design
decision. A relationship to that projection is not usable either -- all three
readers of `Observation.permalink` read it on *detached* instances after their
session has closed, so a lazy load would raise rather than resolve.
So the ordinal is stored, exactly as `note_section.duplicate_index` already does
for duplicate headings. `replace_observations_for_generation` is the one place
that sees a note's whole observation set in document order, and it counts the
ordinal over `observation_permalink_tail` -- shared with `Observation.permalink`
so the count is taken over exactly the identity the address is built from,
rather than rebuilt inline and drifting (#929). Keying on the generated tail
rather than raw values also closes slug aliasing, where `Foo Bar` and `foo-bar`
are different content that generate one permalink.
The ordinal is 0 for the first observation of any identity, so every permalink
that resolves today is byte-identical afterwards; only later twins gain a
suffix. It fixes the same collision for two observations differing only in
`(context)`, which had the same defect for the same reason.
**A valid-time query combined with `note_types` could not match anything.** A
note's type lives in its frontmatter, so only its entity row carries
`metadata.note_type`; observation rows carry tags and relation rows carry
nothing. Both backends read the type off each row, which asks "is this row an
entity of type X?" when the question was "does this row belong to a note of type
X?". Valid time selects observation rows, so the two predicates were never true
of the same row and the conjunction was unsatisfiable.
Resolved through the owning note instead, in one shared builder both backends
call -- only the JSON accessor differs, and that is all each supplies. Every
search row already carries `entity_id` and an entity row's own `id` equals it,
so one non-correlated membership test covers all three row kinds. Non-correlated
for the reason `temporal_filters` documents: SQLite's `search_index` is an FTS5
virtual table and a correlated EXISTS beside a MATCH is refused outright.
This makes `note_types` return observation and relation rows of matching notes,
where it previously collapsed to entity rows. That is the fix, not a side
effect: restricting which *kind* of row may match is `entity_types`' job, the
two axes are independent, and a query setting neither returns all three kinds.
`test_search_type` asserted the old entity-only shape and is updated -- it
recorded what the defect allowed, not an intention.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014pmKq6bqCi6Zp6BTHuZjrp
Signed-off-by: phernandez <paul@basicmachines.co>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3281318271
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| duplicates_seen: dict[str, int] = {} | ||
| rows = [] | ||
| for obs in observations: | ||
| identity = observation_permalink_tail(obs.category, obs.content) |
There was a problem hiding this comment.
Fresh evidence beyond the earlier observation-identity finding is that a hashtag-promoted observation arrives here with category=None, while an explicit [note] observation arrives with category="note"; this identity calculation treats them as different, but SQLAlchemy applies the model's "note" default to the first row during flush. If their peeled content is identical, both therefore receive ordinal 0 and later produce the same permalink, causing SearchService.index_entity_markdown to skip one search row while its temporal assertion points to an unsearchable observation ID. Reindexing repeats the same omission, so normalize None to the persisted category before counting identities.
AGENTS.md reference: AGENTS.md:L161-L165
Useful? React with 👍 / 👎.
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.
What
SPEC-82 Phases 1+2. Observations may carry one authored temporal qualifier saying when a claim applies in the world, and search gains explicit valid-time filtering over it. Recorded-time history is deliberately not implemented — it depends on SPEC-59 stable identity, and a half-version would make false historical claims.
search("cache layer", role=effective, valid_at=2026-07-28)returns only Memcached; at2026-07-01only Redis; with no temporal filter both compete under ordinary relevance, exactly as today.Authored forms
Role is optional in both; omitted, it files on
valid. The bracket form needs no separator (no role name starts with[/(); the point form needs:because a date can begin with a letter (yesterday).Accepts whatever
dateparserreads — already a dependency, already used for frontmatter dates atentity_parser.py:198. Precision is the meaning: a year is the year, a month is the month, a date is from that date onward.Decisions that changed during the build
Three product calls landed mid-implementation and were applied afterward (decision note):
utils.py,recent_activity.py). A rule enforced by one feature and nowhere else is an inconsistency, not a principle. Replaces acceptance criterion 10 with its inverse.date_orderconfig (YMDdefault) so slash-formatted dates resolve by documented preference rather than silent guess.Storage
Portable projection table with scalar bounds and inclusivity flags, so SQLite and PostgreSQL share one logical contract and pass the same containment, overlap, inclusivity, unbounded, and empty-range tests. Rows rebuild on every index pass and die with the entity — a derived projection, never canonical. Native PostgreSQL range columns remain a later optimization generated from these columns.
Guardrails worth noting
mayas May andv2as February; a bare@wordat the head of an observation is a mention or a version, and accepting it would file wrong valid time onto ordinary prose. Relative dates still work with an explicit role (@occurred:yesterday).Verification
ruff, format,
ty— clean. Full unit suite 6,222 passed, 41 skipped (real captured exit codes). 100% coverage ontemporal.py(178/178),temporal_qualifier.py(53/53),temporal_filters.py(38/38). The migration docstring's claim that full-text always drives temporal queries was corrected —has_temporal_filter()counts as criteria, so temporal-only searches are valid.Open question for review
Point precision currently means: year → the year, month → the month, date → from that date onward. That is right for
effective/valid(a state that begins and persists) but arguably wrong foroccurred/due(an event at a time) —@occurred:2026-06-10currently means "occurred from June 10 onward," so a query for June 20 matches it. Making the interpretation role-dependent (state roles open-ended, event roles bounded by the written precision) would resolve it. Flagged rather than decided, and it matters for the Moby Dick eval, which authors@occurredheavily.🤖 Generated with Claude Code
https://claude.ai/code/session_014pmKq6bqCi6Zp6BTHuZjrp
Accepted authored forms (measured, not asserted)
The qualifier is one whitespace-delimited token. dateparser tolerates trailing junk, so there is no signal for where a multi-word date ends — the vocabulary is not the limit, the token boundary is.
valid_during@occurred:2026-06-10[2026-06-10,)@occurred:2026-06[2026-06-01,2026-07-01)@occurred:2026-06-10T10:00:00[2026-06-10T10:00:00Z,)@occurred:03/04/2026[2026-04-03,)YMD·DMY ·[2026-03-04,)MDY@occurred:yesterday[2026-08-31,)@2026-06-10/@2026-06/@2026@effective[2026-06-10,2026-07-27)@occurred:June 10, 2026@occurred:2 days ago@may@v2@1@3.5@yesterday@asserted:2026-06-10The reader (
parse_authored_point) understands more than the grammar can delimit —June 10, 2026,10 June 2026,Jan 15, 2024,2 days ago,last weekall parse correctly. They are unreachable from an observation line only because the token cannot be delimited.A silent-truncation bug found while pinning this
Before this branch,
@occurred:June 10, 2026 The cutover ran.matched only@occurred:June, filed June 2026 as the assertion, and left10, 2026 The cutover ran.as the indexed content — silently valid and wrong. Two shapes that parse are now refused so a truncated read can never become a plausible assertion:1reads as January,3.5as March 5);June,may,v2).A word is taken only when it resolves to a specific day — locale-neutral, no hardcoded English month list. Multi-word dates now stay content whole and byte-identical rather than half-eaten.
Not added, deliberately: an explicit delimiter (
@occurred:"June 10, 2026") would make multi-word dates work with one regex alternative and no ambiguity. That is new authored syntax, so it waits for evidence that people actually reach for it — the same "defer until a real use forces it" rule applied to SPEC-89.Also in this branch
(2026-01-01,2026-01-03)holds only Jan 2 and(2026-01-02,2026-01-04)holds only Jan 3, yet each raw endpoint sits inside the other. Date ranges now canonicalize to half-open[lower,upper)at construction, exactly as PostgreSQL normalizesdaterange, which makes the existing predicate correct with no special-casing. Instants are continuous and untouched. The authored token is preserved insource_text, so files round-trip byte-exact.Path, which renders with backslashes on Windows. Nowas_posix(), matching howentity.file_path, permalinks, and search rows all name files.Contract suite verified on both dialects (441 passed locally, 158 under Postgres). 100% coverage on
temporal.py,temporal_qualifier.py,temporal_filters.py.