Cross Reference Feature and basic mappings

[augustjohnson]

This PR adds a new field into any object with a desc field, where it provides a set of cross-references to other objects.

The overall (empty) data structure looks like this, and appears on (almost) anything with a desc.

    "crossreferences": {
        "from": [],
        "to": []
    },

Within the from and to objects, is approximately this:

                "anchor": "Pantheons",
                "url": "http://localhost:8000/v2/rulesets/srd_pantheons/"
            }

The intent for the "to" dataset is for a UI layer to have data that, within the description of the object you are viewing, the word "Pantheons" reasonably means another object in the API, and that object's url.

The intent for the "from" dataset is to provide the UI with a list of "Things that link here".

How I Tested

The script I used to identify and generate crossreferences had a few "top 10" lists which I spot checked for general accuracy. There were various objects that resulted in false positives, like "Fly" the spell. These were added to special conditions (requiring context) or blacklists to prevent them from being linked at all.

There are quite a few items that have been blacklisted, including some standards. These can be modified or adjusted at a later time, but the list that is crossreferenced now is acceptable in my mind.

I also added a few internal tests, as well as ensured that each modified integration test showed the correct result.

I estimate that I checked probably 60 or 70 objects and their types, focused on the most crossreferenced across the two SRDs. They seemed good enough, but probably not perfect.

Follow Ups

I have added #880 to the backlog to provide the data for a few other documents. It will however require a few scripting and code changes.

Review Suggestions

• Check the changed responses in ./api_v2/tests/responses first.
• Check the fixtures that changed next.
• Check the models and serializer changes.

The scripting part is ugly and intended to be pretty temporary and ad-hoc usage.

[augustjohnson]

@calumbell monster PR incoming!

[calumbell]

Hey @augustjohnson, nice work getting this PR together. Looking forward to this hitting production!

As previously mentioned, this is a big one. Will likely take me some time to get everything reviewed. But off the bat I spotted that we are experiencing what I suspect are N+1 problems on Views where crossreferences are returned.

I A+B tested a few endpoints after noticing the API on the feature branch was a bit sluggish. I installed the Django Debug Toolbar to help diagnose the issues:

Here is /v2/magicitems on the staging branch. Note the CPU time the request took (512.81ms) and the number of SQL queries made (17)

Compare this with /v2/magicitems on feature branch. Note that the CPU time is more than 2.5x longer (1348.26ms) and the number of SQL queries made has ballooned to 131.

Seems like classic sympthoms of an N+1 problem with the new fields.

I observed similar results on the /spells endpoint, and not on the /creatures endpoint, which doesn't look like it returns the crossreferences field?

Would you be able to investigate? I had previously resolved similar issues by making sure serializers were correctly configured and then leveraging the EagerLoadingMixin on the problematic Views

[calumbell]

@augustjohnson - auxilary note: do you think I would be a good idea to add Django Debug Toolbar as a development-only dependency to this project? I use it on my local fork and have gotten a lot of value from it.

Let me know what you think. If you like the idea i'll open an issue : )

[augustjohnson]

On it, I'll see what I can find here. I'm not familiar with debug toolbar so I'll have to learn that.

[augustjohnson]

Ok, I've got it to 32 queries, but I'm not yet convinced that's where it needs to be. I'll keep tinkering with it. There's some funky queries in there.

Also, I like djang-debug-toolbar, I'd generally agree with adding it. I'm having trouble with dependencies right this minute though, so that may be a different PR.

Spells should be ok, and yes, it should return crossreferences.
Creatures don't have a description, therefore don't have any crossreferences.

[calumbell]

@augustjohnson sounds like progress! Ping me when you are ready for another review.

Yeah, this isn't the PR to add the django-debug-toolbar, i'll open a new issue.

Forgot that Creatures don't have a "desc" field 🤦

[augustjohnson]

Ok, I've got good news and bad news.

1. The prefetching works great, and it all comes through quickly and performantly.

Bad news.
2. This means that an individual crossreference object returned cannot differentiate between the /item/ or the /magicitem/ endpoint (this takes a separate query). A crossreference to Flaming Sword will always result in the /items/srd_flaming-sword url. At the moment I'm not considering this a bug, i'm thinking a on-hover effect would actually make the API call and then based on the response would show an "is magical" icon or something.

3. I recognized some bad matches when scrolling through objects, I think I need to revise my matching logic. For example https://beta.open5e.com/magic-items/srd-2024_armor-of-vulnerability-breastplate had a reference to spell Identify (good), spell Remove Curse (good), and spell Resistance (NO!).

[calumbell]

1. Excellent, glad you found a solution to those N+1 errors, nasty business!
2. I can live with that. Is this an artefact of the matching logic, or is it a contraint of the CrossRefernce model itself? What would happen if I were to manualy replace a URL pointing to an Item to point to a Magic Item?
3. Yeah, the matching logic is going to be a challenge to get right. Especially for those one-word spell titles that could easily be used in a regular sentence. Might be worth thinking about case-sensitivity (references to the Resistance spell would likely be capitalised, and use of the word in copy would be uncapitalised, except where it begins a sentence).

But TBH as long as you are happy with the schema, we can sweat the specifics of the matching algorithm in a follow up PR.

[augustjohnson]

Ok, I believe this is ready again for review @calumbell

[calumbell]

Hey @augustjohnson, thanks for making those changes. Looks like the N+1 errors are all fixed. The form of the API response is great, exactly what is needed on the FE.

Only remaining issue is that the matching algorithm is still quite off. The generation of false crossreferences seems very high. Perhaps we could narrow this process further:

• An idea might be to limit the automatic generation of crossreferences to between certain endpoints to solve common use-cases:
  a. CreatureAction -> Spell (this is to handle spells imbedded in statblocks)
  b. Spell -> Creature (to handle spells that summon/conjure specific creatures)
  c. MagicItem -> Spell (to handle magic items that can be used to cast spells)
  d. Spell -> Spell (some Spells mention other spells, ie. srd-2024_contingency mentions Water Breathing)
• Is it possible to only match a Spell when its name is written in Title Case (as it usually is when it is mentioned elsewhere).
• If the above is not possible, could we add Command to the blacklist?

Honestly, i'd be happy if we just striped the crossreference generator out for now, added a few example crossreferences in to help support getting this all hooked up on the frontend, and handling the crossreference generator in its own PR.

[augustjohnson]

Understood and agreed, I will do a data-lite PR. Just as a note, the method I took on matching quality was roughly this:

1. Look at the top 10 outbound matches by count of references and line up each one (this seemed reasonably correct).
2. Eyeball the top 10 inbound matches by count of references. (this was based on my gut feeling of how many times things like "detect magic" were going to be referenced vs random spells like earthquake). This seemed reasonably correct.

As I was going through that validation loop, I encountered a ton of references which needed to be blacklisted (best just to never reference this), and a bunch of special cases, which I wrote some code for ("Light" for example). Those were the two tools I used to try and resolve.

I'd be interested in your specific examples of bad matches, but I'm OK with the data population being pushed downstream.

[calumbell]

Cool, here are some specific examples of weird matches I spotted

Most common false match:

• Combat
• Command
• Darkness
• Darkvision (the spell often gets linked to when the sense is described)
• Defense (feat, fighting style)
• Fly
• Harm
• Healing

Less common false matches include

• Creation
• Dart
• Pouch
• Resting
• String
• Travel

Technically correct matches, but over-keen, occasionally to the point of absurdity

• Ammunition
• Book
• Chest
• Combat
• D20 Test
• Flask
• Jump
• Proficiency
• Temporary Hit Points
• Thrown (when the Thrown weapon property is actually mentioned, it typically appears capitalized)