Skip to content

Remember what a document declares, per file - #649

Merged
Firehed merged 21 commits into
mainfrom
pipeline/2-declaration-source
Sep 29, 2026
Merged

Firehed merged 21 commits into
mainfrom
pipeline/2-declaration-source

Conversation

@Firehed

@Firehed Firehed commented Sep 29, 2026

Copy link
Copy Markdown
Owner

Second of five stacked PRs, on top of the document source. #646 is the total view.

What was wrong

  • Reading a file, parsing it, scanning it, and building its infos was written out in three backends.
  • The symbol cache remembered answers by name, while what changes is a file. It kept side tables to bridge the two and dropped unrelated entries on every change.

What this does

  • DeclarationSourceInterface is the one way to get what a document declares. The three backends hold it in place of a parser, a scanner, and an info factory.
  • CachingDeclarationSource remembers declarations by the file's path and text together. Changed text is a different key, so nothing is invalidated. It holds 2,000 documents and drops the least recently used.
  • CachingSymbolSource is deleted.
  • TextDocument indexes line starts on first use. A document read only for what it declares is never asked for a position, and the index was most of the cost of a cached lookup.

Behaviour changes

  • A class deleted or renamed in an unsaved buffer no longer resolves from disk.
  • Identical text re-sent in a later message is not parsed again.

Measured

openemr, class-name completion while typing, median of seven sessions, change message plus request.

Typed main This branch
new Pa 165 ms 165 ms
new Pat 107 ms 103 ms
new Pati 123 ms 117 ms
new Pat again 116 ms 89 ms

Tests removed or rewritten

Each at Eric's direction.

  • CachingSymbolSourceTest is deleted with its subject.
  • CompositeInvalidatableTest drove the composite through the deleted class. It now changes the disk under each of the three members and asks that member.
  • ServerTest::testParsesAreScopedToOneMessage asserted that identical text re-sent must parse again. It is now testTheSyncPathParsesATextOnce.

DeclarationSymbolInfoFactory::fromDeclarations() has no caller left in src/. Two test files still use it.

Enforcement edits

Edit Class
phpstan.neon lookupConstant() allowlist: drop CachingSymbolSource Tighten

🤖 Generated with Claude Code

@codecov

codecov Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.65%. Comparing base (2fc97b4) to head (afa7542).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@             Coverage Diff              @@
##               main     #649      +/-   ##
============================================
- Coverage     99.65%   99.65%   -0.01%     
+ Complexity     1916     1903      -13     
============================================
  Files           135      136       +1     
  Lines          4946     4882      -64     
============================================
- Hits           4929     4865      -64     
  Misses           17       17              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@Firehed
Firehed added this pull request to stack #653 September 29, 2026 17:17
Comment thread src/Knowledge/KnowledgeStack.php Outdated
Comment thread src/Knowledge/KnowledgeStack.php Outdated
Base automatically changed from pipeline/1-document-source to main September 29, 2026 20:01
Firehed and others added 18 commits September 29, 2026 13:01
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@Firehed
Firehed force-pushed the pipeline/2-declaration-source branch from e3141a4 to 17124c1 Compare September 29, 2026 20:01
Firehed and others added 3 commits September 29, 2026 13:09
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Callers filter allIn() directly. Removes the derived verb the
one-route rule warned against: with the wrapper gone there is
no second walk that could fork from the one it derived from.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@Firehed
Firehed merged commit b5f69e1 into main Sep 29, 2026
9 checks passed
@Firehed
Firehed deleted the pipeline/2-declaration-source branch September 29, 2026 21:10
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